Skip to content

fix: POST /v1/user/subscriptions overwrites current subscriptions - #1791

Draft
davidgamez wants to merge 1 commit into
mainfrom
feat/subscription-feed-unique-per-type
Draft

fix: POST /v1/user/subscriptions overwrites current subscriptions#1791
davidgamez wants to merge 1 commit into
mainfrom
feat/subscription-feed-unique-per-type

Conversation

@davidgamez

Copy link
Copy Markdown
Member

Summary:

This pull request significantly changes how feed-scoped notification subscriptions are handled in the user service. Previously, creating a feed-scoped subscription would replace the set of feeds for an existing subscription; now, each create call generates a new subscription, and each feed can only be targeted by one subscription per user and notification type. Attempts to subscribe a feed already covered by an existing subscription are rejected with a 400 error, both via an application-level pre-check and a database constraint. The tests and documentation have been updated to reflect this new behavior.

Feed-scoped subscription logic changes:

  • Creating a feed-scoped subscription now always creates a new subscription row; it does not replace the feed set of an existing subscription. Each feed can only be targeted by one subscription per user and notification type, and attempts to double-subscribe a feed are rejected with a 400 error. [1] [2] [3] [4]
  • Added an application-level check (_feeds_already_subscribed) to preemptively reject requests targeting feeds already subscribed for the same notification type, and mapped database uniqueness violations (from concurrent creates) to a 400 error instead of a 500. [1] [2] [3]

Test updates and new tests:

  • Updated and expanded unit tests to verify the new behavior: each create call makes a new subscription, duplicate feed subscriptions are rejected, the database constraint is enforced, and the rule is scoped per notification type. [1] [2] [3] [4]
  • Updated mock session setup in tests to support the new uniqueness query logic.
  • Added tests for application-level and database-level uniqueness enforcement, including proper error mapping and scoping.

Documentation updates:

  • Updated docstrings and comments to reflect the new feed-scoped subscription semantics and error handling. [1] [2]

Expected behavior:

Explain and/or show screenshots for how you expect the pull request to work in your testing (in case other devices exhibit different behavior).

Testing tips:

Provide tips, procedures and sample files on how to test the feature.
Testers are invited to follow the tips AND to try anything they deem relevant outside the bounds of the testing tips.

Please make sure these boxes are checked before submitting your pull request - thanks!

  • Run the unit tests with ./scripts/api-tests.sh to make sure you didn't break anything
  • Add or update any needed documentation to the repo
  • Format the title like "feat: [new feature short description]". Title must follow the Conventional Commit Specification(https://www.conventionalcommits.org/en/v1.0.0/).
  • Linked all relevant issues
  • Include screenshot(s) showing how this pull request works and fixes the issue(s)

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.

2 participants