AO3-4954 Check for empty params when updating tags (otwcode#2841)

* AO3-4954 Check for empty params when updating tags

It's possible for an edit form to send back just the tag type
and the fix taggings count (e.g. changing the type of an unsorted tag),
leaving the controller to enforce strong parameters on an empty hash:

    "Required parameter missing: tag"

To avoid this, we only apply strong parameters if params is not empty.

* AO3-4954 Add a test for changing tag category to unsorted

app/controllers/tags_controller.rb

@@ -249,7 +249,9 @@ def update
       @tag = @tag.recategorize(new_tag_type)
     end
 
-    @tag.attributes = tag_params
+    unless params[:tag].empty?
+      @tag.attributes = tag_params
+    end
 
     @tag.syn_string = syn_string if @tag.save
 

features/tags_and_wrangling/tag_wrangling_unsorted.feature

@@ -47,3 +47,14 @@ Feature: Tag Wrangling - Unsorted Tags
      And the "Spike Spiegel" tag should be a "Character" tag
      And the "Annalise Keating & Bonnie Winterbottom" tag should be a "Relationship" tag
      And the "i love good omens" tag should be a "Freeform" tag
+
+  Scenario: Can return a tag to Unsorted after a different category has been set
+    Given I am logged in as a tag wrangler
+      And a unsorted_tag exists with name: "Unsorted Tag"
+    When I edit the tag "Unsorted Tag"
+      And I select "Fandom" from "tag_type"
+      And I press "Save changes"
+    Then I should see "Tag was updated."
+    When I select "UnsortedTag" from "tag_type"
+      And I press "Save changes"
+    Then I should see "Tag was updated."

spec/controllers/tags_controller_spec.rb

@@ -2,6 +2,7 @@
 
 describe TagsController do
   include LoginMacros
+  include RedirectExpectationHelper
 
   before do
     fake_login
@@ -151,22 +152,38 @@
 
   describe "update" do
     context "when updating a tag" do
-      before do
-        @tag = FactoryGirl.create(:freeform)
-      end
+      let(:tag) { create(:freeform) }
+      let(:unsorted_tag) { create(:unsorted_tag) }
 
-      it "should reset the taggings_count" do
+      it "resets the taggings count" do
         # manufacture a tag with borked taggings_count
-        @tag.taggings_count = 10
-        @tag.save
-        put :update, id: @tag.name, tag: { fix_taggings_count: true }
-        @tag.reload
-        expect(@tag.taggings_count).to eq(0)
+        tag.taggings_count = 10
+        tag.save
+
+        put :update, id: tag, tag: { fix_taggings_count: true }
+        it_redirects_to_with_notice edit_tag_path(tag), "Tag was updated."
+
+        tag.reload
+        expect(tag.taggings_count).to eq(0)
+      end
+
+      it "changes just the tag type" do
+        put :update, id: unsorted_tag, tag: { type: "Fandom" }, commit: "Save changes"
+        it_redirects_to_with_notice edit_tag_path(unsorted_tag), "Tag was updated."
+        expect(Tag.find(unsorted_tag.id).class).to eq(Fandom)
+
+        put :update, id: unsorted_tag, tag: { type: "UnsortedTag" }, commit: "Save changes"
+        it_redirects_to_with_notice edit_tag_path(unsorted_tag), "Tag was updated."
+        # The tag now has the original class, we can reload the original record without error.
+        unsorted_tag.reload
       end
 
-      it "you can wrangle" do
-        put :update, id: @tag.name, tag: {canonical: true}, commit: :Wrangle
-        expect(response).to redirect_to(tag_path(@tag) + "/wrangle?page=1&sort_column=name&sort_direction=ASC")
+      it "wrangles" do
+        expect(tag.canonical?).to be_truthy
+        put :update, id: tag, tag: { canonical: false }, commit: "Wrangle"
+        tag.reload
+        expect(tag.canonical?).to be_falsy
+        it_redirects_to wrangle_tag_path(tag, page: 1, sort_column: "name", sort_direction: "ASC")
       end
     end
   end