From ac7773a30df362b886bb552a8c43034a381eff59 Mon Sep 17 00:00:00 2001 From: jbrw Date: Tue, 9 Mar 2021 16:05:11 -0500 Subject: [PATCH] FEATURE: allow category group moderators to pin/unpin topics (#12325) * FEATURE: allow category group moderators to pin/unpin topics Category group moderators should be able to pin/unpin any topics within a category where they have appropraite category group moderator permissions. --- .../app/controllers/feature-topic.js | 5 ++ .../app/templates/modal/feature-topic.hbs | 74 ++++++++++--------- .../discourse/app/widgets/topic-admin-menu.js | 24 +++--- .../discourse/tests/acceptance/topic-test.js | 42 +++++++++++ .../discourse/tests/fixtures/topic.js | 2 + app/controllers/topics_controller.rb | 2 + .../topic_view_details_serializer.rb | 5 ++ lib/guardian/topic_guardian.rb | 1 + spec/requests/topics_controller_spec.rb | 15 +++- 9 files changed, 122 insertions(+), 48 deletions(-) diff --git a/app/assets/javascripts/discourse/app/controllers/feature-topic.js b/app/assets/javascripts/discourse/app/controllers/feature-topic.js index d05003e9a35..f53288b4cad 100644 --- a/app/assets/javascripts/discourse/app/controllers/feature-topic.js +++ b/app/assets/javascripts/discourse/app/controllers/feature-topic.js @@ -47,6 +47,11 @@ export default Controller.extend(ModalFunctionality, { return I18n.t(name, { categoryLink, until }); }, + @discourseComputed("model.details.can_pin_unpin_topic") + canPinGlobally(canPinUnpinTopic) { + return this.currentUser.isElder && canPinUnpinTopic; + }, + @discourseComputed("categoryLink") pinMessage(categoryLink) { return I18n.t("topic.feature_topic.pin", { categoryLink }); diff --git a/app/assets/javascripts/discourse/app/templates/modal/feature-topic.hbs b/app/assets/javascripts/discourse/app/templates/modal/feature-topic.hbs index efc5bdc5f8b..dc280e179d0 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/feature-topic.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/feature-topic.hbs @@ -71,39 +71,26 @@

-
-
-

- {{#conditional-loading-spinner size="small" condition=loading}} - {{#if pinnedGloballyCount}} - {{html-safe (i18n "topic.feature_topic.already_pinned_globally" count=pinnedGloballyCount)}} - {{else}} - {{html-safe (i18n "topic.feature_topic.not_pinned_globally")}} - {{/if}} - {{/conditional-loading-spinner}} -

-

- {{i18n "topic.feature_topic.global_pin_note"}} -

- {{#if site.isMobileDevice}} + {{#if canPinGlobally}} +
+

- {{i18n "topic.feature_topic.pin_globally"}} + {{#conditional-loading-spinner size="small" condition=loading}} + {{#if pinnedGloballyCount}} + {{html-safe (i18n "topic.feature_topic.already_pinned_globally" count=pinnedGloballyCount)}} + {{else}} + {{html-safe (i18n "topic.feature_topic.not_pinned_globally")}} + {{/if}} + {{/conditional-loading-spinner}}

-

- {{future-date-input - class="pin-until" - includeFarFuture=true - clearable=true - input=model.pinnedGloballyUntil - onChangeInput=(action (mut model.pinnedGloballyUntil)) - }} - {{popup-input-tip validation=pinGloballyValidation shownAt=pinGloballyTipShownAt}} +

+ {{i18n "topic.feature_topic.global_pin_note"}}

- {{else}} -

- {{i18n "topic.feature_topic.pin_globally"}} - - {{d-icon "far-clock"}} + {{#if site.isMobileDevice}} +

+ {{i18n "topic.feature_topic.pin_globally"}} +

+

{{future-date-input class="pin-until" includeFarFuture=true @@ -112,14 +99,29 @@ onChangeInput=(action (mut model.pinnedGloballyUntil)) }} {{popup-input-tip validation=pinGloballyValidation shownAt=pinGloballyTipShownAt}} - +

+ {{else}} +

+ {{i18n "topic.feature_topic.pin_globally"}} + + {{d-icon "far-clock"}} + {{future-date-input + class="pin-until" + includeFarFuture=true + clearable=true + input=model.pinnedGloballyUntil + onChangeInput=(action (mut model.pinnedGloballyUntil)) + }} + {{popup-input-tip validation=pinGloballyValidation shownAt=pinGloballyTipShownAt}} + +

+ {{/if}} +

+ {{d-button action=(action "pinGlobally") icon="thumbtack" label="topic.feature.pin_globally" class="btn-primary"}}

- {{/if}} -

- {{d-button action=(action "pinGlobally") icon="thumbtack" label="topic.feature.pin_globally" class="btn-primary"}} -

+
-
+ {{/if}} {{/if}} {{#if currentUser.staff}}
diff --git a/app/assets/javascripts/discourse/app/widgets/topic-admin-menu.js b/app/assets/javascripts/discourse/app/widgets/topic-admin-menu.js index 96c97f778d2..0c7ebe9b449 100644 --- a/app/assets/javascripts/discourse/app/widgets/topic-admin-menu.js +++ b/app/assets/javascripts/discourse/app/widgets/topic-admin-menu.js @@ -206,17 +206,23 @@ export default createWidget("topic-admin-menu", { icon: "far-clock", label: "actions.timed_update", }); + } - if (!isPrivateMessage && (topic.get("visible") || featured)) { - this.addActionButton({ - className: "topic-admin-pin", - buttonClass: "popup-menu-btn", - action: "showFeatureTopic", - icon: "thumbtack", - label: featured ? "actions.unpin" : "actions.pin", - }); - } + if ( + details.get("can_pin_unpin_topic") && + !isPrivateMessage && + (topic.get("visible") || featured) + ) { + this.addActionButton({ + className: "topic-admin-pin", + buttonClass: "popup-menu-btn", + action: "showFeatureTopic", + icon: "thumbtack", + label: featured ? "actions.unpin" : "actions.pin", + }); + } + if (this.get("currentUser.canManageTopic")) { if (this.currentUser.get("staff")) { this.addActionButton({ className: "topic-admin-change-timestamp", diff --git a/app/assets/javascripts/discourse/tests/acceptance/topic-test.js b/app/assets/javascripts/discourse/tests/acceptance/topic-test.js index a48537aa51c..7a5ab8f83c8 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/topic-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/topic-test.js @@ -459,3 +459,45 @@ acceptance("Topic with title decorated", function (needs) { ); }); }); + +acceptance("Topic pinning/unpinning as an admin", function (needs) { + needs.user({ admin: true }); + + test("Admin pinning topic", async function (assert) { + await visit("/t/topic-for-group-moderators/2480"); + + await click(".toggle-admin-menu"); + await click(".topic-admin-pin .btn"); + + assert.ok( + exists(".feature-topic .btn-primary"), + "it should show the 'Pin Topic' button" + ); + + assert.ok( + exists(".make-banner"), + "it should show the 'Banner Topic' button" + ); + }); +}); + +acceptance("Topic pinning/unpinning as a group moderator", function (needs) { + needs.user({ moderator: false, admin: false, trust_level: 1 }); + + test("Group category moderator pinning topic", async function (assert) { + await visit("/t/topic-for-group-moderators/2480"); + + await click(".toggle-admin-menu"); + await click(".topic-admin-pin .btn"); + + assert.ok( + exists(".feature-topic .btn-primary"), + "it should show the 'Pin Topic' button" + ); + + assert.ok( + !exists(".make-banner"), + "it should not show the 'Banner Topic' button" + ); + }); +}); diff --git a/app/assets/javascripts/discourse/tests/fixtures/topic.js b/app/assets/javascripts/discourse/tests/fixtures/topic.js index fbb7f696c9f..95debff3a28 100644 --- a/app/assets/javascripts/discourse/tests/fixtures/topic.js +++ b/app/assets/javascripts/discourse/tests/fixtures/topic.js @@ -2206,6 +2206,7 @@ export default { can_publish_page: true, can_invite_via_email: true, can_toggle_topic_visibility: true, + can_pin_unpin_topic: true, auto_close_at: null, auto_close_hours: null, auto_close_based_on_last_post: false, @@ -5597,6 +5598,7 @@ export default { can_toggle_topic_visibility: true, can_split_merge_topic: true, can_edit_staff_notes: true, + can_pin_unpin_topic: true, can_moderate_category: true, participants: [ { diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 7fa7cf93bf7..171d246432b 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -429,6 +429,8 @@ class TopicsController < ApplicationController guardian.ensure_can_archive_topic!(@topic) when 'visible' guardian.ensure_can_toggle_topic_visibility!(@topic) + when 'pinned' + guardian.ensure_can_pin_unpin_topic!(@topic) else guardian.ensure_can_moderate!(@topic) end diff --git a/app/serializers/topic_view_details_serializer.rb b/app/serializers/topic_view_details_serializer.rb index 2576bf7406e..0a3077c9ce5 100644 --- a/app/serializers/topic_view_details_serializer.rb +++ b/app/serializers/topic_view_details_serializer.rb @@ -21,6 +21,7 @@ class TopicViewDetailsSerializer < ApplicationSerializer :can_split_merge_topic, :can_edit_staff_notes, :can_toggle_topic_visibility, + :can_pin_unpin_topic, :can_moderate_category] end @@ -149,6 +150,10 @@ class TopicViewDetailsSerializer < ApplicationSerializer scope.can_toggle_topic_visibility?(object.topic) end + def include_can_pin_unpin_topic? + scope.can_pin_unpin_topic?(object.topic) + end + def can_perform_action_available_to_group_moderators? @can_perform_action_available_to_group_moderators ||= scope.can_perform_action_available_to_group_moderators?(object.topic) end diff --git a/lib/guardian/topic_guardian.rb b/lib/guardian/topic_guardian.rb index ea6c1bc8b7f..bf17ca08459 100644 --- a/lib/guardian/topic_guardian.rb +++ b/lib/guardian/topic_guardian.rb @@ -239,6 +239,7 @@ module TopicGuardian alias :can_open_topic? :can_perform_action_available_to_group_moderators? alias :can_split_merge_topic? :can_perform_action_available_to_group_moderators? alias :can_edit_staff_notes? :can_perform_action_available_to_group_moderators? + alias :can_pin_unpin_topic? :can_perform_action_available_to_group_moderators? def can_move_posts?(topic) return false if is_silenced? diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 74749f3240f..73f47e727dc 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -940,12 +940,21 @@ RSpec.describe TopicsController do expect(topic.posts.last.action_code).to eq('archived.disabled') end - it 'should not allow a group moderator to pin a topic' do + it 'should allow a group moderator to pin a topic' do put "/t/#{topic.id}/status.json", params: { - status: 'pinned', enabled: 'true' + status: 'pinned', enabled: 'true', until: 2.weeks.from_now } - expect(response.status).to eq(403) + expect(response.status).to eq(200) + expect(topic.reload.pinned_at).to_not eq(nil) + end + + it 'should allow a group moderator to unpin a topic' do + put "/t/#{topic.id}/status.json", params: { + status: 'pinned', enabled: 'false' + } + + expect(response.status).to eq(200) expect(topic.reload.pinned_at).to eq(nil) end