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.

Sign up for GitHub

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

Jump to bottom

Add Bookmark Resetting side effect to DEFAULT_SYNC_DAYS_SETTING updation #518

Merged
merged 1 commit into from
Aug 12, 2024
Merged

Add Bookmark Resetting side effect to DEFAULT_SYNC_DAYS_SETTING updation #518

merged 1 commit into from
Aug 12, 2024

Conversation

samad-yar-khan
Copy link
Contributor

Linked Issue(s)

#500

Acceptance Criteria fulfillment

  • Update BookmarkService.reset_bookmarks in DEFAULT_SYNC_DAYS setter to reset all bookmarks if default sync days is update or increase

Proposed changes (including videos or screenshots)

  • Add _handle_settings_update_side_effect method in settings func to handle any sideeffect for setting updations. -> not recommended unless necessary. This was done to avoid sharing of this context to bff.
  • Handle bookmark resetting for default sync days updation. If bookmark resetting fails, the setting change is reverted.

Further comments

@samad-yar-khan samad-yar-khan mentioned this pull request Aug 12, 2024
Closed
6 tasks
@samad-yar-khan samad-yar-khan merged commit 78eb7b6 into main Aug 12, 2024
3 checks passed
@samad-yar-khan samad-yar-khan deleted the defaullt_sync_days_side_effect branch August 12, 2024 13:18
@samad-yar-khan samad-yar-khan mentioned this pull request Aug 12, 2024
Merged
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adnanhashmi09 adnanhashmi09 adnanhashmi09 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@samad-yar-khan @adnanhashmi09