Commit Graph

61 Commits

Author SHA1 Message Date
Martin Brennan
c6c633e041
FIX: Issues with incorrect unread and private message topic tracking state (#16474)
This commit fixes two issues at play. The first was introduced
in f6c852b (or maybe not introduced
but rather revealed). When a user posted a new message in a topic,
they received the unread topic tracking state MessageBus message,
and the Unread (X) indicator was incremented by one, because with the
aforementioned perf commit we "guess" the correct last read post
for the user, because we no longer calculate individual users' read
status there. This meant that every time a user posted in a topic
they tracked, the unread indicator was incremented. To get around
this, we can just exclude the user who created the post from the
target users of the unread state message.

The second issue was related to the private message topic tracking
state, and was somewhat similar. Whenever a user created a new private
message, the New (X) indicator was incremented, and could not be
cleared until the page was refreshed. To solve this, we just don't
update the topic state for the user when the new_topic tracking state
message comes through if the user who created the topic is the
same as the current user.

cf. https://meta.discourse.org/t/bottom-of-topic-shows-there-is-1-unread-remaining-when-there-are-actually-0-unread-topics-remaining/220817
2022-04-19 11:37:01 +10:00
Krzysztof Kotlarek
ea3a58d051
FIX: indirectly muted categories for topic-tracking-state (#16067)
Topics belonging to indirectly muted categories should be excluded from topic-tracking-state report.
2022-03-02 15:02:09 +11:00
David Taylor
c9dab6fd08
DEV: Automatically require 'rails_helper' in all specs (#16077)
It's very easy to forget to add `require 'rails_helper'` at the top of every core/plugin spec file, and omissions can cause some very confusing/sporadic errors.

By setting this flag in `.rspec`, we can remove the need for `require 'rails_helper'` entirely.
2022-03-01 17:50:50 +00:00
David Taylor
f6c852bf8e
PERF: Use a shared message for replies to tracked topics (#16022)
Previously we were publishing one messagebus message per user which was 'tracking' a topic. On large sites, this can easily be 1000+ messages. The important information in the message is common between all users, so we can manage with a single message on a shared channel, which will be much more efficient.

For user-specific values (notification_level and last_read_post_number), the JS app can infer values which are 'good enough'. Correct values will be loaded as soon as a topic-list containing the topic is visited.
2022-02-22 15:27:46 +00:00
Osama Sayegh
8e6988163f
FIX: Don't allow NULL values for notification_level in category_users (#15407) 2021-12-29 09:19:39 +11:00
Daniel Waterworth
5ff0b86b57
PERF: Make tests faster by prefabricating more things (#15392) 2021-12-22 11:09:43 -06:00
Krzysztof Kotlarek
0301b775a8
FIX: topic_tracking_state not erroring when missing user_stat (#14559)
publish_unread should not error when UserStat is missing
2021-10-11 13:20:55 +11:00
Alan Guo Xiang Tan
5de64b3630
FIX: Correct highest post number for read topic tracking state. (#14273) 2021-09-08 11:55:12 +08:00
Alan Guo Xiang Tan
f66007ec83
FEATURE: Display unread and new counts for messages. (#14059)
There are certain design decisions that were made in this commit.

Private messages implements its own version of topic tracking state because there are significant differences between regular and private_message topics. Regular topics have to track categories and tags while private messages do not. It is much easier to design the new topic tracking state if we maintain two different classes, instead of trying to mash this two worlds together.

One MessageBus channel per user and one MessageBus channel per group. This allows each user and each group to have their own channel backlog instead of having one global channel which requires the client to filter away unrelated messages.
2021-08-25 11:17:56 +08:00
Alan Guo Xiang Tan
aed65ec16d SECURITY: User's read state for topic is leaked to unauthorized clients.
A user's read state for a topic such as the last read post number and the notification level is exposed.
2021-08-12 12:16:45 +08:00
Alan Guo Xiang Tan
9a60c83535 FIX: TopicTrackingState.report not including unread for staff posts.
Follow-up to e15c86e8c5
2021-08-12 11:10:49 +08:00
Alan Guo Xiang Tan
016efeadf6
FEATURE: New and Unread messages for user personal messages. (#13603)
* FEATURE: New and Unread messages for user personal messages.

Co-authored-by: awesomerobot <kris.aubuchon@discourse.org>
2021-08-02 12:41:41 +08:00
Alan Guo Xiang Tan
37b8ce79c9
FEATURE: Add last visit indication to topic view page. (#13471)
This PR also removes grey old unread bubble from the topic badges by
dropping `TopicUser#highest_seen_post_number`.
2021-07-05 14:17:31 +08:00
Kris
83519e7f3a
FEATURE: Always show tag dropdown when tagging is enabled (#12209) 2021-03-10 21:13:15 -05:00
Krzysztof Kotlarek
f39e7fe81d
FEATURE: New way to dismiss new topics (#11927)
This is a try to simplify logic around dismiss new topics to have one solution to work in all places - dismiss all-new, dismiss new in a specific category or even in a specific tag.
2021-02-04 11:27:34 +11:00
Krzysztof Kotlarek
da2a61e36c
FIX: correct tracking when mute all categories (#11441)
Currently, we have a solution for muted topics. Basically, when a post is created first we send a `muted` message to users who muted that specific topic:

https://github.com/discourse/discourse/blob/master/app/models/topic_tracking_state.rb#L91

Later, topic tracking state filters if the topic is muted or not before update state:

https://github.com/discourse/discourse/blob/master/app/assets/javascripts/discourse/app/models/topic-tracking-state.js#L58:L67

That solution works quite well.

I wanted to extend it to handle `mute all categories by default` setting as well.

In that case, we should only inform the user about new topic/post when they explicitly want to.

If that setting is enabled, we would send "unmuted" message to a user who watches specific category, topic or tag. In all other cases, don't inform user about new topic as all categories are muted by default.

Meta: https://meta.discourse.org/t/threads-muted-by-mute-all-by-default-are-showing-up-as-new-but-not-visible/168324
2020-12-10 16:49:05 +11:00
Krzysztof Kotlarek
1f7f8ee72c
FIX: try to make topic_tracking_state_spec stable (#10419)
Sometime parallel spec if failing with error:
```
NoMethodError:
       undefined method `data' for nil:NilClass
     # ./spec/models/topic_tracking_state_spec.rb:339:in `block (4 levels) in <main>'
```

I have a theory that it might be related to instance variables in before block
2020-08-12 15:56:06 +10:00
Sam Saffron
a26b490047
FIX: present correct new/unread counts when filtered by tag
Previously we would incorrectly ignore tags.

This ensures tracking state is properly shipped to client if
show_filter_by_tag is enabled.
2020-06-11 16:47:57 +10:00
Sam Saffron
2acec4370b
FIX: muted tags removed topics with no tags from counts
We previously did not account for completely untagged topics when
looking at muted tags, this caused new/unread counts to be off if

1. You had muted tags
2. You had an unread/new topic
3. This topic had no tags
2020-05-29 12:59:47 +10:00
Sam Saffron
136835370c
FEATURE: optionally allow tags in topic tracking state
This feature allows certain plugins to output tag information
to topic tracking state, this allows per tag stats which can be
used by sidebars and other plugins that need per tag stats

Not enabled by default cause this would add cost to a critical
query
2020-05-29 12:59:47 +10:00
Krzysztof Kotlarek
37e93914fc
FIX: the muted message should be sent after edit (#9593)
Recently, we added feature that we are sending `/muted` to users who muted specific topic just before `/latest` so the client knows to ignore those messages - https://github.com/discourse/discourse/pull/9482

Same `/muted` message should be included when the post is edited
2020-05-01 08:33:57 +10:00
Krzysztof Kotlarek
52c1d7337e
FEATURE: don't display new/unread notification for muted topics (#9482)
* FEATURE: don't display new/unread notification for muted topics

Currently, even if user mute topic, when a new reply to that topic arrives, the user will get "See 1 new or updated topic" message. After clicking on that link, nothing is visible (because the topic is muted)

To solve that problem, we will send background message to all users who recently muted that topic that update is coming and they can ignore the next message about that topic.
2020-04-23 14:57:35 +10:00
Jarek Radosz
29b35aa64c
DEV: Improve flaky time-sensitive specs (#9141) 2020-03-10 22:13:17 +01:00
David Taylor
d1779346e8 FIX: topic_tracking_state when mute_all_categories_by_default is enabled 2020-01-06 18:22:42 +00:00
David Taylor
5df815c2ee
FIX: New/unread count after dismissing new topics in a regular category (#8659)
6e1fe22 introduced the possiblity for category_users to have a NULL notification_level, so that we can store `last_seen_at` dates without locking the notification level. At the time, this did not affect the topic-tracking-state query. However, the query changes in f434de2 introduced a slight change in behavior.

Previously, a subquery would look for a category_user with notification_level=mute. f434de2 refactored this to remove the subquery, and inverted some of the logic to suit.

The new query checked for `notification_level <> :muted`. If `notification_level` is NULL, this comparison will return NULL. In this scenario, notification_level=NULL means that we should fall back to the default tracking level (regular), and so we want the expression to resolve as true, not false. There was already a check for the existence of the category_users row, but it did not check for the existence of a NOT NULL notification_level.

This commit amends the expression so that the notification_level will only be compared if it is non-null.
2020-01-06 16:15:24 +00:00
Krzysztof Kotlarek
81c7d6a462
FIX: Muted tags are respected by TopicTrackingState (#8467)
When the tag is muted and topic contains that tag, we should not mark that message as NEW.

There are 3 possible settings which site admin can set.
remove_muted_tags_from_latest - always
It means that if the topic got at least one muted tag, we should not mark that topic as NEW

remove_muted_tags_from_latest - only muted
Similar to above, however, if at least one tag is not muted, the topic is marked as NEW

remove_muted_tags_from_latest - never
Basically, mute tag setting is ignored and all topics are set as NEW
2019-12-10 09:50:05 +11:00
Krzysztof Kotlarek
f434de2536
FIX: Tracking Topic State know about category_seen_at (#8351)
If category got last_seen_at is set TrackingTopicState should know about it and exclude those topics from marking them as new
2019-11-14 16:11:34 +11:00
Sam Saffron
a49e484519 DEV: correct flaky test testing message content
This test was expecting user id ordering which is not really required.
Order of users does not matter
2019-10-04 09:56:28 +10:00
Roman Rizzi
ebb389ef8a
UX: Read indicator improvements. (#8049)
* The read indicator now shows up when no member has read the last post of the topic (written by a non-member)
* The read indicator works on mobile and receives live updates from message bus
* The icon we display in the topic list was changed
* Added a title to the indicator to indicate its purpose when hovering over it
2019-08-29 12:03:43 -03:00
Roman Rizzi
7c741fa0d6
FEATURE: Publish read state on group messages. (Originally introduced in #7989) (#8025)
* Revert "Revert "FEATURE: Publish read state on group messages. (#7989) [Undo revert] (#8024)""

This reverts commit 36425eb9f0.

* 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
2019-08-27 09:09:00 -03:00
romanrizzi
36425eb9f0 Revert "FEATURE: Publish read state on group messages. (#7989) [Undo revert] (#8024)"
This reverts commit 5dda5c2f7c.
2019-08-20 13:29:22 -03:00
Roman Rizzi
5dda5c2f7c
FEATURE: Publish read state on group messages. (#7989) [Undo revert] (#8024)
* Reenable: "FEATURE: Publish read state on group messages. (#7989)"

This reverts commit 67f5cc1ce8.

* FIX: Read indicator only appears when the group setting is enabled
2019-08-20 11:57:25 -03:00
romanrizzi
67f5cc1ce8 Revert "FEATURE: Publish read state on group messages. (#7989)"
This reverts commit 1630dae2db.
2019-08-20 10:24:34 -03:00
Roman Rizzi
1630dae2db
FEATURE: Publish read state on group messages. (#7989)
* 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
2019-08-20 09:46:57 -03:00
Daniel Waterworth
e219588142 DEV: Prefabrication (test optimization) (#7414)
* Introduced fab!, a helper that creates database state for a group

It's almost identical to let_it_be, except:

 1. It creates a new object for each test by default,
 2. You can disable it using PREFABRICATION=0
2019-05-07 13:12:20 +10:00
Sam Saffron
4ea21fa2d0 DEV: use #frozen_string_literal: true on all spec
This change both speeds up specs (less strings to allocate) and helps catch
cases where methods in Discourse are mutating inputs.

Overall we will be migrating everything to use #frozen_string_literal: true
it will take a while, but this is the first and safest move in this direction
2019-04-30 10:27:42 +10:00
Guo Xiang Tan
8dc1463ab3 Enable Lint/ShadowingOuterLocalVariable for Rubocop. 2018-09-04 10:16:42 +08:00
Guo Xiang Tan
1f27c91924 DEV: Better diagnostic when topic tracking state tests fail. 2018-05-24 15:36:41 +08:00
Guo Xiang Tan
fe96ef6ed2 UX: Use topic list for displaying group messages on group page.
https://meta.discourse.org/t/group-inbox-on-a-groups-page-mockup/71319
2018-03-16 11:56:40 +08:00
Guo Xiang Tan
d5c828213f FIX: Circular references warning in TopicTrackingState. 2018-03-13 08:35:15 +08:00
Guo Xiang Tan
3045c589f6 FIX: Publish live messages to both team inbox and archive on update. 2018-03-07 14:01:20 +08:00
Guo Xiang Tan
5169be6080 FIX: User archiving message should also publish to sent section. 2018-03-07 11:39:23 +08:00
Guo Xiang Tan
bef35f7be5 FIX: User archive messages should only publish to the user. 2018-03-07 11:28:29 +08:00
Guo Xiang Tan
1365bab0d7 FEATURE: Live updates for user's messages page.
https://meta.discourse.org/t/group-inbox-messages-not-updated-for-new-posts/38189
2018-03-06 18:15:21 +08:00
Guo Xiang Tan
d576056cff REFACTOR: Add basic tests for TopicTrackingState#publish_*.
* Ensure we don't publish events for PMs.
2018-03-06 17:37:53 +08:00
Guo Xiang Tan
5012d46cbd Add rubocop to our build. (#5004) 2017-07-28 10:20:09 +09:00
Sam
c04d4171ff FIX: whisper no longer experimental
- Regular users are not notified of whispers
- Regular users no longer have "stuck" topics in unread
- Additional tracking for staff highest post number
- Remove a bunch of unused columns in topics table
2016-12-02 17:03:31 +11:00
Andy Waite
3e50313fdc Prepare for separation of RSpec helper files
Since rspec-rails 3, the default installation creates two helper files:
* `spec_helper.rb`
* `rails_helper.rb`

`spec_helper.rb` is intended as a way of running specs that do not
require Rails, whereas `rails_helper.rb` loads Rails (as Discourse's
current `spec_helper.rb` does).

For more information:

https://www.relishapp.com/rspec/rspec-rails/docs/upgrade#default-helper-files

In this commit, I've simply replaced all instances of `spec_helper` with
`rails_helper`, and renamed the original `spec_helper.rb`.

This brings the Discourse project closer to the standard usage of RSpec
in a Rails app.

At present, every spec relies on loading Rails, but there are likely
many that don't need to. In a future pull request, I hope to introduce a
separate, minimal `spec_helper.rb` which can be used in tests which
don't rely on Rails.
2015-12-01 20:39:42 +00:00
Sam
6f43b575a8 FEATURE: no need to cap new and unread together anymore
- leave unread alone
- cap new at 500 per site, with a site setting
2015-10-01 17:17:15 +10:00
Sam
335be272ff FEATURE: implement capping of new/unread
We cap new and unread at 2/5th of SiteSetting.max_tracked_new_unread

This dynamic capping is applied under 2 conditions:

1. New capping is applied once every 15 minutes in the periodical job, this effectively ensures that usually even super active sites are capped at 200 new items

2. Unread capping is applied if a user hits max_tracked_new_unread,
  meaning if new + unread == 500, we defer a job that runs within 15 minutes that will cap user at 200 unread

This logic ensures that at worst case a user gets "bad" numbers for 15 minutes and then the system goes ahead and fixes itself up
2015-09-07 12:03:17 +10:00