diff --git a/backend/app/downloads/backfill.py b/backend/app/downloads/backfill.py index 8a34d485..ac2293b4 100644 --- a/backend/app/downloads/backfill.py +++ b/backend/app/downloads/backfill.py @@ -7,7 +7,8 @@ and safe to re-run: it only touches assets still missing the field, and only re- Facebook sources (a direct media URL like a reddit HLS manifest carries no channel and its signed URL may have expired — those just get a locally-generated poster instead). A Facebook download titled with the source's placeholder ("Facebook", "Video") is retitled from a signed-out read of its -page, and so is each download name that only repeated the placeholder. +page, and so is each download name that only repeated the placeholder. That page read +(`public_title_info`) also titles every new download, from the worker. Run once per environment: docker exec siftlode-worker-1 python -c "from app.downloads import backfill; print(backfill.run())" @@ -16,7 +17,7 @@ Run once per environment: from urllib.parse import urlsplit import yt_dlp -from sqlalchemy import select +from sqlalchemy import select, update from sqlalchemy.orm import Session from app import logs @@ -88,10 +89,13 @@ def _retitle(db: Session, a: MediaAsset, adapter: adapters.SourceAdapter, url: s named = normalize_title(adapter.title(info)) if not named or adapter.is_placeholder_title(named): return False - jobs = db.execute(select(DownloadJob).where(DownloadJob.asset_id == a.id)).scalars() - for job in jobs: - if job.display_name == a.title: - job.display_name = named[:DISPLAY_TEXT_MAX] + # One conditional write, not a read and a compare: a name its owner gives while this runs (the + # poster step can take seconds before the commit) is not overwritten by what was read before it. + db.execute( + update(DownloadJob) + .where(DownloadJob.asset_id == a.id, DownloadJob.display_name == a.title) + .values(display_name=named[:DISPLAY_TEXT_MAX]) + ) a.title = named return True diff --git a/backend/app/downloads/pipeline.py b/backend/app/downloads/pipeline.py index 215bdc44..1ccbf85a 100644 --- a/backend/app/downloads/pipeline.py +++ b/backend/app/downloads/pipeline.py @@ -7,6 +7,7 @@ from datetime import date, datetime, timezone from pathlib import Path import yt_dlp +from sqlalchemy import update from yt_dlp.utils import download_range_func from app import logs, sysconfig @@ -587,8 +588,10 @@ def _download( # too. Same probe the upload ingest uses. vcodec, acodec, width, height, duration_s = probe.resolve_media_facts(produced, info) # A placeholder title with nothing behind it ("Facebook" from a signed-in read) borrows the - # page's text from one signed-out read; the download itself stays the signed-in one. - titled = backfill.public_title_info(info, url, adapter, asset_id) + # page's text from one signed-out read; the download itself stays the signed-in one. The page + # yt-dlp read, not the link queued: a group post's link is rewritten to its video, and + # signed out the post itself is the login wall. + titled = backfill.public_title_info(info, info.get("webpage_url") or url, adapter, asset_id) meta = storage.MediaMeta( video_id=info.get("id") or source_ref, title=normalize_title(adapter.title(titled)) or source_ref, @@ -1120,14 +1123,20 @@ def _process_clip(job_id: int, asset_id: int) -> None: fields: dict = {"status": JobStatus.DONE, "progress": 100, "phase": None} # A video outside the catalog was queued as "Clip (clip)": the fetch has its title now. placeholder = service.clip_title(None, None) + named = None if asset and fetched_title and asset.title == placeholder: named = service.clip_title(fetched_title, None) asset.title = named - if job_row := db.get(DownloadJob, job_id): - if job_row.display_name == placeholder: - fields["display_name"] = named[:DISPLAY_TEXT_MAX] title = asset.title if asset else None finalized = service.finalize_if_running(db, job_id, **fields) + if finalized and named: + # Only a name that still repeats the placeholder, in one conditional write: one its + # owner gave meanwhile stays. + db.execute( + update(DownloadJob) + .where(DownloadJob.id == job_id, DownloadJob.display_name == placeholder) + .values(display_name=named[:DISPLAY_TEXT_MAX]) + ) db.commit() if finalized: _notify_download_done(db, user_id, title, job_id, kind="edit") diff --git a/backend/tests/test_facebook_titles.py b/backend/tests/test_facebook_titles.py index eb09ecf9..eb92db1f 100644 --- a/backend/tests/test_facebook_titles.py +++ b/backend/tests/test_facebook_titles.py @@ -6,11 +6,18 @@ no post text to fall back on, and the download name copied the placeholder too. the same reel (yt-dlp 2026.08.19, as on prod) still carries the post's text in its description. """ +import threading + +from sqlalchemy import update + from app.config import settings +from app.db import SessionLocal from app.downloads import adapters, backfill, pipeline from app.downloads.models import DownloadJob, MediaAsset _URL = "https://www.facebook.com/reel/1860682468422402/" +# The page yt-dlp says it read for it (`webpage_url`); a group post's link resolves the same way. +_PAGE = "https://www.facebook.com/watch/?v=1860682468422402" # The measured signed-out read of that reel. _PUBLIC = { "title": "163K reactions · 14K shares | sorry not helping #workhumor #officelife | Work dot exe", @@ -20,12 +27,13 @@ _PUBLIC = { def _fake_ydl(monkeypatch, outcome): - """yt-dlp faked at its `YoutubeDL` boundary: records each read's options, answers `outcome`.""" + """yt-dlp faked at its `YoutubeDL` boundary: records each read (its options and its `url`), + answers `outcome`.""" seen: list[dict] = [] class FakeYDL: def __init__(self, opts): - seen.append(opts) + self.opts = opts def __enter__(self): return self @@ -34,6 +42,7 @@ def _fake_ydl(monkeypatch, outcome): return False def extract_info(self, url, download): + seen.append({**self.opts, "url": url}) return outcome monkeypatch.setattr(backfill.yt_dlp, "YoutubeDL", FakeYDL) @@ -61,6 +70,14 @@ def test_a_real_title_reads_nothing_more(monkeypatch): assert seen == [] +def test_a_placeholder_with_the_posts_text_behind_it_reads_nothing_more(monkeypatch): + seen = _fake_ydl(monkeypatch, _PUBLIC) + info = {"title": "Video", "description": "Office jokes, part two"} + + assert backfill.public_title_info(info, _URL, adapters.FacebookAdapter(), 7) is info + assert seen == [] + + def test_a_placeholder_the_signed_out_page_cannot_better_stays(monkeypatch): _fake_ydl(monkeypatch, {"title": "Facebook", "description": None}) fb = adapters.FacebookAdapter() @@ -81,7 +98,7 @@ def test_another_source_is_never_read_again(monkeypatch): def test_the_backfill_retitles_a_placeholder_download_and_the_names_that_repeated_it( db, make_user, monkeypatch ): - _fake_ydl(monkeypatch, _PUBLIC) + seen = _fake_ydl(monkeypatch, _PUBLIC) owner, other = make_user(), make_user() asset = MediaAsset( source_kind="url", @@ -120,6 +137,7 @@ def test_the_backfill_retitles_a_placeholder_download_and_the_names_that_repeate db.commit() assert backfill.run()["retitled"] == 1 + assert len(seen) == 1 db.expire_all() assert db.get(MediaAsset, asset.id).title == "sorry not helping" @@ -127,6 +145,66 @@ def test_the_backfill_retitles_a_placeholder_download_and_the_names_that_repeate assert db.get(DownloadJob, renamed.id).display_name == "Office jokes" # Run again: nothing left to retitle, and no page read for it. assert backfill.run()["retitled"] == 0 + assert len(seen) == 1 + + +def test_a_name_given_while_the_backfill_runs_stays(db, make_user, monkeypatch): + # The owner renames the download between the backfill's retitle and its commit (the poster step + # sits between them): the owner's name wins, it is not overwritten by what was read before it. + _fake_ydl(monkeypatch, _PUBLIC) + owner = make_user() + asset = MediaAsset( + source_kind="url", + source_ref=_URL, + format_sig="fb", + status="ready", + rel_path="facebook/x.mp4", + uploader_url="https://www.facebook.com/1", + title="Facebook", + ref_count=1, + ) + db.add(asset) + db.flush() + job = DownloadJob( + user_id=owner.id, + asset_id=asset.id, + source_kind="url", + source_ref=_URL, + status="done", + queue_pos=0, + profile_snapshot={}, + display_name="Facebook", + ) + db.add(job) + db.commit() + + def rename(): + with SessionLocal() as other: + other.execute( + update(DownloadJob) + .where(DownloadJob.id == job.id) + .values(display_name="Office jokes") + ) + other.commit() + + def poster_while_renamed(a, root): + # A rename that must wait for the backfill's write waits here, and lands after its commit. + t = threading.Thread(target=rename) + t.start() + t.join(timeout=0.5) + renamers.append(t) + return None + + renamers: list[threading.Thread] = [] + monkeypatch.setattr(backfill.editmod, "ensure_poster", poster_while_renamed) + + assert backfill.run()["retitled"] == 1 + for t in renamers: + t.join(timeout=10) + assert not t.is_alive() + + db.expire_all() + assert db.get(DownloadJob, job.id).display_name == "Office jokes" def test_a_download_titled_facebook_is_named_from_the_signed_out_page( @@ -134,7 +212,7 @@ def test_a_download_titled_facebook_is_named_from_the_signed_out_page( ): # The whole download path with yt-dlp and the media tools faked: the signed-in read gives only # the placeholder, and the file and the download's name get the post's text. - _fake_ydl(monkeypatch, _PUBLIC) + seen = _fake_ydl(monkeypatch, _PUBLIC) monkeypatch.setattr(settings, "files_root", str(tmp_path)) monkeypatch.setattr(pipeline.urlguard, "check_url", lambda url: None) monkeypatch.setattr(pipeline, "_ensure_phone_safe", lambda _j, produced, *_a: produced) @@ -168,12 +246,16 @@ def test_a_download_titled_facebook_is_named_from_the_signed_out_page( "id": "1860682468422402", "title": "Facebook", "uploader": "Work dot exe", + "webpage_url": _PAGE, "requested_downloads": [{"filepath": str(f)}], } monkeypatch.setattr(pipeline, "_extract_with_fallback", signed_in_read) pipeline._download(job.id, asset.id, "url", _URL, {}, owner.id) + # The signed-out read goes to the page yt-dlp read, not the link queued: a group post's link + # resolves to its video, and signed out the post itself is the login wall. + assert [r["url"] for r in seen] == [_PAGE] db.expire_all() assert db.get(DownloadJob, job.id).status == "done" assert db.get(MediaAsset, asset.id).title == "sorry not helping" diff --git a/backend/tests/test_upload.py b/backend/tests/test_upload.py index 7462d1b3..e0fcd46d 100644 --- a/backend/tests/test_upload.py +++ b/backend/tests/test_upload.py @@ -579,7 +579,7 @@ def _display_write_sites() -> dict[tuple[str, str], set[str]]: # enqueue_clip slices the source's title to the same width. # files.ingest_spooled passes through `spool_upload`'s value, bounded there. # routes enqueue_clip the `display_name=` keyword passes on the name bound above it. -# pipeline._download / _process_clip / worker._finish_from_cache +# pipeline._download / _process_clip / worker._finish_from_cache / backfill._retitle # slice a fetched or stored title to the column's width. # # Held as EXACT EQUALITY rather than as a list of offenders. The offender form could only fail diff --git a/docs/DESIGN.md b/docs/DESIGN.md index 141f93eb..ea073709 100644 --- a/docs/DESIGN.md +++ b/docs/DESIGN.md @@ -831,4 +831,5 @@ of a full-width panel and nothing on the right. | 2026-10-28 | Notification order (owner A-D1 a, A-D2 a, ★ a) | What asks for the reader first, from either source ("Needs you · n"), then one stream newest first; read rows older than a week fold under "Earlier (n)", closed, its label in the section-heading style with a chevron | One stream with no band; this browser's activity above the server's (the two fixed sections); a server-side retention that deletes old rows | An admin's access requests sat under up to a hundred server rows; week-old read notices crowded what is new; folding keeps the rows (nothing is deleted) and an unread row never folds, whatever its age | | 2026-11-01 | Docked bar glyphs (owner D8 a) | The maximised view's docked bar is glyph-only too (tooltips carry the names and keys): transport, auto-advance, loop, Mark watched (✓ / ✓✓); its controls scroll on a narrow window while Fullscreen and Exit stay pinned at its right end | Labels from `xl` up | On a tablet the labelled bar ran past the right edge and took Exit with it; the side panel had already lost its labels | | 2026-11-01 | Toolbar group glyphs (owner D9 a, D10 a, D11 a) | The feed's, channel page's and Plex Browse's filter pills name their group by a glyph (`Eye` watch state, `Shapes` content types, `ArrowDownUp` sort), the word in the tooltip and the accessible name; Sort + View end the toolbar's row, the chip row shows only with a chip; a channel page has no count pill | The value alone; Sort + View on a row of their own | "Watch state:" and "Sort:" cost a tablet row its room; the value says enough once the glyph names the group; the channel header already counts the videos | -| 2026-11-01 | Channel header on a tablet (owner E2) | Below `xl` the action row drops its two words: "Prio" (the stepper's tooltip names it) and "Subscribed" (a ✓ glyph; its name in the aria-label, Unsubscribe in the tooltip); the tags keep the row's rest, folding into "+N" | The tag lane on a full line of its own below ~12rem; the tags under the channel name | On an iPad (820 px, 44 px targets) the lane got 29 px, a sliver; now 130 px, and every tag shows at 1180 | +| 2026-11-01 | Channel header on a tablet (owner E2) | Below `xl` the action row drops its two words: "Prio" (the stepper's tooltip names it) and "Subscribed" (a ✓ glyph; its name in the aria-label, Unsubscribe in the tooltip); the tags keep the row's rest, folding into "+N" | The tag lane on a full line of its own below ~12rem; the tags under the channel name | On an iPad (820 px, 44 px targets) the lane got 29 px, a sliver; now 130 px, and every tag shows at 1180 | +| 2026-11-01 | Clear watch progress (owner E a) | A video menu's Reset reads "Clear watch progress" in English (HU "Megtekintés visszaállítása" stays); "Reset view" is only a page's view reset (sort, filters, grouping) | "Reset view" for both; the playlist page's view reset renamed instead | On a playlist page the two stood one right-click apart under one name, and one of them erases the video's progress with no undo | diff --git a/frontend/src/components/ui/DataTable.cards.dom.test.tsx b/frontend/src/components/ui/DataTable.cards.dom.test.tsx index abf57aef..f543a227 100644 --- a/frontend/src/components/ui/DataTable.cards.dom.test.tsx +++ b/frontend/src/components/ui/DataTable.cards.dom.test.tsx @@ -2,9 +2,9 @@ // // Below `md` a table is a list of cards, its columns flowing one after another. A column marked // `cardRow` takes a line of its own there — a row's lone "…" otherwise landed at the end of one -// card's line and on the next line of another. +// card's line and on the next line of another. A card is its row's press surface too, as the row is. import { describe, it, expect, afterEach, vi } from "vitest"; -import { render, cleanup, screen } from "@testing-library/react"; +import { render, cleanup, screen, fireEvent } from "@testing-library/react"; import DataTable, { type Column } from "./DataTable"; vi.mock("react-i18next", async (importOriginal) => ({ @@ -43,4 +43,21 @@ describe("DataTable cards", () => { expect(screen.getByText("more").parentElement!.className).toContain("basis-full"); expect(screen.getByText("1 MB").parentElement!.className).not.toContain("basis-full"); }); + + it("opens its object from a tap on the card, and leaves its control's tap to the control", () => { + narrow(false); + const open = vi.fn(); + render( + r.id} + rowOpen={() => open} + />, + ); + fireEvent.click(screen.getByText("1 MB"), { detail: 1 }); + expect(open).toHaveBeenCalledTimes(1); + fireEvent.click(screen.getByText("more"), { detail: 1 }); + expect(open).toHaveBeenCalledTimes(1); + }); }); diff --git a/frontend/src/components/ui/README.md b/frontend/src/components/ui/README.md index adfdacc0..7418c0ad 100644 --- a/frontend/src/components/ui/README.md +++ b/frontend/src/components/ui/README.md @@ -35,13 +35,13 @@ Counter-examples, deliberately NOT here: `PosterTile` (knows about Plex montage ## What lives here -| area | components | -| ---------------------- | --------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| overlays & anchoring | `AnchoredPopover` + `anchoredPopoverHooks` (the popover engine: arrows, typeahead, Escape, anchor-flip), `ActionMenu`, `MembershipMenu` (the body of a "which sets is it in" menu: checkbox rows + a field naming a new set), `ContextMenu` + `useContextMenu` (the SAME menu, opened by right-click / long-press / Shift+F10 / a row's `MoreButton` "…" via `buttonProps`; `anchor` for a panel a row opens; `PageMenu` for a page's background menu), `Modal` (+ `ModalFooter`; `initialFocus` by purpose, C4), `Overlay`, `SidePanel`, `Tooltip`, `Toaster` (+ `toastLevels`, `overlayOrder`; `lib/toastDock` docks one control under the stack, in every host), `CommandPalette`, `KeyboardHelp`, `ConfirmProvider` + `confirmContext` (`useConfirm` / `useChoice`: the app's promise-based confirm, mounted once at the root; a decision with more than two outcomes is one `useChoice`, never two chained confirms), `dialogFocus` (the focus half of the dialog contract: `Modal`'s, and the wizard's own shell), `menuGroups` (one menu from its groups, a rule only between non-empty ones), `pageMenuSlot` + `pageMenuItems` (a page's background menu, mirroring its own toolbar) | -| actions & inputs | `Button`, `IconButton`, `Pressable` (`selectable` when its text is a title or a name), `Chip` + `chipStyles`, `DragHandle` + `OrderButtons` / `useOrderItems` (the non-drag reorder, WCAG 2.5.7) (one per button family, docs/DESIGN.md; a raw `