Release 0.81.1 - #4008
Open
odlbot wants to merge 13 commits into
Open
Release 0.81.1#4008odlbot wants to merge 13 commits into
odlbot wants to merge 13 commits into
Conversation
…file (de)indexing (#3976)
#3963) * feat(website-content): unpublish published items from the listing card Adds the three-dot menu from the design to published article and news listing cards, with Unpublish as its single action, and drops the Unpublish button from the content detail toolbar. The menu renders only for users who can edit content, so no callsite can leak it. Draft cards link to the editor, which is the only page they have. Unpublishing now takes effect within the request: the news feed entry and the article's LearningResource -- published flag and Qdrant points -- go before the response, so the listing's refetch and vector search stop serving what was just taken down. The view cache is cleared after them, so nothing can re-cache the stale listing, and vector search hydrates only published resources. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): reconcile a sync that overtakes an unpublish Both sync tasks check `is_published` with a read and then write, and unpublishing now runs in the request, so it can land between the two with nothing queued behind it to notice: the sync would restore a published, indexed resource -- and the news feed entry -- for content that is no longer public. Whoever writes last reconciles, so each sync task re-reads the row after writing and undoes itself if the item has since been unpublished. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(vector-search): check publication on the payload path too The published-row filter only covered database hydration, which the kill switch turns on; the default path answers from the Qdrant payloads, where a delete that is late or has failed outright kept serving an unpublished article. A payload cannot report this itself -- unpublishing deletes the point rather than rewriting it, so a stale payload still reads as published and no Qdrant-side filter can catch it. The payload path now costs one indexed lookup, asked negatively: only rows the database reports as unpublished are dropped, so a point with no row here -- a snapshot loaded from another system -- is still returned. Covered at the view level on the shipped default, not just in the hit builder. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(api): assert the patch hook invalidates the listings The page tests only assert that the PATCH was sent, so dropping the list invalidation still passed while a card unpublished from the listing stayed in the cache -- and on screen. The hook's own test now pins all three keys it invalidates, the detail retrieve one having been uncovered in the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): drop the topics section for news settings Only an article is projected into a LearningResource, so only there do topics put the content on a topic page. The news drawer keeps its SEO section and no longer fetches the topic list at all. The drawer stays presentational -- the caller decides with `showTopics` -- and reports no topics rather than an empty selection when the section is hidden, so saving news settings cannot empty a selection the editor was never shown, or re-run the publish plugins for nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): require topics to save an article An article's topics are what put it on a topic page, so saving one without them now opens the settings drawer instead -- for a draft save as well as a publish -- and the section says why it opened. News has no topics section at all, so nothing is required of it. The held-back press resumes once a topic is picked, carrying the new selection into the content write rather than PATCHing topics separately, and a publish still confirms as it would have. Closing the drawer abandons it, so a press the editor walked away from cannot fire the next time topics happen to be saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(website-content): pin that the unpublish hooks run in autocommit The hooks call out to Qdrant synchronously and swallow a DatabaseError to fall back to a queued task. Both are safe only outside a transaction: inside one, the call would hold it open across network I/O, and the swallowed error would poison it -- the fallback would never be queued and the next query would raise TransactionManagementError. Nothing asks for a transaction today (no ATOMIC_REQUESTS, no atomic in the write path), which two review passes have now assumed otherwise, so assert it rather than leave it to be re-derived. Verified to fail when ATOMIC_REQUESTS is switched on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert(vector-search): drop the read-path publication filter Filtering hits against the database broke the counts beside them: on the no-query path `total` comes from Qdrant's own exact count() and the facets from facet(), neither of which knows about a Postgres filter, so totals and doc_counts over-reported and the last page came back short -- the very thing exact=True was set to prevent. Reconciling that properly means post-filtering the whole matched set in Python. The guard was partial regardless: content file hits were never checked, and unpublishing leaves content files in Qdrant by design. The removal path converges on its own -- inline delete, the retrying queued task behind it, and the sync task reconciling against the row -- so trust it. Also drops the points via remove_points_matching_params, keyed on the readable_id already in hand, rather than remove_embeddings, which re-serializes the resource only to derive the same filter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): re-attempt a failed reconciliation Both sync tasks undo themselves when the row turns out to be unpublished by the time they have written, and neither undo was retried on failure. In the news task the undo raises inside the task's own try, so the whole task retries -- and the retry took the "not published" path, which returned without touching the entry the previous attempt had created. That path now removes any entry instead of skipping, which is what makes the retry effective; deleting by guid is a no-op when there is nothing there, so the ordinary "queued, then unpublished" case is unchanged. The learning resource task does not retry at all, so a failed undo left the resource published and indexed with nothing behind it. It now hands the undo to the task that does retry, whose republish guard makes a late run safe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): do not fail an unpublish over the index work The inline removal caught only DatabaseError, but it runs the search and vector hooks inline, which fail in other ways -- an unreachable broker escapes try_with_retry_as_task, whose own fallback is an unguarded .delay(). That reached the client as a 500 after both rows had already committed as unpublished, and a retried request fires no hooks at all, since perform_update keys them off the published->unpublished transition. The deindex and the Qdrant removal were simply lost. Any failure now hands off to the retrying task, which redoes the removal in full. Queueing can fail too, for the same broker reason, so that is logged and the indexes are left to the next reindex rather than failing an unpublish whose own rows are already correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): close the drawer's way past the topics rule Gating the save buttons left the drawer itself open: it is the one place a selection can be taken away, so removing every topic and saving straight from there PATCHed `topics: []` -- an article, published one included, left without the topics that put it on a topic page. Saving is now refused while a required selection is empty, which the section already explains, rather than silently ignoring the removal the editor can see on screen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): catch any failed undo, not just a database one The reconciliation caught `DatabaseError` alone, but the undo it guards runs the search and vector hooks inline, which fail in other ways -- an unreachable broker escapes `try_with_retry_as_task`, whose own fallback is an unguarded `.delay()`. This task does not retry, so that raised straight out, leaving the resource unpublished in the database and still in the index with nothing queued to remove it. Any failure now hands the undo to the retrying task, and queueing is itself guarded, since the broker is what may have failed. The same shape as the plugin's inline removal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ahtesham Quraish <ahtesham.quraish@arbisoft.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…3977) * Sanitize MITPE news summary/content like every other source Signed-off-by: Alex H <ahiguera@mit.edu> * Sanitize MITPE events summary/content, same gap as mitpe_news Signed-off-by: Alex H <ahiguera@mit.edu> * Address review feedback: dedupe sanitization, test onerror payload Signed-off-by: Alex H <ahiguera@mit.edu> --------- Signed-off-by: Alex H <ahiguera@mit.edu>
…ons (#3984) Signed-off-by: Alex H <ahiguera@mit.edu>
* WIP cut at adding a posthog purchase event * Add readable_id and contentType to posthog capture
…on (#3978) * feat(website-content): unpublish published items from the listing card Adds the three-dot menu from the design to published article and news listing cards, with Unpublish as its single action, and drops the Unpublish button from the content detail toolbar. The menu renders only for users who can edit content, so no callsite can leak it. Draft cards link to the editor, which is the only page they have. Unpublishing now takes effect within the request: the news feed entry and the article's LearningResource -- published flag and Qdrant points -- go before the response, so the listing's refetch and vector search stop serving what was just taken down. The view cache is cleared after them, so nothing can re-cache the stale listing, and vector search hydrates only published resources. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): reconcile a sync that overtakes an unpublish Both sync tasks check `is_published` with a read and then write, and unpublishing now runs in the request, so it can land between the two with nothing queued behind it to notice: the sync would restore a published, indexed resource -- and the news feed entry -- for content that is no longer public. Whoever writes last reconciles, so each sync task re-reads the row after writing and undoes itself if the item has since been unpublished. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(vector-search): check publication on the payload path too The published-row filter only covered database hydration, which the kill switch turns on; the default path answers from the Qdrant payloads, where a delete that is late or has failed outright kept serving an unpublished article. A payload cannot report this itself -- unpublishing deletes the point rather than rewriting it, so a stale payload still reads as published and no Qdrant-side filter can catch it. The payload path now costs one indexed lookup, asked negatively: only rows the database reports as unpublished are dropped, so a point with no row here -- a snapshot loaded from another system -- is still returned. Covered at the view level on the shipped default, not just in the hit builder. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(api): assert the patch hook invalidates the listings The page tests only assert that the PATCH was sent, so dropping the list invalidation still passed while a card unpublished from the listing stayed in the cache -- and on screen. The hook's own test now pins all three keys it invalidates, the detail retrieve one having been uncovered in the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): drop the topics section for news settings Only an article is projected into a LearningResource, so only there do topics put the content on a topic page. The news drawer keeps its SEO section and no longer fetches the topic list at all. The drawer stays presentational -- the caller decides with `showTopics` -- and reports no topics rather than an empty selection when the section is hidden, so saving news settings cannot empty a selection the editor was never shown, or re-run the publish plugins for nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): require topics to save an article An article's topics are what put it on a topic page, so saving one without them now opens the settings drawer instead -- for a draft save as well as a publish -- and the section says why it opened. News has no topics section at all, so nothing is required of it. The held-back press resumes once a topic is picked, carrying the new selection into the content write rather than PATCHing topics separately, and a publish still confirms as it would have. Closing the drawer abandons it, so a press the editor walked away from cannot fire the next time topics happen to be saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(website-content): pin that the unpublish hooks run in autocommit The hooks call out to Qdrant synchronously and swallow a DatabaseError to fall back to a queued task. Both are safe only outside a transaction: inside one, the call would hold it open across network I/O, and the swallowed error would poison it -- the fallback would never be queued and the next query would raise TransactionManagementError. Nothing asks for a transaction today (no ATOMIC_REQUESTS, no atomic in the write path), which two review passes have now assumed otherwise, so assert it rather than leave it to be re-derived. Verified to fail when ATOMIC_REQUESTS is switched on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert(vector-search): drop the read-path publication filter Filtering hits against the database broke the counts beside them: on the no-query path `total` comes from Qdrant's own exact count() and the facets from facet(), neither of which knows about a Postgres filter, so totals and doc_counts over-reported and the last page came back short -- the very thing exact=True was set to prevent. Reconciling that properly means post-filtering the whole matched set in Python. The guard was partial regardless: content file hits were never checked, and unpublishing leaves content files in Qdrant by design. The removal path converges on its own -- inline delete, the retrying queued task behind it, and the sync task reconciling against the row -- so trust it. Also drops the points via remove_points_matching_params, keyed on the readable_id already in hand, rather than remove_embeddings, which re-serializes the resource only to derive the same filter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): re-attempt a failed reconciliation Both sync tasks undo themselves when the row turns out to be unpublished by the time they have written, and neither undo was retried on failure. In the news task the undo raises inside the task's own try, so the whole task retries -- and the retry took the "not published" path, which returned without touching the entry the previous attempt had created. That path now removes any entry instead of skipping, which is what makes the retry effective; deleting by guid is a no-op when there is nothing there, so the ordinary "queued, then unpublished" case is unchanged. The learning resource task does not retry at all, so a failed undo left the resource published and indexed with nothing behind it. It now hands the undo to the task that does retry, whose republish guard makes a late run safe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): do not fail an unpublish over the index work The inline removal caught only DatabaseError, but it runs the search and vector hooks inline, which fail in other ways -- an unreachable broker escapes try_with_retry_as_task, whose own fallback is an unguarded .delay(). That reached the client as a 500 after both rows had already committed as unpublished, and a retried request fires no hooks at all, since perform_update keys them off the published->unpublished transition. The deindex and the Qdrant removal were simply lost. Any failure now hands off to the retrying task, which redoes the removal in full. Queueing can fail too, for the same broker reason, so that is logged and the indexes are left to the next reindex rather than failing an unpublish whose own rows are already correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): save drafts automatically, drop the draft button A draft writes itself a couple of seconds after typing stops, and the control bar says so -- "Saving..." while the write is in flight, then "Saved" until the next edit, as the design has it. The Save as Draft button is gone with it; a draft that has never been saved comes into existence the same way. Published content is untouched: every save there pushes edits live, so it stays an explicit press of Publish. Topics are consequently required to publish rather than to save at all. Autosave cannot stop to ask, and a drawer opening itself on a timer while someone types is not a prompt -- it is an interruption. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): lay the control bar out as the design has it The status leads the row and the actions follow at the other end, with the settings control reduced to its icon and paired with Publish. Nothing labels the icon on screen any more, so it carries an aria-label and a title -- the existing "Settings" queries keep working through the former. Shared with the published view's bar, so the control looks the same wherever it appears. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): align the action row with the article's column The row spanned the toolbar's full width, so the status sat against the far left and the actions against the far right, well outside the text they belong to. It now takes the article's own column -- the same 890px centred, 24px-padded column the banner and the body already use -- so the status lines up with the breadcrumb and title, and the actions with the far edge of the text. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): register the save indicator before it speaks `role="status"` only announces reliably if the region was already in the page when its text changed; mounted in the same paint as its first message, that message is routinely dropped -- and it is the one that matters. The region is now mounted empty for the session and only its text changes. The spinner beside it is hidden from the region too: the text says the same thing, and its own "Loading" label would be read out as well. Also carries the edit-bar changes made alongside this -- a shorter status readout, a Publish button that no longer names the content type, and no Delete in the bar -- with the code they left dead removed and the tests brought in line. Deleting a draft remains on the drafts listing, which covers it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * style(website-content): put the settings icon in a button box `bordered` rather than `text`, so it reads as a button alongside Publish and matches its height, with the icon still its only content. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * refactor(website-content): a flag, since only a publish is held back `pendingSave` was a tri-state remembering whether the held-back save was a publish, from when a draft save could be held back too. Since the draft button went, nothing set it to false, so the draft branch in `handleSettingsSave` was unreachable -- and `saveQuietly`, whose only caller it was, was dead with it. Now `awaitingTopicsForPublish`, a boolean that says what it is. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): close the drawer's way past the topics rule Gating the save buttons left the drawer itself open: it is the one place a selection can be taken away, so removing every topic and saving straight from there PATCHed `topics: []` -- an article, published one included, left without the topics that put it on a topic page. Saving is now refused while a required selection is empty, which the section already explains, rather than silently ignoring the removal the editor can see on screen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): refuse an empty topic selection only once published The drawer's gate arrived with the stricter rule it was written for, where a draft could not be saved without topics either. Autosave changed that: a draft writes itself and cannot stop to ask, so it may sit without topics and is stopped at publishing instead. Left as it was, an editor could not remove a topic from a draft at all. So the two rules are now separate props. `topicsRequired` still says topics are wanted before publishing, which is what tells an editor why the drawer opened on them; `topicsMayNotBeEmptied` refuses the save, and the editor passes it only for content that is already public. The copy follows whichever rule is speaking rather than claiming a draft cannot be saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): catch any failed undo, not just a database one The reconciliation caught `DatabaseError` alone, but the undo it guards runs the search and vector hooks inline, which fail in other ways -- an unreachable broker escapes `try_with_retry_as_task`, whose own fallback is an unguarded `.delay()`. This task does not retry, so that raised straight out, leaving the resource unpublished in the database and still in the index with nothing queued to remove it. Any failure now hands the undo to the retrying task, and queueing is itself guarded, since the broker is what may have failed. The same shape as the plugin's inline removal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): stop autosave re-navigating and re-creating Every autosave calls `onSave`, and the edit page answered it by pushing the route it is already on -- pointless work every couple of seconds, since the page reads its item through React Query, which the mutation already invalidates. The push is now guarded on the pathname, which also drops the same redundant push the draft button used to make. No progress bar was flickering, though: `next-nprogress-bar` compares the target with the current URL and suppresses the bar for a same-URL push. It still pushes, which is what this saves. Separately, an item created by autosave could be created twice. The caller moves the editor to the new item's URL only once the create has returned, so a second write before the route changed would insert another row; the editor now remembers what it created and updates that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): tell the caller only when the item has moved `onSave` exists to move the editor to where the saved item now lives, which is somewhere new on two transitions only: the save that created the item and the one that published it. Fired on every autosave, it asked the caller to navigate to the route it was already on, for as long as anyone kept typing. The edit page's pathname guard stayed as it is -- it also covers a URL that names the item by slug -- but the churn is now stopped at the source rather than at one caller of many. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): serialize the writes, and fix the review's findings The reviewed items, in order: Drop the MUI `Container` wrapper. It is not the 890px column its comment claimed -- that is `TiptapEditor`'s own local `Container` -- but MUI's, which defaults to `maxWidth="lg"` and adds gutters of its own on top of the column `ActionRow` already implements. Serialize every write through one queue. A publish confirmed while a debounced draft save was still running raced it, and whichever landed last decided whether the item ended up public, whatever the dialog reported. A publish from this editor now also stops further draft writes: `contentItem` still says draft, so one queued behind it would take the item straight back down. The publish spinner stops on failure too, rather than staying on for the next autosave. Hold a created item until nothing is unsaved before handing it to the caller, which navigates and so unmounts the editor along with anything typed while the create was in flight. Stop the bar claiming "Saved" through the debounce after the next keystroke: it is read together with whether anything has changed since. Give the removal task a retry policy. The callers that hand off to it describe it as the one carrying the retries, and it had none. Not changed: the report of a failing save being retried every two seconds. An instrumented probe -- slow failing response, so the pending transition is observable -- fires the save exactly once and never re-arms, so there is nothing to guard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): a publish supersedes a pending handoff A created item is held until nothing is unsaved before being handed to the caller, which navigates. Confirming a publish while the create was still in flight left that handoff waiting on `isPending`, and it fired once the publish settled -- handing over the create response, which says `is_published: false`, so the caller sent the editor to the draft page for an item that was now public. The publish now clears it, and the effect refuses to hand anything over after a publish from this editor. Two of the tests here typed into a heading node captured before a re-render, so the keystrokes went nowhere and the save they waited for never came. That was the whole of the flakiness in these suites: with the node re-queried, seven consecutive runs are clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): narrow the request body the test reads `RequestArgs.body` is `unknown` in the request harness, so reading `is_published` off it is an error. CI caught it and I had not: the repo-level typecheck stops at the first workspace that fails, and this checkout's `api` workspace fails on its own before `main` is reached -- its @mitodl/mitxonline-api-axios is older than the code expects. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(website-content): keep autosave out of the tests it is not about Autosave gave every typing test a two-second timer that fired mid interaction, re-rendering the toolbar and warning about updates outside `act` -- which the suites fail on. It surfaced as a different test each run, on CI as the edit page's drawer tests. The editors and the edit page now take the delay as a prop, defaulting to what production uses. The suites set it far out so nothing saves while they work, and the tests that are about autosave set it back. The create test no longer types while the create is in flight, which is where those gaps were widest; it types once the first save has settled, which exercises the same thing -- a second save updating what was created rather than inserting another row. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): queue the settings write with the others Autosave went through the queue; the settings write did not. A content write already in flight carries the topics as they were when it started, so the two overlapped and the older list could be what the server stored last -- taking the selection the editor had just made back out, with nothing on screen to say so. It is queued now, like the publish. The test for it kept the indicator in its saving state long enough to surface a second defect: the indicator was a `Typography`, so a `<p>`, and the spinner it holds renders a div. It is its own span now, taking the typography from the theme, since `styled()` drops the polymorphic `component` prop that would have changed the tag. Also awaits the news suite's heading query. Under load the editor's content mounts after its container, and the synchronous query missed it about one run in three. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ahtesham Quraish <ahtesham.quraish@arbisoft.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* fix(product-pages): smooth FAQ accordion open/close animation
…lic default (#3970) * fix(webhooks): make WEBHOOK_SECRET fail closed instead of using a public default WEBHOOK_SECRET fell back to the literal string "please-change-this" when unset, with nothing checking it was ever overridden -- unlike the sibling UNSUBSCRIBE_SECRET_KEY, which already raises ImproperlyConfigured if left unset. Since that fallback is a public string checked into this open-source repo, any deployment that ever left it unset would have its three webhook endpoints (course-content ingestion, OVS video upsert, content-file deletion) protected by a "secret" anyone can already read. Raise ImproperlyConfigured at startup instead, matching the existing UNSUBSCRIBE_SECRET_KEY pattern. Adds a non-default WEBHOOK_SECRET to the places that need one to keep booting: pytest's env config, CI's test job, and the local dev env template. * test(settings): add WEBHOOK_SECRET to REQUIRED_SETTINGS, cover legacy-default rejection settings_test.py's REQUIRED_SETTINGS mapping is used to reload main.settings under a cleared, minimal environment across dozens of tests (including an unconditional tearDown reload after every test in the class). Without WEBHOOK_SECRET in that mapping, every one of those reloads now hit the new ImproperlyConfigured check added in the previous commit -- breaking the entire suite, not just webhook-specific tests. Adding it here also means the existing test_required_settings loop (which asserts ImproperlyConfigured for each required setting when unset) automatically exercises the missing-WEBHOOK_SECRET case. Added a second, explicit test for the other branch that loop doesn't reach: rejecting the value when it's present but equal to the legacy "please-change-this" default. Caught by Copilot's review on PR #3970. * fix(env): add WEBHOOK_SECRET and UNSUBSCRIBE_SECRET_KEY to codespaces.env docker-compose.codespaces.yml's web/watch/celery services set env_file: env/codespaces.env directly via extends, which replaces (rather than merges with) whatever env file docker-compose.apps.yml's base services normally use -- so this file is genuinely the only env source for those services in Codespaces. It had neither secret, so Codespaces would fail to start both before this PR (UNSUBSCRIBE_SECRET_KEY, a pre-existing gap) and after it (WEBHOOK_SECRET, newly required by the previous commit). Caught by Sentry's bug-prediction bot on PR #3970.
* feat(website-content): unpublish published items from the listing card Adds the three-dot menu from the design to published article and news listing cards, with Unpublish as its single action, and drops the Unpublish button from the content detail toolbar. The menu renders only for users who can edit content, so no callsite can leak it. Draft cards link to the editor, which is the only page they have. Unpublishing now takes effect within the request: the news feed entry and the article's LearningResource -- published flag and Qdrant points -- go before the response, so the listing's refetch and vector search stop serving what was just taken down. The view cache is cleared after them, so nothing can re-cache the stale listing, and vector search hydrates only published resources. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): reconcile a sync that overtakes an unpublish Both sync tasks check `is_published` with a read and then write, and unpublishing now runs in the request, so it can land between the two with nothing queued behind it to notice: the sync would restore a published, indexed resource -- and the news feed entry -- for content that is no longer public. Whoever writes last reconciles, so each sync task re-reads the row after writing and undoes itself if the item has since been unpublished. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(vector-search): check publication on the payload path too The published-row filter only covered database hydration, which the kill switch turns on; the default path answers from the Qdrant payloads, where a delete that is late or has failed outright kept serving an unpublished article. A payload cannot report this itself -- unpublishing deletes the point rather than rewriting it, so a stale payload still reads as published and no Qdrant-side filter can catch it. The payload path now costs one indexed lookup, asked negatively: only rows the database reports as unpublished are dropped, so a point with no row here -- a snapshot loaded from another system -- is still returned. Covered at the view level on the shipped default, not just in the hit builder. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(api): assert the patch hook invalidates the listings The page tests only assert that the PATCH was sent, so dropping the list invalidation still passed while a card unpublished from the listing stayed in the cache -- and on screen. The hook's own test now pins all three keys it invalidates, the detail retrieve one having been uncovered in the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): drop the topics section for news settings Only an article is projected into a LearningResource, so only there do topics put the content on a topic page. The news drawer keeps its SEO section and no longer fetches the topic list at all. The drawer stays presentational -- the caller decides with `showTopics` -- and reports no topics rather than an empty selection when the section is hidden, so saving news settings cannot empty a selection the editor was never shown, or re-run the publish plugins for nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): require topics to save an article An article's topics are what put it on a topic page, so saving one without them now opens the settings drawer instead -- for a draft save as well as a publish -- and the section says why it opened. News has no topics section at all, so nothing is required of it. The held-back press resumes once a topic is picked, carrying the new selection into the content write rather than PATCHing topics separately, and a publish still confirms as it would have. Closing the drawer abandons it, so a press the editor walked away from cannot fire the next time topics happen to be saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(website-content): pin that the unpublish hooks run in autocommit The hooks call out to Qdrant synchronously and swallow a DatabaseError to fall back to a queued task. Both are safe only outside a transaction: inside one, the call would hold it open across network I/O, and the swallowed error would poison it -- the fallback would never be queued and the next query would raise TransactionManagementError. Nothing asks for a transaction today (no ATOMIC_REQUESTS, no atomic in the write path), which two review passes have now assumed otherwise, so assert it rather than leave it to be re-derived. Verified to fail when ATOMIC_REQUESTS is switched on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert(vector-search): drop the read-path publication filter Filtering hits against the database broke the counts beside them: on the no-query path `total` comes from Qdrant's own exact count() and the facets from facet(), neither of which knows about a Postgres filter, so totals and doc_counts over-reported and the last page came back short -- the very thing exact=True was set to prevent. Reconciling that properly means post-filtering the whole matched set in Python. The guard was partial regardless: content file hits were never checked, and unpublishing leaves content files in Qdrant by design. The removal path converges on its own -- inline delete, the retrying queued task behind it, and the sync task reconciling against the row -- so trust it. Also drops the points via remove_points_matching_params, keyed on the readable_id already in hand, rather than remove_embeddings, which re-serializes the resource only to derive the same filter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): re-attempt a failed reconciliation Both sync tasks undo themselves when the row turns out to be unpublished by the time they have written, and neither undo was retried on failure. In the news task the undo raises inside the task's own try, so the whole task retries -- and the retry took the "not published" path, which returned without touching the entry the previous attempt had created. That path now removes any entry instead of skipping, which is what makes the retry effective; deleting by guid is a no-op when there is nothing there, so the ordinary "queued, then unpublished" case is unchanged. The learning resource task does not retry at all, so a failed undo left the resource published and indexed with nothing behind it. It now hands the undo to the task that does retry, whose republish guard makes a late run safe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): do not fail an unpublish over the index work The inline removal caught only DatabaseError, but it runs the search and vector hooks inline, which fail in other ways -- an unreachable broker escapes try_with_retry_as_task, whose own fallback is an unguarded .delay(). That reached the client as a 500 after both rows had already committed as unpublished, and a retried request fires no hooks at all, since perform_update keys them off the published->unpublished transition. The deindex and the Qdrant removal were simply lost. Any failure now hands off to the retrying task, which redoes the removal in full. Queueing can fail too, for the same broker reason, so that is logged and the indexes are left to the next reindex rather than failing an unpublish whose own rows are already correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): persist the SEO title and description The drawer has collected both for a while with nowhere to put them. `WebsiteContent` now stores them, blank by default, and the serializer round-trips them for either content type -- they are what search engines and link previews show in place of the title and the opening of the content, which is the editor's call on news as much as on an article. The settings write carries them for both types. It previously bailed out when the drawer reported no topics, which is the news case, so news would have kept losing them; `topics` is still left out of that patch rather than sent as `[]`. Lengths are storage limits, not the guidance on how long a title or description ought to be: truncating the editor's typing would lose it. Nothing renders these into the page head yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): close the drawer's way past the topics rule Gating the save buttons left the drawer itself open: it is the one place a selection can be taken away, so removing every topic and saving straight from there PATCHed `topics: []` -- an article, published one included, left without the topics that put it on a topic page. Saving is now refused while a required selection is empty, which the section already explains, rather than silently ignoring the removal the editor can see on screen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): catch any failed undo, not just a database one The reconciliation caught `DatabaseError` alone, but the undo it guards runs the search and vector hooks inline, which fail in other ways -- an unreachable broker escapes `try_with_retry_as_task`, whose own fallback is an unguarded `.delay()`. This task does not retry, so that raised straight out, leaving the resource unpublished in the database and still in the index with nothing queued to remove it. Any failure now hands the undo to the retrying task, and queueing is itself guarded, since the broker is what may have failed. The same shape as the plugin's inline removal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): refuse an empty topic selection only once published The drawer's gate arrived with the stricter rule it was written for, where a draft could not be saved without topics either. Autosave changed that: a draft writes itself and cannot stop to ask, so it may sit without topics and is stopped at publishing instead. Left as it was, an editor could not remove a topic from a draft at all. So the two rules are now separate props. `topicsRequired` still says topics are wanted before publishing, which is what tells an editor why the drawer opened on them; `topicsMayNotBeEmptied` refuses the save, and the editor passes it only for content that is already public. The copy follows whichever rule is speaking rather than claiming a draft cannot be saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(website-content): the settings patch carries the SEO fields too The draft-clearing assertion came over with the scoping fix, where the patch was topics alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): a held-back draft save still happens without topics Pressing Save as Draft on a topicless article opens the drawer, and saving it from there resumed only when a topic had been picked. Since the drawer now lets a draft be saved with none, the other path dropped the held-back save and sent the settings alone -- losing the title or body edit that asked for it, with the drawer closed and the press forgotten. A draft now resumes either way; a publish still waits for a topic. Also brings the drawer's own documentation up to date: the SEO values are no longer local-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Cap the SEO title at what the server stores `WebsiteContent.seo_title` is a `CharField(max_length=255)`, and the drawer's save is fired and forgotten -- a rejected PATCH reaches the editor only as a generic banner, with nothing to say which field was too long. The field enforces the limit itself, states it, and counts against it, following `RefundRequestDialog`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Show MIT's SEO budgets in the settings drawer MIT's SEO rules ask for a title tag of 50-60 characters and a description of 160 -- 120 on a phone, so the important part goes first. The drawer was showing 255, the varchar width, and nothing at all for the description. Both are now shown as budgets with a counter that flags going over, not as limits: the numbers stand in for pixel widths that characters only approximate, and a tag a little long is truncated by the search engine rather than rejected. The one hard stop stays at 255, which really does fail. The title's budget is the tag's less the " | <site name>" that `standardizeMetadata` appends, since the guidance is about what reaches search results -- 48 characters against MIT Learn's name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ahtesham Quraish <ahtesham.quraish@arbisoft.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…page head (#3996) * feat(website-content): unpublish published items from the listing card Adds the three-dot menu from the design to published article and news listing cards, with Unpublish as its single action, and drops the Unpublish button from the content detail toolbar. The menu renders only for users who can edit content, so no callsite can leak it. Draft cards link to the editor, which is the only page they have. Unpublishing now takes effect within the request: the news feed entry and the article's LearningResource -- published flag and Qdrant points -- go before the response, so the listing's refetch and vector search stop serving what was just taken down. The view cache is cleared after them, so nothing can re-cache the stale listing, and vector search hydrates only published resources. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): reconcile a sync that overtakes an unpublish Both sync tasks check `is_published` with a read and then write, and unpublishing now runs in the request, so it can land between the two with nothing queued behind it to notice: the sync would restore a published, indexed resource -- and the news feed entry -- for content that is no longer public. Whoever writes last reconciles, so each sync task re-reads the row after writing and undoes itself if the item has since been unpublished. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(vector-search): check publication on the payload path too The published-row filter only covered database hydration, which the kill switch turns on; the default path answers from the Qdrant payloads, where a delete that is late or has failed outright kept serving an unpublished article. A payload cannot report this itself -- unpublishing deletes the point rather than rewriting it, so a stale payload still reads as published and no Qdrant-side filter can catch it. The payload path now costs one indexed lookup, asked negatively: only rows the database reports as unpublished are dropped, so a point with no row here -- a snapshot loaded from another system -- is still returned. Covered at the view level on the shipped default, not just in the hit builder. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(api): assert the patch hook invalidates the listings The page tests only assert that the PATCH was sent, so dropping the list invalidation still passed while a card unpublished from the listing stayed in the cache -- and on screen. The hook's own test now pins all three keys it invalidates, the detail retrieve one having been uncovered in the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): drop the topics section for news settings Only an article is projected into a LearningResource, so only there do topics put the content on a topic page. The news drawer keeps its SEO section and no longer fetches the topic list at all. The drawer stays presentational -- the caller decides with `showTopics` -- and reports no topics rather than an empty selection when the section is hidden, so saving news settings cannot empty a selection the editor was never shown, or re-run the publish plugins for nothing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): require topics to save an article An article's topics are what put it on a topic page, so saving one without them now opens the settings drawer instead -- for a draft save as well as a publish -- and the section says why it opened. News has no topics section at all, so nothing is required of it. The held-back press resumes once a topic is picked, carrying the new selection into the content write rather than PATCHing topics separately, and a publish still confirms as it would have. Closing the drawer abandons it, so a press the editor walked away from cannot fire the next time topics happen to be saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(website-content): pin that the unpublish hooks run in autocommit The hooks call out to Qdrant synchronously and swallow a DatabaseError to fall back to a queued task. Both are safe only outside a transaction: inside one, the call would hold it open across network I/O, and the swallowed error would poison it -- the fallback would never be queued and the next query would raise TransactionManagementError. Nothing asks for a transaction today (no ATOMIC_REQUESTS, no atomic in the write path), which two review passes have now assumed otherwise, so assert it rather than leave it to be re-derived. Verified to fail when ATOMIC_REQUESTS is switched on. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * revert(vector-search): drop the read-path publication filter Filtering hits against the database broke the counts beside them: on the no-query path `total` comes from Qdrant's own exact count() and the facets from facet(), neither of which knows about a Postgres filter, so totals and doc_counts over-reported and the last page came back short -- the very thing exact=True was set to prevent. Reconciling that properly means post-filtering the whole matched set in Python. The guard was partial regardless: content file hits were never checked, and unpublishing leaves content files in Qdrant by design. The removal path converges on its own -- inline delete, the retrying queued task behind it, and the sync task reconciling against the row -- so trust it. Also drops the points via remove_points_matching_params, keyed on the readable_id already in hand, rather than remove_embeddings, which re-serializes the resource only to derive the same filter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): re-attempt a failed reconciliation Both sync tasks undo themselves when the row turns out to be unpublished by the time they have written, and neither undo was retried on failure. In the news task the undo raises inside the task's own try, so the whole task retries -- and the retry took the "not published" path, which returned without touching the entry the previous attempt had created. That path now removes any entry instead of skipping, which is what makes the retry effective; deleting by guid is a no-op when there is nothing there, so the ordinary "queued, then unpublished" case is unchanged. The learning resource task does not retry at all, so a failed undo left the resource published and indexed with nothing behind it. It now hands the undo to the task that does retry, whose republish guard makes a late run safe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): do not fail an unpublish over the index work The inline removal caught only DatabaseError, but it runs the search and vector hooks inline, which fail in other ways -- an unreachable broker escapes try_with_retry_as_task, whose own fallback is an unguarded .delay(). That reached the client as a 500 after both rows had already committed as unpublished, and a retried request fires no hooks at all, since perform_update keys them off the published->unpublished transition. The deindex and the Qdrant removal were simply lost. Any failure now hands off to the retrying task, which redoes the removal in full. Queueing can fail too, for the same broker reason, so that is logged and the indexes are left to the next reindex rather than failing an unpublish whose own rows are already correct. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(website-content): persist the SEO title and description The drawer has collected both for a while with nowhere to put them. `WebsiteContent` now stores them, blank by default, and the serializer round-trips them for either content type -- they are what search engines and link previews show in place of the title and the opening of the content, which is the editor's call on news as much as on an article. The settings write carries them for both types. It previously bailed out when the drawer reported no topics, which is the news case, so news would have kept losing them; `topics` is still left out of that patch rather than sent as `[]`. Lengths are storage limits, not the guidance on how long a title or description ought to be: truncating the editor's typing would lose it. Nothing renders these into the page head yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): close the drawer's way past the topics rule Gating the save buttons left the drawer itself open: it is the one place a selection can be taken away, so removing every topic and saving straight from there PATCHed `topics: []` -- an article, published one included, left without the topics that put it on a topic page. Saving is now refused while a required selection is empty, which the section already explains, rather than silently ignoring the removal the editor can see on screen. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): catch any failed undo, not just a database one The reconciliation caught `DatabaseError` alone, but the undo it guards runs the search and vector hooks inline, which fail in other ways -- an unreachable broker escapes `try_with_retry_as_task`, whose own fallback is an unguarded `.delay()`. This task does not retry, so that raised straight out, leaving the resource unpublished in the database and still in the index with nothing queued to remove it. Any failure now hands the undo to the retrying task, and queueing is itself guarded, since the broker is what may have failed. The same shape as the plugin's inline removal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): refuse an empty topic selection only once published The drawer's gate arrived with the stricter rule it was written for, where a draft could not be saved without topics either. Autosave changed that: a draft writes itself and cannot stop to ask, so it may sit without topics and is stopped at publishing instead. Left as it was, an editor could not remove a topic from a draft at all. So the two rules are now separate props. `topicsRequired` still says topics are wanted before publishing, which is what tells an editor why the drawer opened on them; `topicsMayNotBeEmptied` refuses the save, and the editor passes it only for content that is already public. The copy follows whichever rule is speaking rather than claiming a draft cannot be saved. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test(website-content): the settings patch carries the SEO fields too The draft-clearing assertion came over with the scoping fix, where the patch was topics alone. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(website-content): a held-back draft save still happens without topics Pressing Save as Draft on a topicless article opens the drawer, and saving it from there resumed only when a topic had been picked. Since the drawer now lets a draft be saved with none, the other path dropped the held-back save and sent the settings alone -- losing the title or body edit that asked for it, with the drawer closed and the press forgotten. A draft now resumes either way; a publish still waits for a topic. Also brings the drawer's own documentation up to date: the SEO values are no longer local-only. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Cap the SEO title at what the server stores `WebsiteContent.seo_title` is a `CharField(max_length=255)`, and the drawer's save is fired and forgotten -- a rejected PATCH reaches the editor only as a generic banner, with nothing to say which field was too long. The field enforces the limit itself, states it, and counts against it, following `RefundRequestDialog`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Show MIT's SEO budgets in the settings drawer MIT's SEO rules ask for a title tag of 50-60 characters and a description of 160 -- 120 on a phone, so the important part goes first. The drawer was showing 255, the varchar width, and nothing at all for the description. Both are now shown as budgets with a counter that flags going over, not as limits: the numbers stand in for pixel widths that characters only approximate, and a tag a little long is truncated by the search engine rather than rejected. The one hard stop stays at 255, which really does fail. The title's budget is the tag's less the " | <site name>" that `standardizeMetadata` appends, since the guidance is about what reaches search results -- 48 characters against MIT Learn's name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Render the SEO title and description into the page head Stored on the row these fields did nothing: it is `<title>` and `<meta name="description">` in the server's response that a crawler reads and a search result shows. Both detail routes now prefer them, falling back to the content's title and the opening of its body as before. Chosen on emptiness rather than on null -- the serializer defaults both to blank, so `??` would let `""` through and emit an empty title. The preference lives in one helper so the two routes cannot drift. `standardizeMetadata` feeds the same values into OpenGraph and the Twitter card, so these also drive link previews, not only search. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ahtesham Quraish <ahtesham.quraish@arbisoft.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
OpenAPI Changes44 changes: 0 error, 0 warning, 44 info Unexpected changes? Ensure your branch is up-to-date with |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ahtesham Quraish
Sar
Zaman Afzal
Dan Subak
Alex H
Matt Bertrand
Anastasia Beglova