FIX: Allow any type of post revision for staff alias topics (#86)

Currently, when a post is modified, and the post is created by the "staff alias user", we go through some validations like

is the user trying to edit a whisper? (not allowed)
is the post modification done by "a user who can use the staff alias (SiteSetting.staff_alias_allowed_groups)? (ok)
the modification is a wiki, post_type, user_id, title, tag -type modification.
When a new modification-type is missing, the user will face a 422 error.

This PR removes the criteria 3 to allow any modifications as long as the acting user is part of criteria 2. Related fixes in the past (73dffcd, 8762cc2, 291999b) were merely adding on to criteria 3 and it doesn't make sense long term.

plugin.rb

@@ -98,11 +98,10 @@
       end
     elsif self.post.user_id == SiteSetting.get(:staff_alias_user_id) &&
           User.human_user_id?(self.user_id)
-      if (self.modifications.keys & %w[wiki post_type user_id title tags]).present? &&
-           DiscourseStaffAlias.user_allowed?(self.user)
-        self.user_id = SiteSetting.get(:staff_alias_user_id)
-      else
+      if !DiscourseStaffAlias.user_allowed?(self.user)
         self.errors.add(:base, I18n.t("post_revisions.errors.cannot_edit_aliased_post_as_staff"))
+      else
+        self.user_id = SiteSetting.get(:staff_alias_user_id)
       end
     end
   end

spec/models/post_revision_spec.rb

@@ -50,7 +50,7 @@
     post = Fabricate(:post, user: DiscourseStaffAlias.alias_user)
     post_revision = Fabricate.build(:post_revision, post: post, user: Discourse.system_user)
 
-    expect(post_revision.valid?).to eq(true)
+    expect(post_revision).to be_valid
   end
 
   it "switches revision user to staff alias user when changing wiki status of post made as staff alias user" do
@@ -66,7 +66,7 @@
         },
       )
 
-    expect(post_revision.valid?).to eq(true)
+    expect(post_revision).to be_valid
     expect(post_revision.user_id).to eq(SiteSetting.get(:staff_alias_user_id))
   end
 
@@ -83,7 +83,7 @@
         },
       )
 
-    expect(post_revision.valid?).to eq(true)
+    expect(post_revision).to be_valid
     expect(post_revision.user_id).to eq(SiteSetting.get(:staff_alias_user_id))
   end
 
@@ -101,7 +101,7 @@
         },
       )
 
-    expect(post_revision.valid?).to eq(true)
+    expect(post_revision).to be_valid
     expect(post_revision.user_id).to eq(another_user.id)
   end
 
@@ -118,7 +118,7 @@
         },
       )
 
-    expect(post_revision.valid?).to eq(true)
+    expect(post_revision).to be_valid
   end
 
   it "allows tag revisions in topics by staff alias users" do
@@ -127,7 +127,23 @@
     post_revision =
       Fabricate.build(:post_revision, post: post, user: admin, modifications: { "tags" => "x" })
 
-    expect(post_revision.valid?).to eq(true)
+    expect(post_revision).to be_valid
+  end
+
+  it "allows category revisions in topics by staff alias users" do
+    post = Fabricate(:post, user: DiscourseStaffAlias.alias_user, post_type: Post.types[:regular])
+
+    post_revision =
+      Fabricate.build(
+        :post_revision,
+        post: post,
+        user: admin,
+        modifications: {
+          "category_id" => "x",
+        },
+      )
+
+    expect(post_revision).to be_valid
   end
 
   it "does not error out if no post revisions" do

spec/requests/posts_controller_spec.rb

@@ -168,36 +168,6 @@
       end.not_to change { post_1.post_revisions.count }
     end
 
-    it "does not allow a post by the alias user to be edited as staff user" do
-      sign_in(moderator)
-
-      post "/posts.json",
-           params: {
-             raw: "this is a post",
-             topic_id: post_1.topic_id,
-             reply_to_post_number: 1,
-             as_staff_alias: true,
-           }
-
-      post_2 = Post.last
-
-      expect do
-        put "/posts/#{post_2.id}.json",
-            params: {
-              post: {
-                raw: "new raw body",
-                edit_reason: "typo",
-              },
-            }
-
-        expect(response.status).to eq(422)
-
-        expect(response.parsed_body["errors"].first).to eq(
-          I18n.t("post_revisions.errors.cannot_edit_aliased_post_as_staff"),
-        )
-      end.not_to change { post_1.post_revisions.count }
-    end
-
     it "advances the draft sequence for the staff user" do
       sign_in(moderator)