From 283dc7dd2d7d609c90232171e75dbafc58c097e5 Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Thu, 13 Mar 2014 10:47:37 -0400 Subject: [PATCH] Trust level 4: add ability to edit any post and see edit history --- .../javascripts/discourse/models/post.js | 9 +--- .../discourse/templates/post.js.handlebars | 2 +- app/controllers/posts_controller.rb | 1 + app/serializers/post_serializer.rb | 7 +++- lib/guardian/post_guardian.rb | 11 ++++- spec/components/guardian_spec.rb | 41 ++++++++++++++++++- spec/controllers/posts_controller_spec.rb | 17 +++++++- spec/fabricators/user_fabricator.rb | 16 +++++++- 8 files changed, 88 insertions(+), 16 deletions(-) diff --git a/app/assets/javascripts/discourse/models/post.js b/app/assets/javascripts/discourse/models/post.js index dad1348dda8..0cb2f93ac48 100644 --- a/app/assets/javascripts/discourse/models/post.js +++ b/app/assets/javascripts/discourse/models/post.js @@ -350,14 +350,7 @@ Discourse.Post = Discourse.Model.extend({ var topic = this.get('topic'); return !topic.isReplyDirectlyBelow(this); - }.property('reply_count'), - - canViewEditHistory: function() { - return (Discourse.SiteSettings.edit_history_visible_to_public || - (Discourse.User.current() && - (Discourse.User.current().get('staff') || Discourse.User.current().get('id') === this.get('user_id')))); - }.property() - + }.property('reply_count') }); Discourse.Post.reopenClass({ diff --git a/app/assets/javascripts/discourse/templates/post.js.handlebars b/app/assets/javascripts/discourse/templates/post.js.handlebars index b6f3cbb2809..fca80bd430d 100644 --- a/app/assets/javascripts/discourse/templates/post.js.handlebars +++ b/app/assets/javascripts/discourse/templates/post.js.handlebars @@ -55,7 +55,7 @@
{{#if hasHistory}}
- {{#if canViewEditHistory}} + {{#if can_view_edit_history}} {{editCount}} diff --git a/app/controllers/posts_controller.rb b/app/controllers/posts_controller.rb index 42fb3016428..fa34151ca25 100644 --- a/app/controllers/posts_controller.rb +++ b/app/controllers/posts_controller.rb @@ -178,6 +178,7 @@ class PostsController < ApplicationController def revisions post_revision = find_post_revision_from_params + guardian.ensure_can_see!(post_revision) post_revision_serializer = PostRevisionSerializer.new(post_revision, scope: guardian, root: false) render_json_dump(post_revision_serializer) end diff --git a/app/serializers/post_serializer.rb b/app/serializers/post_serializer.rb index 0c168d5edcc..faedc38b1e8 100644 --- a/app/serializers/post_serializer.rb +++ b/app/serializers/post_serializer.rb @@ -45,7 +45,8 @@ class PostSerializer < BasicPostSerializer :deleted_at, :deleted_by, :user_deleted, - :edit_reason + :edit_reason, + :can_view_edit_history def moderator? @@ -200,6 +201,10 @@ class PostSerializer < BasicPostSerializer SiteSetting.enable_names? end + def can_view_edit_history + scope.can_view_post_revisions?(object) + end + private def post_actions diff --git a/lib/guardian/post_guardian.rb b/lib/guardian/post_guardian.rb index 4d2e4873ed5..27d55c6d305 100644 --- a/lib/guardian/post_guardian.rb +++ b/lib/guardian/post_guardian.rb @@ -69,7 +69,7 @@ module PostGuardain # Editing Method def can_edit_post?(post) - is_staff? || (!post.topic.archived? && is_my_own?(post) && !post.user_deleted && !post.deleted_at && !post.edit_time_limit_expired?) + is_staff? || @user.has_trust_level?(:elder) || (!post.topic.archived? && is_my_own?(post) && !post.user_deleted && !post.deleted_at && !post.edit_time_limit_expired?) end # Deleting Methods @@ -111,8 +111,15 @@ module PostGuardain def can_see_post_revision?(post_revision) return false if post_revision.nil? + can_view_post_revisions?(post_revision.post) + end + + def can_view_post_revisions?(post) + return false if post.nil? return true if SiteSetting.edit_history_visible_to_public - authenticated? && (is_staff? || can_see_post?(post_revision.post)) + authenticated? && + (is_staff? || @user.has_trust_level?(:elder) || @user.id == post.user_id) && + can_see_post?(post) end def can_vote?(post, opts={}) diff --git a/spec/components/guardian_spec.rb b/spec/components/guardian_spec.rb index 08b0457f403..bd79b9517f3 100644 --- a/spec/components/guardian_spec.rb +++ b/spec/components/guardian_spec.rb @@ -8,6 +8,7 @@ describe Guardian do let(:moderator) { build(:moderator) } let(:admin) { build(:admin) } let(:leader) { build(:user, trust_level: 3) } + let(:elder) { build(:user, trust_level: 4) } let(:another_admin) { build(:admin) } let(:coding_horror) { build(:coding_horror) } @@ -259,7 +260,6 @@ describe Guardian do end describe 'a Post' do - let(:another_admin) { Fabricate(:admin) } it 'correctly handles post visibility' do post = Fabricate(:post) @@ -279,8 +279,43 @@ describe Guardian do Guardian.new(user).can_see?(post).should be_false Guardian.new(admin).can_see?(post).should be_true end + end + describe 'a PostRevision' do + let(:post_revision) { Fabricate(:post_revision) } + context 'edit_history_visible_to_public is true' do + before { SiteSetting.stubs(:edit_history_visible_to_public).returns(true) } + + it 'is false for nil' do + Guardian.new.can_see?(nil).should be_false + end + + it 'is true if not logged in' do + Guardian.new.can_see?(post_revision).should == true + end + + it 'is true when logged in' do + Guardian.new(Fabricate(:user)).can_see?(post_revision).should == true + end + end + + context 'edit_history_visible_to_public is false' do + before { SiteSetting.stubs(:edit_history_visible_to_public).returns(false) } + + it 'is true for staff' do + Guardian.new(Fabricate(:admin)).can_see?(post_revision).should == true + Guardian.new(Fabricate(:moderator)).can_see?(post_revision).should == true + end + + it 'is true for trust level 4' do + Guardian.new(Fabricate(:elder)).can_see?(post_revision).should == true + end + + it 'is false for trust level lower than 4' do + Guardian.new(Fabricate(:leader)).can_see?(post_revision).should == false + end + end end end @@ -551,6 +586,10 @@ describe Guardian do Guardian.new(admin).can_edit?(post).should be_true end + it 'returns true as a trust level 4 user' do + Guardian.new(elder).can_edit?(post).should be_true + end + context 'post is older than post_edit_time_limit' do let(:old_post) { build(:post, topic: topic, user: topic.user, created_at: 6.minutes.ago) } before do diff --git a/spec/controllers/posts_controller_spec.rb b/spec/controllers/posts_controller_spec.rb index a01847cd211..459253ccfda 100644 --- a/spec/controllers/posts_controller_spec.rb +++ b/spec/controllers/posts_controller_spec.rb @@ -431,7 +431,13 @@ describe PostsController do before { SiteSetting.stubs(:edit_history_visible_to_public).returns(false) } - it "ensures anonymous can not see the revisions" do + it "ensures anonymous cannot see the revisions" do + xhr :get, :revisions, post_id: post_revision.post_id, revision: post_revision.number + response.should be_forbidden + end + + it "ensures regular user cannot see the revisions" do + u = log_in(:user) xhr :get, :revisions, post_id: post_revision.post_id, revision: post_revision.number response.should be_forbidden end @@ -444,11 +450,18 @@ describe PostsController do it "ensures poster can see the revisions" do user = log_in(:active_user) - pr = Fabricate(:post_revision, user: user) + post = Fabricate(:post, user: user) + pr = Fabricate(:post_revision, user: user, post: post) xhr :get, :revisions, post_id: pr.post_id, revision: pr.number response.should be_success end + it "ensures trust level 4 can see the revisions" do + log_in(:elder) + xhr :get, :revisions, post_id: post_revision.post_id, revision: post_revision.number + response.should be_success + end + end context "when edit history is visible to everyone" do diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index 48fc0c39837..686cdc7d120 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/spec/fabricators/user_fabricator.rb @@ -60,5 +60,19 @@ Fabricator(:active_user, from: :user) do password 'myawesomepassword' trust_level TrustLevel.levels[:basic] active true - bio_raw "Don't as me about my dad!" + bio_raw "Don't ask me about my dad!" end + +Fabricator(:leader, from: :user) do + name 'Leader McLeaderman' + username { sequence(:username) { |i| "leader#{i}" } } + email { sequence(:email) { |i| "leader#{i}@leaderfun.com" } } + trust_level TrustLevel.levels[:leader] +end + +Fabricator(:elder, from: :user) do + name 'Elder McElderson' + username { sequence(:username) { |i| "elder#{i}" } } + email { sequence(:email) { |i| "elder#{i}@elderfun.com" } } + trust_level TrustLevel.levels[:elder] +end \ No newline at end of file