From 6b04967e2f0962514fe8b7db7b83ba38789e85ff Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Thu, 25 Jan 2018 15:38:40 -0500 Subject: [PATCH] FEATURE: Staff members can lock posts Locking a post prevents it from being edited. This is useful if the user has posted something which has been edited out, and the staff members don't want them to be able to edit it back in again. --- .../discourse/controllers/topic.js.es6 | 8 +++ .../discourse/lib/transform-post.js.es6 | 1 + .../javascripts/discourse/templates/topic.hbs | 2 + .../discourse/widgets/post-admin-menu.js.es6 | 9 ++++ .../javascripts/discourse/widgets/post.js.es6 | 11 +++++ .../stylesheets/common/base/topic-post.scss | 3 ++ app/controllers/posts_controller.rb | 33 ++++++++++++- app/models/post.rb | 4 ++ app/models/user_history.rb | 8 ++- app/serializers/post_serializer.rb | 12 ++++- app/services/staff_action_logger.rb | 8 +++ config/locales/client.en.yml | 7 +++ config/routes.rb | 1 + .../20180125185717_add_locked_by_to_posts.rb | 5 ++ lib/guardian/post_guardian.rb | 9 ++++ lib/post_locker.rb | 23 +++++++++ spec/components/guardian_spec.rb | 11 +++++ spec/components/post_locker_spec.rb | 49 +++++++++++++++++++ spec/requests/posts_controller_spec.rb | 19 +++++++ 19 files changed, 218 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20180125185717_add_locked_by_to_posts.rb create mode 100644 lib/post_locker.rb create mode 100644 spec/components/post_locker_spec.rb diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6 index 2e351654e9e..49110ce83bf 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js.es6 +++ b/app/assets/javascripts/discourse/controllers/topic.js.es6 @@ -518,6 +518,14 @@ export default Ember.Controller.extend(BufferedContent, { this.send('changeOwner'); }, + lockPost(post) { + return post.updatePostField('locked', true); + }, + + unlockPost(post) { + return post.updatePostField('locked', false); + }, + grantBadge(post) { this.set("selectedPostIds", [post.id]); this.send('showGrantBadgeModal'); diff --git a/app/assets/javascripts/discourse/lib/transform-post.js.es6 b/app/assets/javascripts/discourse/lib/transform-post.js.es6 index ac81e64e682..03ac928c1fe 100644 --- a/app/assets/javascripts/discourse/lib/transform-post.js.es6 +++ b/app/assets/javascripts/discourse/lib/transform-post.js.es6 @@ -77,6 +77,7 @@ export function transformBasicPost(post) { cooked_hidden: !!post.cooked_hidden, expandablePost: false, replyCount: post.reply_count, + locked: post.locked }; _additionalAttributes.forEach(a => postAtts[a] = post[a]); diff --git a/app/assets/javascripts/discourse/templates/topic.hbs b/app/assets/javascripts/discourse/templates/topic.hbs index b8ffe6b5e96..1c9678a69c1 100644 --- a/app/assets/javascripts/discourse/templates/topic.hbs +++ b/app/assets/javascripts/discourse/templates/topic.hbs @@ -175,6 +175,8 @@ rebakePost=(action "rebakePost") changePostOwner=(action "changePostOwner") grantBadge=(action "grantBadge") + lockPost=(action "lockPost") + unlockPost=(action "unlockPost") unhidePost=(action "unhidePost") replyToPost=(action "replyToPost") toggleWiki=(action "toggleWiki") diff --git a/app/assets/javascripts/discourse/widgets/post-admin-menu.js.es6 b/app/assets/javascripts/discourse/widgets/post-admin-menu.js.es6 index e21317a74fb..d79580ae45a 100644 --- a/app/assets/javascripts/discourse/widgets/post-admin-menu.js.es6 +++ b/app/assets/javascripts/discourse/widgets/post-admin-menu.js.es6 @@ -73,6 +73,15 @@ export function buildManageButtons(attrs, currentUser) { action: 'grantBadge', className: 'grant-badge' }); + + const action = attrs.locked ? "unlock" : "lock"; + contents.push({ + icon: action, + label: `post.controls.${action}_post`, + action: `${action}Post`, + title: `post.controls.${action}_post_description`, + className: `${action}-post` + }); } if (attrs.canManage || attrs.canWiki) { diff --git a/app/assets/javascripts/discourse/widgets/post.js.es6 b/app/assets/javascripts/discourse/widgets/post.js.es6 index 471ca682933..29618627e69 100644 --- a/app/assets/javascripts/discourse/widgets/post.js.es6 +++ b/app/assets/javascripts/discourse/widgets/post.js.es6 @@ -7,6 +7,7 @@ import { h } from 'virtual-dom'; import DiscourseURL from 'discourse/lib/url'; import { dateNode } from 'discourse/helpers/node'; import { translateSize, avatarUrl, formatUsername } from 'discourse/lib/utilities'; +import hbs from 'discourse/widgets/hbs-compiler'; export function avatarImg(wanted, attrs) { const size = translateSize(wanted); @@ -139,6 +140,12 @@ createWidget('post-avatar', { } }); +createWidget('post-locked-indicator', { + tagName: 'div.post-info.post-locked', + template: hbs`{{d-icon "lock"}}`, + title: () => I18n.t("post.locked") +}); + createWidget('post-email-indicator', { tagName: 'div.post-info.via-email', @@ -207,6 +214,10 @@ createWidget('post-meta-data', { result.push(this.attach('post-email-indicator', attrs)); } + if (attrs.locked) { + result.push(this.attach('post-locked-indicator', attrs)); + } + if (attrs.version > 1 || attrs.wiki) { result.push(this.attach('post-edits-indicator', attrs)); } diff --git a/app/assets/stylesheets/common/base/topic-post.scss b/app/assets/stylesheets/common/base/topic-post.scss index bcf774d15a1..b10b4d18331 100644 --- a/app/assets/stylesheets/common/base/topic-post.scss +++ b/app/assets/stylesheets/common/base/topic-post.scss @@ -320,8 +320,11 @@ aside.quote { } .post-info { + &.via-email, &.whisper { line-height: $line-height-medium; + } + &.via-email, &.whisper, &.post-locked { margin-right: 5px; .d-icon { font-size: $font-0; diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index a091e7e69d0..639ee168c29 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -4,12 +4,34 @@ require_dependency 'post_destroyer' require_dependency 'post_merger' require_dependency 'distributed_memoizer' require_dependency 'new_post_result_serializer' +require_dependency 'post_locker' class PostsController < ApplicationController - before_action :ensure_logged_in, except: [:show, :replies, :by_number, :short_link, :reply_history, :replyIids, :revisions, :latest_revision, :expand_embed, :markdown_id, :markdown_num, :cooked, :latest, :user_posts_feed] + before_action :ensure_logged_in, except: [ + :show, + :replies, + :by_number, + :short_link, + :reply_history, + :replyIids, + :revisions, + :latest_revision, + :expand_embed, + :markdown_id, + :markdown_num, + :cooked, + :latest, + :user_posts_feed + ] - skip_before_action :preload_json, :check_xhr, only: [:markdown_id, :markdown_num, :short_link, :latest, :user_posts_feed] + skip_before_action :preload_json, :check_xhr, only: [ + :markdown_id, + :markdown_num, + :short_link, + :latest, + :user_posts_feed + ] def markdown_id markdown Post.find(params[:id].to_i) @@ -381,6 +403,13 @@ class PostsController < ApplicationController render_json_dump(result) end + def locked + post = find_post_from_params + locker = PostLocker.new(post, current_user) + params[:locked] === "true" ? locker.lock : locker.unlock + render_json_dump(locked: post.locked?) + end + def bookmark if params[:bookmarked] == "true" post = find_post_from_params diff --git a/app/models/post.rb b/app/models/post.rb index 855d770349c..7a8ce9a5731 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -735,6 +735,10 @@ class Post < ActiveRecord::Base UserActionCreator.log_post(self) end + def locked? + locked_by_id.present? + end + private def parse_quote_into_arguments(quote) diff --git a/app/models/user_history.rb b/app/models/user_history.rb index d2f7f25a947..980ba8728f0 100644 --- a/app/models/user_history.rb +++ b/app/models/user_history.rb @@ -63,7 +63,9 @@ class UserHistory < ActiveRecord::Base backup_download: 45, backup_destroy: 46, notified_about_get_a_room: 47, - change_name: 48) + change_name: 48, + post_locked: 49, + post_unlocked: 50) end # Staff actions is a subset of all actions, used to audit actions taken by staff users. @@ -104,7 +106,9 @@ class UserHistory < ActiveRecord::Base :activate_user, :change_readonly_mode, :backup_download, - :backup_destroy] + :backup_destroy, + :post_locked, + :post_unlocked] end def self.staff_action_ids diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index d756ee1dd31..3f1b2a7254d 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -69,7 +69,8 @@ class PostSerializer < BasicPostSerializer :is_auto_generated, :action_code, :action_code_who, - :last_wiki_edit + :last_wiki_edit, + :locked def initialize(object, opts) super(object, opts) @@ -354,6 +355,15 @@ class PostSerializer < BasicPostSerializer include_action_code? && action_code_who.present? end + def locked + true + end + + # Only show locked posts to the users who made the post and staff + def include_locked? + object.locked? && (yours || scope.is_staff?) + end + def last_wiki_edit object.revisions.last.updated_at end diff --git a/app/services/staff_action_logger.rb b/app/services/staff_action_logger.rb index eb5d87abf14..6f57021af12 100644 --- a/app/services/staff_action_logger.rb +++ b/app/services/staff_action_logger.rb @@ -95,6 +95,14 @@ class StaffActionLogger target_user_id: user.id)) end + def log_post_lock(post, opts = {}) + raise Discourse::InvalidParameters.new(:post) unless post && post.is_a?(Post) + UserHistory.create!(params(opts).merge( + action: UserHistory.actions[opts[:locked] ? :post_locked : :post_unlocked], + post_id: post.id) + ) + end + def log_site_setting_change(setting_name, previous_value, new_value, opts = {}) raise Discourse::InvalidParameters.new(:setting_name) unless setting_name.present? && SiteSetting.respond_to?(setting_name) UserHistory.create(params(opts).merge(action: UserHistory.actions[:change_site_setting], diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 69184d393fc..dc948cbbdb6 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1900,6 +1900,7 @@ en: other: "(post withdrawn by author, will be automatically deleted in %{count} hours unless flagged)" collapse: "collapse" expand_collapse: "expand/collapse" + locked: "a staff member has locked this post from being edited" gap: one: "view 1 hidden reply" other: "view {{count}} hidden replies" @@ -1981,6 +1982,10 @@ en: unhide: "Unhide" change_owner: "Change Ownership" grant_badge: "Grant Badge" + lock_post: "Lock Post" + lock_post_description: "prevent the poster from editing this post" + unlock_post: "Unlock Post" + unlock_post_description: "allow the poster to edit this post" actions: flag: 'Flag' @@ -3220,6 +3225,8 @@ en: backup_destroy: "destroy backup" reviewed_post: "reviewed post" custom_staff: "plugin custom action" + post_locked: "post locked" + post_unlocked: "post unlocked" screened_emails: title: "Screened Emails" description: "When someone tries to create a new account, the following email addresses will be checked and the registration will be blocked, or some other action performed." diff --git a/config/routes.rb b/config/routes.rb index f3cb50b6fbf..23d6447d866 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -475,6 +475,7 @@ Discourse::Application.routes.draw do put "post_type" put "rebake" put "unhide" + put "locked" get "replies" get "revisions/latest" => "posts#latest_revision" get "revisions/:revision" => "posts#revisions", constraints: { revision: /\d+/ } diff --git a/db/migrate/20180125185717_add_locked_by_to_posts.rb b/db/migrate/20180125185717_add_locked_by_to_posts.rb new file mode 100644 index 00000000000..ec832cdd94c --- /dev/null +++ b/db/migrate/20180125185717_add_locked_by_to_posts.rb @@ -0,0 +1,5 @@ +class AddLockedByToPosts < ActiveRecord::Migration[5.1] + def change + add_column :posts, :locked_by_id, :integer + end +end diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index bda12b7d947..da7411d1eb2 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -46,6 +46,10 @@ module PostGuardian !!result end + def can_lock_post?(post) + can_see_post?(post) && is_staff? + end + def can_defer_flags?(post) can_see_post?(post) && is_staff? && post end @@ -98,6 +102,9 @@ module PostGuardian return true if is_admin? + # Must be staff to edit a locked post + return false if post.locked? && !is_staff? + if is_staff? || @user.has_trust_level?(TrustLevel[4]) return can_create_post?(post.topic) end @@ -106,6 +113,7 @@ module PostGuardian return false end + if post.wiki && (@user.trust_level >= SiteSetting.min_trust_to_edit_wiki_post.to_i) return can_create_post?(post.topic) end @@ -114,6 +122,7 @@ module PostGuardian return false end + if is_my_own?(post) if post.hidden? return false if post.hidden_at.present? && diff --git a/lib/post_locker.rb b/lib/post_locker.rb new file mode 100644 index 00000000000..d78951231c4 --- /dev/null +++ b/lib/post_locker.rb @@ -0,0 +1,23 @@ +class PostLocker + def initialize(post, user) + @post, @user = post, user + end + + def lock + Guardian.new(@user).ensure_can_lock_post!(@post) + + Post.transaction do + @post.update_column(:locked_by_id, @user.id) + StaffActionLogger.new(@user).log_post_lock(@post, locked: true) + end + end + + def unlock + Guardian.new(@user).ensure_can_lock_post!(@post) + + Post.transaction do + @post.update_column(:locked_by_id, nil) + StaffActionLogger.new(@user).log_post_lock(@post, locked: false) + end + end +end diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index a849e68cb22..09d4dc042d4 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -2,6 +2,7 @@ require 'rails_helper'; require 'guardian' require_dependency 'post_destroyer' +require_dependency 'post_locker' describe Guardian do @@ -1091,6 +1092,11 @@ describe Guardian do expect(Guardian.new(post.user).can_edit?(post)).to be_truthy end + it 'returns false if you try to edit a locked post' do + post.locked_by_id = moderator.id + expect(Guardian.new(post.user).can_edit?(post)).to be_falsey + end + it "returns false if the post is hidden due to flagging and it's too soon" do post.hidden = true post.hidden_at = Time.now @@ -1164,6 +1170,11 @@ describe Guardian do expect(Guardian.new(moderator).can_edit?(post)).to be_truthy end + it 'returns true as a moderator, even if locked' do + post.locked_by_id = admin.id + expect(Guardian.new(moderator).can_edit?(post)).to be_truthy + end + it 'returns true as an admin' do expect(Guardian.new(admin).can_edit?(post)).to be_truthy end diff --git a/spec/components/post_locker_spec.rb b/spec/components/post_locker_spec.rb new file mode 100644 index 00000000000..dd1e0291a04 --- /dev/null +++ b/spec/components/post_locker_spec.rb @@ -0,0 +1,49 @@ +require 'rails_helper' +require_dependency 'post_locker' + +describe PostLocker do + let(:moderator) { Fabricate(:moderator) } + let(:post) { Fabricate(:post) } + + it "doesn't allow regular users to lock posts" do + expect { + PostLocker.new(post, post.user).lock + }.to raise_error(Discourse::InvalidAccess) + + expect(post).not_to be_locked + expect(post.locked_by_id).to be_blank + end + + it "doesn't allow regular users to unlock posts" do + PostLocker.new(post, moderator).lock + + expect { + PostLocker.new(post, post.user).lock + }.to raise_error(Discourse::InvalidAccess) + + expect(post).to be_locked + expect(post.locked_by_id).to eq(moderator.id) + end + + it "allows staff to lock and unlock posts" do + expect(post).not_to be_locked + expect(post.locked_by_id).to be_blank + + PostLocker.new(post, moderator).lock + expect(post).to be_locked + expect(post.locked_by_id).to eq(moderator.id) + expect(UserHistory.where( + acting_user_id: moderator.id, + action: UserHistory.actions[:post_locked] + ).exists?).to eq(true) + + PostLocker.new(post, moderator).unlock + expect(post).not_to be_locked + expect(post.locked_by_id).to be_blank + expect(UserHistory.where( + acting_user_id: moderator.id, + action: UserHistory.actions[:post_unlocked] + ).exists?).to eq(true) + end + +end diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 62256403bf0..6d290d0e1aa 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -199,4 +199,23 @@ RSpec.describe PostsController do end end end + + describe "#locked" do + before do + sign_in(Fabricate(:moderator)) + end + + it 'can lock and unlock the post' do + put "/posts/#{public_post.id}/locked.json", params: { locked: "true" } + expect(response).to be_success + public_post.reload + expect(public_post).to be_locked + + put "/posts/#{public_post.id}/locked.json", params: { locked: "false" } + expect(response).to be_success + public_post.reload + expect(public_post).not_to be_locked + end + end + end