fix(viewer): skip deleted/deactivated asset immediately#2875
Merged
Conversation
Previously a delete or deactivate on the currently-displayed asset only took effect after its scheduled duration elapsed — a 1-hour image kept showing for the rest of the hour. The viewer's reload handler now also checks whether the displayed asset is still active and wakes asset_loop's wait() so it rotates on the next tick. Server API delete/update/reorder paths publish reload alongside the existing Django page-view paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sonar flagged the v1_2.update and v2.update bodies as duplicated once the issue #2430 viewer-publish was added on top of the already-near-identical active-asset bookkeeping. Extract that post-save block (refresh, reposition in the active list, save ordering, publish reload) into ``api.helpers.finalize_asset_update`` so the API views shrink to a single call and Sonar's clone detector no longer trips. Also fixes the mypy attr-defined errors on the new test_viewer.py tests by patching ``Asset.objects.filter`` via its string path instead of ``viewer.Asset.objects`` (which isn't an explicit export). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #2430 by ensuring the viewer can immediately advance when the currently displayed asset is deleted or becomes inactive, instead of waiting for the originally configured duration to elapse.
Changes:
- Viewer: extend the
reloadcommand handler to also signal a skip when the currently displayed asset is deleted/deactivated. - Server API: publish
reloadfrom asset-mutation endpoints (delete/update/reorder) so the viewer is notified promptly. - Tests: add viewer-side unit tests for the skip-on-reload behavior and server-side API tests asserting
reloadis published.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_viewer.py |
Adds unit tests covering reload handling + skip decision when current asset is deleted/deactivated. |
src/anthias_viewer/__init__.py |
Implements _handle_reload() and _skip_if_current_asset_inactive() and wires reload command to new handler. |
src/anthias_server/api/views/v2.py |
Publishes reload after asset update to wake viewer. |
src/anthias_server/api/views/v1.py |
Publishes reload after asset update to wake viewer. |
src/anthias_server/api/views/v1_2.py |
Publishes reload after asset update to wake viewer (adds import). |
src/anthias_server/api/views/mixins.py |
Publishes reload on delete and playlist reorder. |
src/anthias_server/api/tests/test_assets.py |
Adds API tests asserting reload is published on delete/update/reorder. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Addresses Copilot review feedback on #2875: - v1_1.AssetView.put now publishes reload too — issue #2430 affects v1_1 clients the same as v1/v1_2/v2 and there's no reason to leave it silent. Test extended to cover v1_1. - PlaylistOrderViewMixin no longer publishes reload: the comment claimed reordering can drop an asset from the active set, but save_active_assets_ordering only updates play_order — it can't deactivate anything. Removing the inaccurate hook (and its test). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Subsequent pushes on this PR didn't trigger the GitHub Actions workflows (only Sonar + CodeQL ran). Forcing a fresh push to wake up mypy / unit-tests / lint / openapi-schema workflows. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…set-immediately # Conflicts: # src/anthias_server/api/tests/test_assets.py
|
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
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.
Issues Fixed
Fixes #2430
Description
Previously, deleting or deactivating an asset while it was on screen had no effect until the originally-scheduled
durationelapsed — a 1-hour image kept showing for the rest of the hour even after the operator removed it.Viewer (
src/anthias_viewer/__init__.py): thereloadhandler now also checks whether the currently-displayed asset is still in the active set. If not, it setsskip_eventsoasset_loop'swait()returns early and the loop rotates on the next tick. Playlist refresh itself still flows through the existingget_db_mtimeshort-circuit insideget_next_asseton the main thread; the subscriber thread only signals.Server: API endpoints that mutate an asset's active state now publish
reloadalongside the Django page views that already did:DeleteAssetViewMixin.delete(shared across all API versions)AssetViewV1.put,AssetViewV1_1.put,AssetViewV1_2.update,AssetViewV2.updateThe shared post-save bookkeeping for v1_2/v2 (active-asset repositioning + viewer publish) was extracted into
api.helpers.finalize_asset_updateto keep the view bodies short. Playlist reordering is not wired to publishreload—save_active_assets_orderingonly updatesplay_order, it can't deactivate an asset, so there's no scenario where the currently-displayed asset becomes inactive from a reorder alone.Checklist