Land Slices 11-13 into main (stacked-PR base gap) - #36
Merged
Conversation
…ons/{id}
Implements Slice 11: authenticated user can list their own subscriptions
and fetch a single one. GetById returns 404 (not 403) for another user's
record, to avoid leaking existence.
- ISubscriptionRepository.GetAllForUserAsync/GetByIdAsync: query directly
into SubscriptionDto via a left-joined LINQ projection (categories,
payment_sources, subscription_catalog) rather than loading entities and
mapping separately - one SQL round trip, no N+1
- SubscriptionDto: extended with CategoryName, PaymentSourceLabel,
CatalogLogoUrl (all nullable - the joins are left joins since category/
payment_source/catalog_id are all optional on user_subscriptions)
- SubscriptionsController: added GetAll/GetById; Create now re-fetches
through GetByIdAsync after insert so its 201 response has the same
resolved shape as the read endpoints, instead of a bare unresolved DTO
- Extracted the repeated Guid.Parse(User.FindFirstValue(...)) into a
private GetUserId() helper, used by all three actions now
Tests cover the 3 required cases plus unauthenticated-list, nonexistent-id,
and a dedicated test proving the category/payment-source/catalog joins
actually resolve names and the logo URL end to end.
Closes #12
Implements Slice 12: authenticated owner can update their subscription's editable fields; 404 for another user's record; 400 on invalid payload. Reuses CreateSubscriptionRequest (and its validator) as the update body type rather than introducing a near-duplicate UpdateSubscriptionRequest - the editable field set is identical, so a second type would just be the first one copy-pasted. Directly satisfies the issue's "extract shared rules if duplicated" by not duplicating the type in the first place. ISubscriptionRepository.UpdateAsync loads the tracked entity scoped to (id, user_id), applies the request fields, saves, then re-queries through the same joined DTO projection GetByIdAsync uses so the response has resolved category/payment-source/catalog fields like every other read. Tests cover the 3 required cases plus a nonexistent-id 404 case. Closes #13
Implements Slice 13: authenticated owner can delete their subscription; 404 for another user's record or an already-deleted one (not 500). ISubscriptionRepository.DeleteAsync loads the tracked entity scoped to (id, user_id) - same ownership pattern as UpdateAsync - and removes it if found. This completes subscriptions CRUD (create/read/update/delete). Tests cover the 3 required cases plus an attacker-can't-affect-owner's- record proof and unauthenticated-request rejection. Closes #14
Slices 11-13: Subscriptions — List, Get, Update, Delete
3 tasks
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
1 participant
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.
What
Corrective PR. PR #35 (Slices 11-13: List/Get/Update/Delete) was based on and merged into the
slice-10-subscriptions-createbranch (stacked-PR pattern), notmain. PR #34 (Slice 10: Create) had already merged intomainseparately before that, so there was no open PR left to carry #35's commits forward intomain- they landed onslice-10-subscriptions-createonly.Net effect: issues #12, #13, #14 show closed (their branches said "Closes #N" and did merge - just into the wrong ultimate target), but
GET /api/v1/subscriptions,GET /api/v1/subscriptions/{id},PUT /api/v1/subscriptions/{id}, andDELETE /api/v1/subscriptions/{id}do not exist onmainright now.Changes
None beyond what #35 already contained and already reviewed - this PR is
main<-slice-10-subscriptions-create, bringing in exactly the 3 commitsmainis missing:2a3af85feat: add GET /api/v1/subscriptions (list) and GET /api/v1/subscriptions/{id}2d774a8feat: add PUT /api/v1/subscriptions/{id} (update)12dc89bfeat: add DELETE /api/v1/subscriptions/{id}Verification
Already verified in #35 (52/52 tests passing, 0 warnings) - no new code, just landing it in the right place. Re-ran locally after
git fetchto confirm the branch state before opening this.