Discource-C
/
discourse
mirror of https://github.com/discourse/discourse.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

FIX: Improve reliability of topic tracking state (#17387) 

The `unread_not_too_old` attribute is a little odd because there should never be a case where
the user's first_unread_at column is less than the `Topic#updated_at`
column of an unread topic. The `unread_not_too_old` attribute is causing
a bug where topic states synced into `TopicTrackingState` do not appear
as unread because the attribute does not exsist on a normal `Topic`
object and hence never set.
This commit is contained in:
Alan Guo Xiang Tan 2022-07-14 13:44:58 +08:00 committed by GitHub
parent 6b2ea1b47b
commit 92e1e43104
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 1 additions and 49 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
app/assets/javascripts/discourse/app/models/topic-tracking-state.js
View File

@ -23,8 +23,7 @@ function isUnread(topic) {
return (
topic.last_read_post_number !== null &&
topic.last_read_post_number < topic.highest_post_number &&
topic.notification_level >= NotificationLevels.TRACKING &&
topic.unread_not_too_old
topic.notification_level >= NotificationLevels.TRACKING
);
}

4
app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js
View File

@ -277,7 +277,6 @@ acceptance("Sidebar - Categories Section", function (needs) {
category_id: category1.id,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -288,7 +287,6 @@ acceptance("Sidebar - Categories Section", function (needs) {
category_id: category1.id,
notification_level: 2,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -299,7 +297,6 @@ acceptance("Sidebar - Categories Section", function (needs) {
category_id: category2.id,
notification_level: 2,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -310,7 +307,6 @@ acceptance("Sidebar - Categories Section", function (needs) {
category_id: category2.id,
notification_level: 2,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
]);

4
app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js
View File

@ -219,7 +219,6 @@ acceptance("Sidebar - Tags section", function (needs) {
category_id: 1,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["tag1"],
},
@ -231,7 +230,6 @@ acceptance("Sidebar - Tags section", function (needs) {
category_id: 2,
notification_level: 2,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["tag1"],
},
@ -243,7 +241,6 @@ acceptance("Sidebar - Tags section", function (needs) {
category_id: 3,
notification_level: 2,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["tag2"],
},
@ -255,7 +252,6 @@ acceptance("Sidebar - Tags section", function (needs) {
category_id: 4,
notification_level: 2,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["tag4"],
},

10
app/assets/javascripts/discourse/tests/acceptance/sidebar-topics-section-test.js
View File

@ -287,7 +287,6 @@ acceptance("Sidebar - Topics Section", function (needs) {
category_id: 1,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -298,7 +297,6 @@ acceptance("Sidebar - Topics Section", function (needs) {
category_id: 2,
notification_level: 2,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -309,7 +307,6 @@ acceptance("Sidebar - Topics Section", function (needs) {
category_id: 3,
notification_level: 2,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -320,7 +317,6 @@ acceptance("Sidebar - Topics Section", function (needs) {
category_id: 4,
notification_level: 2,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
]);
@ -484,7 +480,6 @@ acceptance("Sidebar - Topics Section", function (needs) {
category_id: category.id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -495,7 +490,6 @@ acceptance("Sidebar - Topics Section", function (needs) {
category_id: category.subcategories[0].id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -506,7 +500,6 @@ acceptance("Sidebar - Topics Section", function (needs) {
category_id: category.subcategories[0].subcategories[0].id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -517,7 +510,6 @@ acceptance("Sidebar - Topics Section", function (needs) {
category_id: 3,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -528,7 +520,6 @@ acceptance("Sidebar - Topics Section", function (needs) {
category_id: 3,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -539,7 +530,6 @@ acceptance("Sidebar - Topics Section", function (needs) {
category_id: 1234,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["tag3"],
},

13
app/assets/javascripts/discourse/tests/acceptance/topic-discovery-tracked-test.js
View File

@ -49,7 +49,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: 1,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -60,7 +59,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: 2,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
]);
@ -116,7 +114,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: category.id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -127,7 +124,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: category.subcategories[0].id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -138,7 +134,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: category.subcategories[0].subcategories[0].id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -149,7 +144,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: 3,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -160,7 +154,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: 3,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -171,7 +164,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: 1234,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["tag3"],
},
@ -250,7 +242,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: 1,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["someothertag"],
},
@ -299,7 +290,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: category.id,
notification_level: null,
created_in_new_period: true,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -310,7 +300,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: category.id,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -321,7 +310,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: 3,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
},
{
@ -332,7 +320,6 @@ acceptance("Topic Discovery Tracked", function (needs) {
category_id: 1234,
notification_level: NotificationLevels.TRACKING,
created_in_new_period: false,
unread_not_too_old: true,
treat_as_new_topic_start_date: "2022-05-09T03:17:34.286Z",
tags: ["tag3"],
},

1
app/assets/javascripts/discourse/tests/integration/components/widgets/hamburger-menu-test.js
View File

@ -136,7 +136,6 @@ module("Integration | Component | Widget | hamburger-menu", function (hooks) {
last_read_post_number: 1,
highest_post_number: 2,
notification_level: NotificationLevels.TRACKING,
unread_not_too_old: true,
});
}
} else {

7
app/assets/javascripts/discourse/tests/unit/models/topic-tracking-state-test.js
View File

@ -67,7 +67,6 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
highest_post_number: 7,
tags: ["pending"],
notification_level: NotificationLevels.TRACKING,
unread_not_too_old: true,
},
{
topic_id: 5,
@ -75,7 +74,6 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
highest_post_number: 7,
tags: ["bar", "pending"],
notification_level: NotificationLevels.TRACKING,
unread_not_too_old: true,
},
{
topic_id: 6,
@ -133,7 +131,6 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
highest_post_number: 7,
tags: ["pending"],
notification_level: NotificationLevels.TRACKING,
unread_not_too_old: true,
},
{
topic_id: 5,
@ -141,7 +138,6 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
highest_post_number: 7,
tags: ["bar", "pending"],
notification_level: NotificationLevels.TRACKING,
unread_not_too_old: true,
},
{
topic_id: 6,
@ -219,7 +215,6 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
category_id: 7,
tags: ["bug"],
notification_level: NotificationLevels.TRACKING,
unread_not_too_old: true,
},
{
topic_id: 5,
@ -228,7 +223,6 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
tags: ["bar", "bug"],
category_id: 7,
notification_level: NotificationLevels.TRACKING,
unread_not_too_old: true,
},
{
topic_id: 6,
@ -455,7 +449,6 @@ discourseModule("Unit | Model | topic-tracking-state", function (hooks) {
last_read_post_number: 4,
highest_post_number: 5,
notification_level: NotificationLevels.TRACKING,
unread_not_too_old: true,
},
{
topic_id: 222,

1
app/models/topic_tracking_state.rb
View File

@ -171,7 +171,6 @@ class TopicTrackingState
created_at: post.created_at,
category_id: post.topic.category_id,
archetype: post.topic.archetype,
unread_not_too_old: true
}
if tags

6
app/serializers/topic_tracking_state_serializer.rb
View File

@ -8,7 +8,6 @@ class TopicTrackingStateSerializer < ApplicationSerializer
:category_id,
:notification_level,
:created_in_new_period,
:unread_not_too_old,
:treat_as_new_topic_start_date,
:tags
@ -17,11 +16,6 @@ class TopicTrackingStateSerializer < ApplicationSerializer
object.created_at >= treat_as_new_topic_start_date
end
def unread_not_too_old
return true if object.first_unread_at.blank?
object.updated_at >= object.first_unread_at
end
def include_tags?
object.respond_to?(:tags)
end

1
spec/serializers/topic_tracking_state_serializer_spec.rb
View File

@ -14,7 +14,6 @@ describe TopicTrackingStateSerializer do
expect(serialized[:created_at]).to be_present
expect(serialized[:notification_level]).to eq(nil)
expect(serialized[:created_in_new_period]).to eq(true)
expect(serialized[:unread_not_too_old]).to eq(true)
expect(serialized[:treat_as_new_topic_start_date]).to be_present
expect(serialized.has_key?(:tags)).to eq(false)
end