Skip to content

Scope legacy video play history to its user - #396

Draft
MinasukiHikimuna wants to merge 4 commits into
mainfrom
signed/play-history-ownership
Draft

Scope legacy video play history to its user#396
MinasukiHikimuna wants to merge 4 commits into
mainfrom
signed/play-history-ownership

Conversation

@MinasukiHikimuna

Copy link
Copy Markdown
Collaborator

Summary

Draft design proposal — not currently intended for merge. This branch preserves the proposed implementation for scoping legacy video play history to its owning user. Play-history rows currently have no user relationship, so one member's delete or reset operation can remove another member's activity for the video.

The proposal adds nullable user ownership, scopes ordinary reads and mutations to the current principal, assigns imported history to the migration owner, cascades history when its user is deleted, and backfills existing rows to the active system owner when available. Privileged cleanup retains access to owned and legacy rows, while anonymous and share-link contexts cannot access user history.

The ownership migration is intentionally deferred until the canonical relationship between legacy play history and modern playback sessions is decided in #395. This draft remains available for implementation-level discussion and comparison; merging it now would commit to migration and compatibility behavior before that redesign decision.

The failing-first API regression records plays from two members and demonstrates the isolation gap. The independently mergeable like and aspect-rating coverage is being extracted to a separate PR; once that lands on main, this draft's effective diff should be limited to the deferred ownership proposal.

Linked issue

Tracks #393 without closing it.

Architectural follow-up: #395.

Type of change

  • Bug fix
  • New feature
  • Refactor / tech debt
  • Docs
  • Other:

AI usage

  • No AI was used for this PR.
  • AI was used for this PR.
    • Model(s): GPT-5.6 Sol
    • Where / how: Reconstructed the historical failing-test-first slice, adapted endpoint accounting and migration metadata to the sliced mainline, verified user isolation and legacy behavior, and drafted the issue and pull request handoff.
  • A human (me) has reviewed, understands, and takes full responsibility for every change here including that the design and architecture are sound. (required)

Testing done & evidence

  • Before the production fix, GivenTwoMembersPlayVideo_WhenOneDeletesAndResetsOwnHistory_ThenOtherMemberHistoryRemains failed because the second member's two history entries were removed.
  • dotnet build src/Cove.slnx -c Release --no-restore — passed with 0 errors and 2 pre-existing warnings.
  • dotnet test src/Cove.Tests/Cove.Tests.csproj -c Release --no-build --no-restore --logger "console;verbosity=normal" with configuration reload watchers disabled for the exhausted devbox inotify pool — 1688/1688 passed.
  • dotnet test src/Cove.ApiTests/Cove.ApiTests.csproj -c Release --no-build --no-restore --filter "FullyQualifiedName~VideoEngagementLifecycleApiTests|FullyQualifiedName~EndpointCoverageTests" --logger "console;verbosity=normal" — 5/5 passed.
  • node --test scripts/check-api-controller-coverage.test.mjs — 6/6 passed.
  • dotnet tool run dotnet-coverage -- collect --settings src/Cove.ApiTests/coverage.config --output artifacts/coverage/api-tests.cobertura.xml --output-format cobertura dotnet test src/Cove.ApiTests/Cove.ApiTests.csproj -c Release --no-restore --verbosity normal followed by node scripts/check-api-controller-coverage.mjs artifacts/coverage/api-tests.cobertura.xml — 308/308 passed; controller coverage reached 8,334/14,335 lines (58.137%) and the 8,108/14,069 ratchet passed.

Checklist

  • I have read and followed the Contribution Guide.
  • Builds and existing tests pass.
  • I added or updated tests where it makes sense.
  • I updated docs where needed.
  • This PR is focused and does not bundle unrelated changes.

Legacy video play, like, and rating routes were still outside exact endpoint coverage, and play history operations had no cross-user behavioral contract.

Add ten exact route claims and user-isolated lifecycle tests for play history, like history, and aspect ratings, advancing the ledger to 278 of 504 mapped endpoints. The play test intentionally remains red because one member's reset removes another member's history; the user-owned history model follows in a separate commit.
Associate play-history rows with the current user, scope reads and mutations consistently, preserve ownerless legacy history for system operation, and backfill existing rows to the active system owner when available.

Assign imported history to the migration owner, cascade user deletion, and cover user isolation, privileged cleanup, share-link denial, and legacy-system behavior.
Verify each play and like transition at the point it occurs, preserve cross-user and owner isolation, compare historical timestamps exactly, and document the legacy overall-rating response while asserting aspect persistence through fresh reads.
Ratcheting controller coverage to 8,108 of 14,069 eligible lines records the independently verified gain from the user-scoped play, like, and rating lifecycle slice.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant