mirror of
https://github.com/discourse/discourse.git
synced 2026-09-05 04:40:41 -05:00
UX: Remove the topics_unread_when_closed user preference (#42899)
the "consider topics unread when they are closed" preference and the "notify users" option on bulk close currently have no effect, because #40481 stopped small actions from advancing `highest_post_number` this change removes the preference, the dead `TopicStatusUpdater#update_read_state_for`, and the notify option on bulk close
This commit is contained in:
@@ -22,6 +22,7 @@ class UserOption < ActiveRecord::Base
|
||||
"only_chat_push_notifications", # TODO(2027-01): replaced by push_notification_level; drop the column in a follow-up PR once this has shipped
|
||||
"chat_send_shortcut", # TODO(2027-01): replaced by send_shortcut; drop the column in a follow-up PR once this has shipped
|
||||
"enable_defer", # TODO(2027-02): the preference was removed; drop the column in a follow-up PR once this has shipped
|
||||
"topics_unread_when_closed", # TODO: Remove when 20260826124054_drop_topics_unread_when_closed_from_user_options has been promoted to pre-deploy
|
||||
]
|
||||
# TODO: remove after 20260824051214_drop_ai_search_discovery_preferences_from_user_options has been promoted
|
||||
self.ignored_columns += %w[
|
||||
@@ -350,7 +351,6 @@ end
|
||||
# theme_key_seq :integer default(0), not null
|
||||
# timezone :string
|
||||
# title_count_mode_key :integer default(0), not null
|
||||
# topics_unread_when_closed :boolean default(TRUE), not null
|
||||
# understood_languages :string default([]), not null, is an Array
|
||||
# watched_precedence_over_muted :boolean default(FALSE), not null
|
||||
# color_scheme_id :integer
|
||||
|
||||
@@ -46,7 +46,6 @@ class UserOptionSerializer < ApplicationSerializer
|
||||
:sidebar_link_to_filtered_list,
|
||||
:sidebar_show_count_of_new_items,
|
||||
:watched_precedence_over_muted,
|
||||
:topics_unread_when_closed,
|
||||
:composition_mode,
|
||||
:interface_color_mode,
|
||||
:show_original_content,
|
||||
|
||||
@@ -10,15 +10,7 @@ TopicStatusUpdater =
|
||||
updated = nil
|
||||
Topic.transaction do
|
||||
updated = change(status, opts)
|
||||
if updated
|
||||
highest_post_number = topic.highest_post_number
|
||||
create_moderator_post_for(status, opts)
|
||||
update_read_state_for(
|
||||
status,
|
||||
highest_post_number,
|
||||
silent_tracking: opts[:silent_tracking],
|
||||
)
|
||||
end
|
||||
create_moderator_post_for(status, opts) if updated
|
||||
end
|
||||
|
||||
updated
|
||||
@@ -106,29 +98,6 @@ TopicStatusUpdater =
|
||||
topic.reload
|
||||
end
|
||||
|
||||
def update_read_state_for(status, old_highest_read, silent_tracking: false)
|
||||
if (status.autoclosed? && status.enabled?) || (status.closed? && silent_tracking)
|
||||
# let's pretend all the people that read up to the autoclose message
|
||||
# actually read the topic
|
||||
PostTiming.pretend_read(topic.id, old_highest_read, topic.highest_post_number)
|
||||
end
|
||||
|
||||
if status.closed? && status.enabled?
|
||||
sql_query = <<-SQL
|
||||
SELECT DISTINCT post_timings.user_id
|
||||
FROM post_timings
|
||||
JOIN user_options ON user_options.user_id = post_timings.user_id
|
||||
WHERE post_timings.topic_id = :topic_id
|
||||
AND user_options.topics_unread_when_closed = 'f'
|
||||
SQL
|
||||
user_ids = DB.query_single(sql_query, topic_id: topic.id)
|
||||
|
||||
if user_ids.present?
|
||||
PostTiming.pretend_read(topic.id, old_highest_read, topic.highest_post_number, user_ids)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
def message_for(status)
|
||||
if status.autoclosed?
|
||||
locale_key = status.locale_key.dup
|
||||
|
||||
@@ -61,7 +61,6 @@ class UserUpdater
|
||||
sidebar_link_to_filtered_list
|
||||
sidebar_show_count_of_new_items
|
||||
watched_precedence_over_muted
|
||||
topics_unread_when_closed
|
||||
composition_mode
|
||||
send_shortcut
|
||||
automatically_translate
|
||||
|
||||
@@ -2484,7 +2484,6 @@ en:
|
||||
watch_topic: "Watch topic"
|
||||
track_topic: "Track topic"
|
||||
do_nothing: "Do nothing"
|
||||
topics_unread_when_closed: "Consider topics unread when they are closed"
|
||||
|
||||
invited:
|
||||
title: "Invites"
|
||||
|
||||
@@ -0,0 +1,15 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
require "migration/column_dropper"
|
||||
|
||||
class DropTopicsUnreadWhenClosedFromUserOptions < ActiveRecord::Migration[8.0]
|
||||
DROPPED_COLUMNS = { user_options: %i[topics_unread_when_closed] }
|
||||
|
||||
def up
|
||||
DROPPED_COLUMNS.each { |table, columns| Migration::ColumnDropper.execute_drop(table, columns) }
|
||||
end
|
||||
|
||||
def down
|
||||
raise ActiveRecord::IrreversibleMigration
|
||||
end
|
||||
end
|
||||
+1
-1
@@ -11873,7 +11873,6 @@ CREATE TABLE public.user_options (
|
||||
sidebar_show_count_of_new_items boolean DEFAULT false NOT NULL,
|
||||
watched_precedence_over_muted boolean DEFAULT false NOT NULL,
|
||||
chat_separate_sidebar_mode integer DEFAULT 0 NOT NULL,
|
||||
topics_unread_when_closed boolean DEFAULT true NOT NULL,
|
||||
show_thread_title_prompts boolean DEFAULT true NOT NULL,
|
||||
auto_image_caption boolean DEFAULT false NOT NULL,
|
||||
enable_smart_lists boolean DEFAULT true NOT NULL,
|
||||
@@ -23358,6 +23357,7 @@ ALTER TABLE ONLY public.ad_plugin_house_ads_groups
|
||||
SET search_path TO "$user", public;
|
||||
|
||||
INSERT INTO "schema_migrations" (version) VALUES
|
||||
('20260826124054'),
|
||||
('20260824091843'),
|
||||
('20260824072257'),
|
||||
('20260824051214'),
|
||||
|
||||
@@ -297,7 +297,6 @@ export default class BulkSelectTopicsDropdown extends Component {
|
||||
break;
|
||||
case "close-topics":
|
||||
this.showBulkTopicActionsModal("close", "close_topics", {
|
||||
allowSilent: true,
|
||||
description: i18n(`topic_bulk_actions.close_topics.description`),
|
||||
confirmButtonTranslationKey: "topics.bulk.confirm_close_topics",
|
||||
});
|
||||
|
||||
@@ -170,7 +170,6 @@ export default class extends Controller {
|
||||
"tracked_category_ids",
|
||||
"watched_first_post_category_ids",
|
||||
"watched_precedence_over_muted",
|
||||
"topics_unread_when_closed",
|
||||
];
|
||||
|
||||
if (this.siteSettings.tagging_enabled) {
|
||||
|
||||
@@ -154,7 +154,6 @@ let userOptionFields = [
|
||||
"theme_ids",
|
||||
"timezone",
|
||||
"title_count_mode",
|
||||
"topics_unread_when_closed",
|
||||
"understood_languages",
|
||||
"watched_precedence_over_muted",
|
||||
];
|
||||
|
||||
@@ -69,11 +69,6 @@ export default <template>
|
||||
customAttrNames=@controller.customAttrNames
|
||||
}}
|
||||
/>
|
||||
|
||||
<PreferenceCheckbox
|
||||
@labelKey="user.topics_unread_when_closed"
|
||||
@checked={{@controller.model.user_option.topics_unread_when_closed}}
|
||||
/>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
||||
@@ -238,12 +238,7 @@ class TopicsBulkAction
|
||||
def close
|
||||
topics.each do |t|
|
||||
if guardian.can_moderate?(t)
|
||||
t.update_status(
|
||||
"closed",
|
||||
true,
|
||||
@user,
|
||||
{ message: @operation[:message], silent_tracking: @operation[:silent] },
|
||||
)
|
||||
t.update_status("closed", true, @user, { message: @operation[:message] })
|
||||
@changed_ids << t.id
|
||||
end
|
||||
end
|
||||
|
||||
@@ -61,7 +61,6 @@ module Migrations
|
||||
theme_key_seq: item[:theme_key_seq],
|
||||
timezone: item[:timezone],
|
||||
title_count_mode_key: item[:title_count_mode_key],
|
||||
topics_unread_when_closed: item[:topics_unread_when_closed],
|
||||
understood_languages: item[:understood_languages],
|
||||
watched_precedence_over_muted: item[:watched_precedence_over_muted],
|
||||
)
|
||||
|
||||
@@ -560,7 +560,6 @@ CREATE TABLE user_options
|
||||
theme_key_seq INTEGER,
|
||||
timezone TEXT,
|
||||
title_count_mode_key INTEGER,
|
||||
topics_unread_when_closed BOOLEAN,
|
||||
understood_languages TEXT,
|
||||
watched_precedence_over_muted BOOLEAN
|
||||
);
|
||||
|
||||
@@ -61,12 +61,11 @@ module Migrations
|
||||
theme_key_seq,
|
||||
timezone,
|
||||
title_count_mode_key,
|
||||
topics_unread_when_closed,
|
||||
understood_languages,
|
||||
watched_precedence_over_muted
|
||||
)
|
||||
VALUES (
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
|
||||
?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?
|
||||
)
|
||||
SQL
|
||||
private_constant :SQL
|
||||
@@ -124,7 +123,6 @@ module Migrations
|
||||
# @param theme_key_seq [Integer, nil]
|
||||
# @param timezone [String, nil]
|
||||
# @param title_count_mode_key [Integer, nil]
|
||||
# @param topics_unread_when_closed [Boolean, nil]
|
||||
# @param understood_languages [String, nil]
|
||||
# @param watched_precedence_over_muted [Boolean, nil]
|
||||
#
|
||||
@@ -181,7 +179,6 @@ module Migrations
|
||||
theme_key_seq: nil,
|
||||
timezone: nil,
|
||||
title_count_mode_key: nil,
|
||||
topics_unread_when_closed: nil,
|
||||
understood_languages: nil,
|
||||
watched_precedence_over_muted: nil
|
||||
)
|
||||
@@ -238,7 +235,6 @@ module Migrations
|
||||
theme_key_seq,
|
||||
timezone,
|
||||
title_count_mode_key,
|
||||
Migrations::Database.format_boolean(topics_unread_when_closed),
|
||||
understood_languages,
|
||||
Migrations::Database.format_boolean(watched_precedence_over_muted),
|
||||
)
|
||||
|
||||
@@ -841,9 +841,6 @@
|
||||
"seen_popups": {
|
||||
"type": ["array", "null"]
|
||||
},
|
||||
"topics_unread_when_closed": {
|
||||
"type": "boolean"
|
||||
},
|
||||
"composition_mode": {
|
||||
"type": "integer"
|
||||
},
|
||||
@@ -905,7 +902,6 @@
|
||||
"title_count_mode",
|
||||
"timezone",
|
||||
"skip_new_user_tips",
|
||||
"topics_unread_when_closed",
|
||||
"interface_color_mode",
|
||||
"show_original_content",
|
||||
"send_shortcut",
|
||||
|
||||
@@ -7,8 +7,7 @@ RSpec.describe TopicStatusUpdater do
|
||||
fab!(:user) { Fabricate(:user, refresh_auto_groups: true) }
|
||||
fab!(:admin)
|
||||
|
||||
it "avoids notifying on automatically closed topics" do
|
||||
# TODO: TopicStatusUpdater should suppress message bus updates from the users it "pretends to read"
|
||||
it "does not advance read state when a topic is automatically closed" do
|
||||
post =
|
||||
PostCreator.create(
|
||||
user,
|
||||
@@ -23,16 +22,14 @@ RSpec.describe TopicStatusUpdater do
|
||||
|
||||
expect(post.topic.posts.count).to eq(2)
|
||||
|
||||
# The autoclose small_action advances no counter, so the author's read state
|
||||
# stays on the original post.
|
||||
tu = TopicUser.find_by(user_id: user.id)
|
||||
expect(tu.last_read_post_number).to eq(1)
|
||||
end
|
||||
|
||||
it "respects topics_unread_when_closed preference for private messages" do
|
||||
user_wants_unread = Fabricate(:user)
|
||||
user_wants_unread.user_option.update!(topics_unread_when_closed: true)
|
||||
|
||||
user_wants_read = Fabricate(:user)
|
||||
user_wants_read.user_option.update!(topics_unread_when_closed: false)
|
||||
it "does not advance read state when a private message is closed" do
|
||||
recipient = Fabricate(:user)
|
||||
|
||||
post =
|
||||
PostCreator.create(
|
||||
@@ -40,27 +37,21 @@ RSpec.describe TopicStatusUpdater do
|
||||
raw: "this is a private message",
|
||||
title: "private message title",
|
||||
archetype: Archetype.private_message,
|
||||
target_usernames: [user_wants_unread.username, user_wants_read.username],
|
||||
target_usernames: [recipient.username],
|
||||
)
|
||||
|
||||
TopicUser.update_last_read(user_wants_unread, post.topic.id, 1, 1, 0)
|
||||
TopicUser.update_last_read(user_wants_read, post.topic.id, 1, 1, 0)
|
||||
|
||||
PostTiming.create!(topic: post.topic, post_number: 1, user: user_wants_unread, msecs: 1000)
|
||||
PostTiming.create!(topic: post.topic, post_number: 1, user: user_wants_read, msecs: 1000)
|
||||
TopicUser.update_last_read(recipient, post.topic.id, 1, 1, 0)
|
||||
PostTiming.create!(topic: post.topic, post_number: 1, user: recipient, msecs: 1000)
|
||||
|
||||
TopicStatusUpdater.new(post.topic, admin).update!("closed", true)
|
||||
|
||||
# Should have 2 posts (original + close action)
|
||||
expect(post.topic.posts.count).to eq(2)
|
||||
|
||||
# In PMs, close small_action posts only bump highest_staff_post_number,
|
||||
# so neither user's last_read_post_number advances past the original post.
|
||||
tu_wants_unread = TopicUser.find_by(user: user_wants_unread, topic: post.topic)
|
||||
expect(tu_wants_unread.last_read_post_number).to eq(1)
|
||||
|
||||
tu_wants_read = TopicUser.find_by(user: user_wants_read, topic: post.topic)
|
||||
expect(tu_wants_read.last_read_post_number).to eq(1)
|
||||
# The close small_action advances no counter, so the recipient's read state
|
||||
# stays on the original post.
|
||||
tu = TopicUser.find_by(user: recipient, topic: post.topic)
|
||||
expect(tu.last_read_post_number).to eq(1)
|
||||
end
|
||||
|
||||
it "adds an autoclosed message" do
|
||||
|
||||
@@ -16,10 +16,6 @@ module PageObjects
|
||||
find("#dismiss-read-confirm").click
|
||||
end
|
||||
|
||||
def click_notify
|
||||
find("#topic-bulk-action-options__notify").click
|
||||
end
|
||||
|
||||
def fill_in_close_note(message)
|
||||
find("#bulk-close-note").set(message)
|
||||
end
|
||||
|
||||
@@ -9,7 +9,6 @@ describe "Topic bulk select" do
|
||||
let(:topic_list) { PageObjects::Components::TopicList.new }
|
||||
let(:topic_page) { PageObjects::Pages::Topic.new }
|
||||
let(:topic_bulk_actions_modal) { PageObjects::Modals::TopicBulkActions.new }
|
||||
let(:topic_view) { PageObjects::Components::TopicView.new }
|
||||
|
||||
def open_bulk_actions_modal(topics_to_select = nil, action)
|
||||
topic_list_header.click_bulk_select_button
|
||||
@@ -356,7 +355,6 @@ describe "Topic bulk select" do
|
||||
topic_list.click_topic_checkbox(topics.third)
|
||||
topic_list_header.click_bulk_select_topics_dropdown
|
||||
topic_list_header.click_bulk_button("close-topics")
|
||||
topic_bulk_actions_modal.click_notify
|
||||
topic_bulk_actions_modal.click_bulk_topics_confirm
|
||||
expect(topic_list).to have_closed_status(topics.third)
|
||||
end
|
||||
@@ -367,32 +365,6 @@ describe "Topic bulk select" do
|
||||
expect(topic_list).to have_no_unread_badge(topics.third)
|
||||
end
|
||||
|
||||
it "closes topics silently" do
|
||||
# Watch the topic as a user
|
||||
sign_in(user)
|
||||
topic = topics.first
|
||||
visit("/t/#{topic.slug}/#{topic.id}")
|
||||
expect(topic_view).to have_read_post(topic.posts.first)
|
||||
topic_page.watch_topic
|
||||
|
||||
# Bulk close the topic as an admin
|
||||
using_session(:admin) do
|
||||
sign_in(admin)
|
||||
visit("/latest")
|
||||
topic_list_header.click_bulk_select_button
|
||||
topic_list.click_topic_checkbox(topics.first)
|
||||
topic_list_header.click_bulk_select_topics_dropdown
|
||||
topic_list_header.click_bulk_button("close-topics")
|
||||
topic_bulk_actions_modal.click_bulk_topics_confirm
|
||||
expect(topic_list).to have_closed_status(topics.first)
|
||||
end
|
||||
|
||||
# Check that the user didn't receive a new post notification badge
|
||||
sign_in(user)
|
||||
visit("/latest")
|
||||
expect(topic_list).to have_no_unread_badge(topics.first)
|
||||
end
|
||||
|
||||
it "closes topics with message" do
|
||||
# Bulk close the topic with a message
|
||||
sign_in(admin)
|
||||
|
||||
@@ -0,0 +1,24 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
describe "Closing a topic" do
|
||||
fab!(:topics) { Fabricate.times(10, :post).map(&:topic) }
|
||||
fab!(:admin)
|
||||
fab!(:user)
|
||||
|
||||
let(:topic_list) { PageObjects::Components::TopicList.new }
|
||||
let(:topic_page) { PageObjects::Pages::Topic.new }
|
||||
|
||||
it "does not mark the topic unread" do
|
||||
sign_in(user)
|
||||
topic = topics.third
|
||||
topic_page.visit_topic(topic)
|
||||
topic_page.watch_topic
|
||||
expect(topic_page).to have_read_post(1)
|
||||
|
||||
TopicStatusUpdater.new(topic, admin).update!("closed", true)
|
||||
|
||||
# The close action is a small action, which no longer marks a topic unread.
|
||||
visit("/latest")
|
||||
expect(topic_list).to have_no_unread_badge(topics.third)
|
||||
end
|
||||
end
|
||||
@@ -1,45 +0,0 @@
|
||||
# frozen_string_literal: true
|
||||
|
||||
describe "Topics unread when closed" do
|
||||
fab!(:topics) { Fabricate.times(10, :post).map(&:topic) }
|
||||
let(:topic_list) { PageObjects::Components::TopicList.new }
|
||||
let(:topic_page) { PageObjects::Pages::Topic.new }
|
||||
|
||||
context "when closing a topic" do
|
||||
fab!(:admin)
|
||||
fab!(:user)
|
||||
|
||||
it "close notifications do not appear when disabled" do
|
||||
user.user_option.update!(topics_unread_when_closed: false)
|
||||
sign_in(user)
|
||||
topic = topics.third
|
||||
topic_page.visit_topic(topic)
|
||||
topic_page.watch_topic
|
||||
expect(topic_page).to have_read_post(1)
|
||||
|
||||
# Close the topic as an admin
|
||||
TopicStatusUpdater.new(topic, admin).update!("closed", true)
|
||||
|
||||
# Check that the user did not receive a new post notification badge
|
||||
visit("/latest")
|
||||
expect(topic_list).to have_no_unread_badge(topics.third)
|
||||
end
|
||||
|
||||
it "close notifications do not appear even when enabled (the default)" do
|
||||
user.user_option.update!(topics_unread_when_closed: true)
|
||||
sign_in(user)
|
||||
topic = topics.third
|
||||
topic_page.visit_topic(topic)
|
||||
topic_page.watch_topic
|
||||
expect(topic_page).to have_read_post(1)
|
||||
|
||||
# Close the topic as an admin
|
||||
TopicStatusUpdater.new(topic, admin).update!("closed", true)
|
||||
|
||||
# The close action is a small action, which no longer marks a topic unread,
|
||||
# so the badge does not appear regardless of the topics_unread_when_closed preference.
|
||||
visit("/latest")
|
||||
expect(topic_list).to have_no_unread_badge(topics.third)
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in New Issue
Block a user