Allows creating a bookmark with the `for_topic` flag introduced in d1d2298a4c set to true. This happens when clicking on the Bookmark button in the topic footer when no other posts are bookmarked. In a later PR, when clicking on these topic-level bookmarks the user will be taken to the last unread post in the topic, not the OP. Only the OP can have a topic level bookmark, and users can also make a post-level bookmark on the OP of the topic.
I had to do some pretty heavy refactors because most of the bookmark code in the JS topics controller was centred around instances of Post JS models, but the topic level bookmark is not centred around a post. Some refactors were just for readability as well.
Also removes some missed reminderType code from the purge in 41e19adb0d
We don't need no stinkin' denormalization! This commit ignores
the topic_id column on bookmarks, to be deleted at a later date.
We don't really need this column and it's better to rely on the
post.topic_id as the canonical topic_id for bookmarks, then we
don't need to remember to update both columns if the bookmarked
post moves to another topic.
In order to include the new/unread count in the browse more message
under suggested topics, a couple of technical changes have to be made.
1. `PrivateMessageTopicTrackingState` is now auto-injected which is
similar to how it is done for `TopicTrackingState`. This is done so
we don't have to attempt to pass the `PrivateMessageTopicTrackingState`
object multiple levels down into the suggested-topics component. While
the object is auto-injected, we only fetch the initial state and start
tracking when the relevant private messages routes has been hit and only
when a private message's suggested topics is loaded. This is
done as we do not want to add the extra overhead of fetching the inital
state to all page loads but instead wait till the private messages
routes are hit.
2. Previously, we would stop tracking once the `user-private-messages`
route has been deactivated. However, that is not ideal since
navigating out of the route and back means we send an API call to the
server each time. Since `PrivateMessageTopicTrackingState` is kept in
sync cheaply via messageBus, we can just continue to track the state
even if the user has navigated away from the relevant stages.
The date shown in topic timeline was one day later if the post at that
position was made near midnight. This happened because the days number
was rounded down.
This TODO comment has existed for 8 years. Sort must be working just
fine or we would have prioritized fixing it.
Removing this comment as a tiny step toward keeping our codebase nice
and tidy.
We changed (https://github.com/discourse/discourse/pull/13407) behaviour of the topic level bookmark button recently. That PR made the button be opening the edit bookmark modal when there is only one bookmark on the topic instead of just removing that bookmark as it was before.
This PR fixes the next problems that weren't taken into account in the previous PR:
1. Everything should work fine even on very big topics when a bookmarked post is unloaded from the post stream. I've added code that loads the post we need and makes everything work as expected
2. When at least one bookmark on the topic has a reminder, we should always be showing the icon with a clock on the topic level bookmark button
3. We should show correct tooltips for the topic level bookmark button
In #12841, we started setting the ReviewableQueuedPost's target and topic after approving it instead of storing them in the payload. As a result, the reviewable_counts query started to include queued posts.
When a category is set to require approval, every post has an associated reviewable. Pointing that each post has an associated queued post is not necessary in this case, so I added a WHERE clause to skip them.
This fixes an N+1 queires problem when generating `Post#image_url` which
requires the upload to be loaded.
Follow-up to 141f16eb6b6f3f35a10dc1133e66507dd5931fb0
* FIX: flaky specs after topic view custom filters
When ensuring TopicView class variables return to the original state it should use empty Hash instead of empty Array. That
https://github.com/discourse/discourse/blob/master/lib/topic_view.rb#L60
* FIX: convert to string for topic view custom filter
* FEATURE - allow category group moderators to delete topics
* Allow individual posts to be deleted
* DEV - refactor for new `can_moderate_topic?` method
On forums with a large amount of posts when a user had a bookmark in the topic, PostgreSQL was using an inefficient query plan to fetch the first post of the topic. When running this ActiveRecord query:
```
topic.posts.with_deleted.where(post_number: 1).first
```
The following query plan was produced:
```
Limit (cost=0.43..583.49 rows=1 width=891) (actual time=3850.515..3850.515 rows=1 loops=1)
-> Index Scan using posts_pkey on posts (cost=0.43..391231.51 rows=671 width=891) (actual time=3850.514..3850.514
rows=1 loops=1)
Filter: ((topic_id = 160918) AND (post_number = 1))
Rows Removed by Filter: 2274520
Planning time: 0.200 ms
Execution time: 3850.559 ms
(6 rows)
```
The issue here is the combination of ORDER BY and LIMIT causing the ineficcient Index Scan using posts_pkey on posts to be used. When we correct the AR call to this:
```
topic.posts.with_deleted.find_by(post_number: 1)
```
We end up with a query that still has a LIMIT but no ORDER BY, which in turn creates a much more efficient query plan:
```
Limit (cost=0.43..1.44 rows=1 width=891) (actual time=0.033..0.034 rows=1 loops=1)
-> Index Scan using index_posts_on_topic_id_and_post_number on posts (cost=0.43..678.82 rows=671 width=891) (actua
l time=0.033..0.033 rows=1 loops=1)
Index Cond: ((topic_id = 160918) AND (post_number = 1))
Planning time: 0.167 ms
Execution time: 0.072 ms
(5 rows)
```
This query plan uses the correct index, `Index Scan using index_posts_on_topic_id_and_post_number on posts`. Note that this is only a problem on forums with a larger amount of posts; tiny forums would not notice the difference. On large forums a query for a topic that takes 1s without a bookmark can take 8-30 seconds, and even end up with 502 errors from nginx.
This PR ensures that new bookmarks cannot be created for deleted posts and topics, and also makes sure that if a bookmark was created and then the topic deleted that the show topic page does not error from trying to retrieve the bookmark reminder at.
When a tab is open but left unattended for a while, the red, green, and blue
pills tend to go out of sync.
So whevener we open the notifications menu, we sync up the notification count
(eg. blue and green pills) with the server.
However, the reviewable count (eg. the red pill) is not a notification and
is located in the hamburger menu. This commit adds a new route on the server
side to retrieve the reviewable count for the current user and a ping
(refreshReviewableCount) from the client side to sync the reviewable count
whenever they open the hamburger menu.
REFACTOR: I also refactored the hamburger-menu widget code to prevent repetitive uses
of "this.".
PERF: I improved the performance of the 'notify_reviewable' job by doing only 1 query
to the database to retrieve all the pending reviewables and then tallying based on the
various rights.
The assign plugin is one of two situations where a post can be both a whisper and a small-action. Check the action_code field to filter out small-actions.
On large topics, the cost of sending the entire post ID list back over to the database is signficant. Just have the DB recalculate the list of visible posts instead.
Instead of loading all of the user bookmarks using all the post IDs in a topic, load all the bookmarks for a user using the topic ID. This eliminates a costly WHERE ID IN query.
Previously, while generating the topic page's canoncial url we used the current post number. It will create invalid canonical path if the topic has whsiper posts. Now we only taking the visible posts for current page index calculation.
When plugin is hooking into TopicView joining other tables, it may fail because `created_at` is potentially available on 2 tables. Therefore we should explicitly define which `created_at` we want.
* Show the correct bookmark with clock icon when topic-level bookmark reminder time is set and show the time of the reminder in the title on hover.
* Add a new bookmark lib and reminder time formatting function to show time with today/tomorrow shorthand for readability. E.g. tomorrow at 8:00am instead of Apr 16 2020 at 8:00am. This only applies to today + tomorrow, future dates are still treated the same.
If the feature is enabled, staff members can construct a URL and publish a
topic for others to browse without the regular Discourse chrome.
This is useful if you want to use Discourse like a CMS and publish
topics as articles, which can then be embedded into other systems.
* This PR changes the user activity bookmarks stream to show a new list of bookmarks based on the Bookmark record.
* If a bookmark has a name or reminder it will be shown as metadata above the topic title in the list
* The categories, tags, topic status, and assigned show for each bookmarked post based on the post topic
* Bookmarks can be deleted from the [...] menu in the list
* As well as this, the list of bookmarks from the quick access panel is now drawn from the Bookmarks table for a user:
* All of this new functionality is gated behind the enable_bookmarks_with_reminders site setting
The /bookmarks/ route now redirects directly to /user/:username/activity/bookmarks-with-reminders
* The structure of the Ember for the list of bookmarks is not ideal, this is an MVP PR so we can start testing this functionality internally. There is a little repeated code from topic.js.es6. There is an ongoing effort to start standardizing these lists that will be addressed in future PRs.
* This PR also fixes issues with feature detection for at_desktop bookmark reminders
People rarely want to have their avatars show up as the preview image on social media platforms. Instead, we should fall back to the site opengraph image.
Note: All of this functionality is hidden behind a hidden, default false, site setting called `enable_bookmarks_with_reminders`. Also, any feedback on Ember code would be greatly appreciated!
This is part 1 of the bookmark improvements. The next PR will address the backend logic to send reminder notifications for bookmarked posts to users. This PR adds the following functionality:
* We are adding a new `bookmarks` table and `Bookmark` model to make the bookmarks a first-class citizen and to allow attaching reminders to them.
* Posts now have a new button in their actions menu that has the icon of an actual book
* Clicking the button opens the new bookmark modal.
* Both name and the reminder type are optional.
* If you close the modal without doing anything, the bookmark is saved with no reminder.
* If you click the Cancel button, no bookmark is saved at all.
* All of the reminder type tiles are dynamic and the times they show will be based on your user timezone set in your profile (this should already be set for you).
* If for some reason a user does not have their timezone set they will not be able to set a reminder, but they will still be able to create a bookmark.
* A bookmark can be deleted by clicking on the book icon again which will be red if the post is bookmarked.
This PR does NOT do anything to migrate or change existing bookmarks in the form of `PostActions`, the two features live side-by-side here. Also this does nothing to the topic bookmarking.
This bug was causing some unusual behavior when the last post is filtered (e.g. from an ignored user). In some situations this would cause suggested topics to be omitted from the payload.
The next_page specs have been updated to remove most of the stubs
Doing .pluck(:column).first is a very common pattern in Discourse and in
most cases, a limit cause isn't being added. Instead of adding a limit
clause to all these callsites, this commit adds two new methods to
ActiveRecord::Relation:
pluck_first, equivalent to limit(1).pluck(*columns).first
and pluck_first! which, like other finder methods, raises an exception
when no record is found
Zeitwerk simplifies working with dependencies in dev and makes it easier reloading class chains.
We no longer need to use Rails "require_dependency" anywhere and instead can just use standard
Ruby patterns to require files.
This is a far reaching change and we expect some followups here.
* Revert "Revert "FEATURE: Publish read state on group messages. (#7989) [Undo revert] (#8024)""
This reverts commit 36425eb9f04cfac7201632d648050cb43a035bc1.
* Fix: Show who read only if the attribute is enabled
* PERF: Precalculate the last post readed by a group member
* Use book-reader icon instear of far-eye
* FIX: update topic groups correctly
* DEV: Tidy up read indicator update on write
* Reenable: "FEATURE: Publish read state on group messages. (#7989)"
This reverts commit 67f5cc1ce8da8d6301db0ab6ebcd6fc497f28455.
* FIX: Read indicator only appears when the group setting is enabled
* Enable or disable read state based on group attribute
* When read state needs to be published, the minimum unread count is calculated in the topic query. This way, we can know if someone reads the last post
* The option can be enabled/disabled from the UI
* The read indicator will live-updated using message bus
* Show read indicator on every post
* The read indicator now shows read count and can be expanded to see user avatars
* Read count gets updated everytime someone reads a message
* Simplify topic-list read indicator logic
* Unsubscribe from message bus on willDestroyElement, removed unnecesarry values from post-menu, and added a comment to explain where does minimum_unread_count comes from
