Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): delete related notifications when deleting identifier #971

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

iFergal
Copy link
Collaborator

@iFergal iFergal commented Feb 21, 2025

Description

We generally cannot act on notifications that relate to deleted identifiers, and we shouldn't anyway. #969 skips any incoming notifications related to deleted identifiers, and handles long running operations that complete which are related to deleted identifiers.

This PR will delete all notification records that are related to an identifier being deleted (and the local group member too, if applicable). This way the user cannot get into an error state acting on old notifications. Also included is a small cleanup in general.

Finally, I also decided we should sync deleted identifiers locally. We soft delete identifiers for historical reasons, so they should be re-synced because other areas of the code assumes the identifier metadata record might exist. We can refactor in the future to fully delete instead of soft deleting and handle the idempotency.

For example, if you delete a group and the local group member identifier has been marked as deleted on the cloud, but fails from there on/crashes, and then you recover on a new device, if you try to delete the group again it will fail as it assumes there's a local group member identifier record in the local DB. This fixes that edge case.

Checklist before requesting a review

Issue ticket number and link

  • This PR has a valid ticket number or issue: [link]

Testing & Validation

  • This PR has been tested/validated in IOS, Android and browser.
  • The code has been tested locally with test coverage match expectations.
  • Added new Unit/Component testing (if relevant).

@iFergal iFergal self-assigned this Feb 21, 2025
@iFergal iFergal changed the base branch from main to feat/DTIS-1446-General-notification-re-process-idempotency February 21, 2025 19:18
@iFergal iFergal changed the title Feat/dtis 1914 delete related notifications when deleting identifier feat(core): delete related notifications when deleting identifier Feb 21, 2025
@iFergal iFergal marked this pull request as ready for review February 21, 2025 19:20
@iFergal iFergal requested a review from jimcase February 21, 2025 19:23
Base automatically changed from feat/DTIS-1446-General-notification-re-process-idempotency to main February 24, 2025 12:47
…-related-notifications-when-deleting-identifier
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