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

FEATURE: Improving bookmarks part 2 -- Topic Bookmarking #8954

Merged

Conversation

@martin-brennan
Copy link
Contributor

martin-brennan commented Feb 13, 2020

Note: All the changes in this PR are gated between the enable_bookmarks_with_reminders setting. If it is off none of this new stuff will happen, and it will not affect current features.

UI Changes

If SiteSetting.enable_bookmarks_with_reminders is enabled:

  • Clicking "Bookmark" on a topic will create a new Bookmark record instead of a post + user action
  • Clicking "Clear Bookmarks" on a topic will delete all the new Bookmark records on a topic
  • The topic bookmark buttons control the post bookmark flags correctly and vice-versa
    Disabled selecting the "reminder type" for bookmarks in the UI because the backend functionality is not done yet (of sending users notifications etc.)

Other Changes

  • Added delete bookmark route (but no UI yet)
  • Added a rake task to sync the old PostAction bookmarks to the new Bookmark table, which can be run as many times as we want for a site (it will not create duplicates).
* we do not yet have the functionality to actually
  send the reminders so we don't want the users to see
  them yet
* also add minimal acceptance test fort the bookmark
  modal
* update existing bookmark record bookmark IDs
* add missing model relationships
* fix fabricator
@discoursebot

This comment has been minimized.

Copy link

discoursebot commented Feb 13, 2020

You've signed the CLA, martin-brennan. Thank you! This pull request is ready for review.

@martin-brennan martin-brennan changed the title FEATURE: Improving bookmarks part 2 -- Lists and Topics FEATURE: Improving bookmarks part 2 -- Topic Bookmarking Feb 13, 2020
@martin-brennan martin-brennan marked this pull request as ready for review Feb 13, 2020
@martin-brennan martin-brennan merged commit e1e74ab into master Feb 13, 2020
7 checks passed
7 checks passed
PLUGINS-BACKEND
Details
CORE-BACKEND
Details
PLUGINS-FRONTEND
Details
CORE-FRONTEND
Details
PLUGINS-LINT
Details
CORE-LINT
Details
license/cla Contributor License Agreement is signed.
Details
@martin-brennan martin-brennan deleted the feature/bookmarks-with-reminders-user-list-changes branch Feb 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.