Merge pull request #2035 from sul-dlss/tags_controller

Extract TagsController from ItemsController

app/controllers/items_controller.rb

@@ -15,7 +15,6 @@ class ItemsController < ApplicationController
     purge_object
     source_id
     catkey
-    tags
     update_rights
     embargo_update
     embargo_form
@@ -188,34 +187,6 @@ def catkey
     end
   end
 
-  def tags
-    current_tags = tags_client.list
-
-    if params[:add]
-      tags = params.slice(:new_tag1, :new_tag2, :new_tag3).values.reject(&:empty?)
-      tags_client.create(tags: tags) if tags.any?
-    end
-
-    if params[:del]
-      tag_to_delete = current_tags[params[:tag].to_i - 1]
-      raise 'failed to delete' unless tags_client.destroy(tag: tag_to_delete)
-    end
-
-    if params[:update]
-      count = 1
-      current_tags.each do |tag|
-        tags_client.update(current: tag, new: params["tag#{count}".to_sym])
-        count += 1
-      end
-    end
-
-    reindex
-    respond_to do |format|
-      msg = "Tags for #{params[:id]} have been updated!"
-      format.any { redirect_to solr_document_path(params[:id]), notice: msg }
-    end
-  end
-
   def purge_object
     if dor_lifecycle(@object, 'submitted')
       render status: :bad_request, plain: 'Cannot purge an object after it is submitted.'
@@ -319,15 +290,6 @@ def rights
     end
   end
 
-  def tags_ui
-    @pid = @object.pid
-    @tags = tags_client.list
-
-    respond_to do |format|
-      format.html { render layout: !request.xhr? }
-    end
-  end
-
   def catkey_ui
     respond_to do |format|
       format.html { render layout: !request.xhr? }
@@ -348,10 +310,6 @@ def source_id_ui
 
   private
 
-  def tags_client
-    Dor::Services::Client.object(@object.pid).administrative_tags
-  end
-
   # Filters
   def create_obj
     raise 'missing druid' unless params[:id]

app/controllers/tags_controller.rb

@@ -0,0 +1,54 @@
+# frozen_string_literal: true
+
+# This controller is responsible for managing tags on an item
+class TagsController < ApplicationController
+  def update
+    @object = Dor.find params[:item_id]
+    authorize! :manage_item, @object
+
+    current_tags = tags_client.list
+
+    if params[:add]
+      tags = params.slice(:new_tag1, :new_tag2, :new_tag3).values.reject(&:empty?)
+      tags_client.create(tags: tags) if tags.any?
+    end
+
+    if params[:del]
+      tag_to_delete = current_tags[params[:tag].to_i - 1]
+      raise 'failed to delete' unless tags_client.destroy(tag: tag_to_delete)
+    end
+
+    if params[:update]
+      count = 1
+      current_tags.each do |tag|
+        tags_client.update(current: tag, new: params["tag#{count}".to_sym])
+        count += 1
+      end
+    end
+
+    reindex
+    respond_to do |format|
+      msg = "Tags for #{params[:item_id]} have been updated!"
+      format.any { redirect_to solr_document_path(params[:item_id]), notice: msg }
+    end
+  end
+
+  def edit
+    @pid = params[:item_id]
+    @tags = tags_client.list
+
+    respond_to do |format|
+      format.html { render layout: !request.xhr? }
+    end
+  end
+
+  private
+
+  def tags_client
+    Dor::Services::Client.object(params[:item_id]).administrative_tags
+  end
+
+  def reindex
+    Argo::Indexer.reindex_pid_remotely(params[:item_id])
+  end
+end

app/presenters/buttons_presenter.rb

@@ -22,7 +22,7 @@ def initialize(ability:, solr_document:, object:)
            :workflow_service_published_path,
            :purge_item_path,
            :source_id_ui_item_path,
-           :tags_ui_item_path,
+           :edit_item_tags_path,
            :catkey_ui_item_path,
            :collection_ui_item_path,
            :item_content_type_path,
@@ -72,7 +72,7 @@ def buttons
     buttons << purge_button
 
     buttons << { url: source_id_ui_item_path(id: pid), label: 'Change source id' }
-    buttons << { url: tags_ui_item_path(id: pid), label: 'Edit tags' }
+    buttons << { url: edit_item_tags_path(item_id: pid), label: 'Edit tags' }
     if [Dor::Item, Dor::Set].any? { |clazz| object.is_a? clazz } # these only apply for items, sets and collections
       buttons << { url: catkey_ui_item_path(id: pid), label: 'Manage catkey' }
       buttons << { url: collection_ui_item_path(id: pid), label: 'Edit collections' }

app/views/tags/_edit.html.erb

@@ -0,0 +1,27 @@
+<%= form_with url: item_tags_path(item_id: @pid, update: 'true'), method: 'patch' do %>
+  <% @tags.each_with_index do |tag, count| %>
+    <div class="form-group">
+      <div class="input-group">
+        <input class="form-control tag" name="tag<%= count + 1 %>" id="tag<%= count + 1 %>" type="text" value="<%= tag %>">
+        <span class='input-group-btn'>
+          <%= link_to item_tags_path(item_id: @pid, del: 'true', tag: count + 1),
+                      method: 'patch',
+                      class: 'btn btn-default' do %>
+            <span class='glyphicon glyphicon-remove glyphicon text-danger'></span>
+          <% end %>
+        </span>
+      </div>
+    </div>
+  <% end %>
+  <button class='btn btn-primary'>Update</button>
+<% end %>
+<hr>
+<%= form_tag item_tags_path(item_id: @pid, add: 'true'), method: 'patch'  do %>
+  <div class='form-group'>
+    <label>Add a tag</label>
+    <input class="form-control" name="new_tag1" id="new_tag1" type="text" value="">
+    <input class="form-control" name="new_tag2" id="new_tag2" type="text" value="">
+    <input class="form-control" name="new_tag3" id="new_tag3" type="text" value="">
+  </div>
+  <button class='btn btn-primary'>Add</button>
+<% end %>

app/views/items/tags_ui.html.erb → app/views/tags/edit.html.erb

@@ -1,4 +1,4 @@
 <%= render BlacklightModalComponent.new do |component| %>
   <% component.with(:header, 'Update tags or delete a tag') %>
-  <% component.with(:body) { render 'tags_ui' } %>
+  <% component.with(:body) { render 'edit' } %>
 <% end %>

config/routes.rb

@@ -115,6 +115,8 @@
       end
     end
 
+    resource :tags, only: %i[edit update]
+
     member do
       get 'purl_preview'
       post 'refresh_metadata'
@@ -124,9 +126,7 @@
       get 'embargo_form'
       post 'datastream', action: :datastream_update, as: 'datastream_update'
       get 'source_id_ui'
-      get 'tags_ui'
       get 'catkey_ui'
-      match 'tags', via: %i[get post]
       get 'collection_ui'
       get 'collection/delete',   action: :remove_collection, as: 'remove_collection'
       post 'collection/add',     action: :add_collection,    as: 'add_collection'

spec/controllers/items_controller_spec.rb

@@ -188,40 +188,6 @@
     end
   end
 
-  describe '#tags' do
-    context 'when they have manage access' do
-      let(:current_tag) { 'Some : Thing' }
-      let(:fake_tags_client) do
-        instance_double(Dor::Services::Client::AdministrativeTags,
-                        list: [current_tag],
-                        update: true,
-                        destroy: true,
-                        create: true)
-      end
-
-      before do
-        allow(controller).to receive(:authorize!).and_return(true)
-        allow(controller).to receive(:tags_client).and_return(fake_tags_client)
-        expect(Argo::Indexer).to receive(:reindex_pid_remotely)
-      end
-
-      it 'updates tags' do
-        expect(fake_tags_client).to receive(:update).with(current: current_tag, new: 'Some : Thing : Else').once
-        post 'tags', params: { id: pid, update: 'true', tag1: 'Some : Thing : Else' }
-      end
-
-      it 'deletes tag' do
-        expect(fake_tags_client).to receive(:destroy).with(tag: current_tag).once
-        post 'tags', params: { id: pid, tag: '1', del: 'true' }
-      end
-
-      it 'adds a tag' do
-        expect(fake_tags_client).to receive(:create).with(tags: ['New : Thing'])
-        post 'tags', params: { id: pid, new_tag1: 'New : Thing', add: 'true' }
-      end
-    end
-  end
-
   describe '#set_rights' do
     it 'sets an item to dark' do
       expect(item).to receive(:read_rights=).with('dark')

spec/controllers/tags_controller_spec.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+RSpec.describe TagsController, type: :controller do
+  before do
+    sign_in user
+    allow(Dor).to receive(:find).with(pid).and_return(item)
+    allow(Argo::Indexer).to receive(:reindex_pid_remotely)
+  end
+
+  let(:pid) { 'druid:bc123df4567' }
+  let(:item) { Dor::Item.new pid: pid }
+  let(:user) { create(:user) }
+
+  describe '#update' do
+    context 'when they have manage access' do
+      let(:current_tag) { 'Some : Thing' }
+      let(:tags_client) do
+        instance_double(Dor::Services::Client::AdministrativeTags,
+                        list: [current_tag],
+                        update: true,
+                        destroy: true,
+                        create: true)
+      end
+
+      before do
+        allow(controller).to receive(:authorize!).and_return(true)
+        allow(controller).to receive(:tags_client).and_return(tags_client)
+      end
+
+      it 'updates tags' do
+        post :update, params: { item_id: pid, update: 'true', tag1: 'Some : Thing : Else' }
+        expect(tags_client).to have_received(:update).with(current: current_tag, new: 'Some : Thing : Else')
+        expect(Argo::Indexer).to have_received(:reindex_pid_remotely)
+      end
+
+      it 'deletes tag' do
+        post :update, params: { item_id: pid, tag: '1', del: 'true' }
+        expect(tags_client).to have_received(:destroy).with(tag: current_tag)
+        expect(Argo::Indexer).to have_received(:reindex_pid_remotely)
+      end
+
+      it 'adds a tag' do
+        post :update, params: { item_id: pid, new_tag1: 'New : Thing', add: 'true' }
+        expect(Argo::Indexer).to have_received(:reindex_pid_remotely)
+        expect(tags_client).to have_received(:create).with(tags: ['New : Thing'])
+      end
+    end
+  end
+end

spec/features/apo_displays_spec.rb

@@ -109,10 +109,7 @@
 
     context 'tag ui' do
       it 'renders the tag ui' do
-        idmd = double(Dor::IdentityMetadataDS)
-        allow(Dor::Item).to receive(:identityMetadata).and_return(idmd)
-        allow(idmd).to receive(:tags).and_return(['something:123'])
-        visit '/items/druid:zt570tx3016/tags_ui'
+        visit '/items/druid:zt570tx3016/tags/edit'
         expect(page).to have_content('Update tags')
       end
     end

spec/presenters/buttons_presenter_spec.rb

@@ -96,7 +96,7 @@
           },
           {
             label: 'Edit tags',
-            url: "/items/#{item_id}/tags_ui"
+            url: "/items/#{item_id}/tags/edit"
           },
           {
             label: 'Edit collections',
@@ -246,7 +246,7 @@
           },
           {
             label: 'Edit tags',
-            url: "/items/#{view_apo_id}/tags_ui"
+            url: "/items/#{view_apo_id}/tags/edit"
           },
           {
             label: 'Manage release',

spec/views/items/_tags_ui.html.erb_spec.rb → spec/views/tags/_edit.html.erb_spec.rb

@@ -2,7 +2,7 @@
 
 require 'rails_helper'
 
-RSpec.describe 'items/_tags_ui.html.erb' do
+RSpec.describe 'tags/_edit.html.erb' do
   let(:pid) { 'druid:bc123df5678' }
   let(:tags) { ['Catz are awesome', 'Nice!'] }
 

spec/views/items/tags_ui.html.erb_spec.rb → spec/views/tags/edit.html.erb_spec.rb

@@ -2,9 +2,9 @@
 
 require 'rails_helper'
 
-RSpec.describe 'items/tags_ui.html.erb' do
+RSpec.describe 'tags/edit.html.erb' do
   it 'renders the JS template' do
-    stub_template 'items/_tags_ui.html.erb' => 'stubbed_tags_ui'
+    stub_template 'tags/_edit.html.erb' => 'stubbed_tags_ui'
     render
     expect(rendered)
       .to have_css '.modal-header h3.modal-title', text: 'Update tags or delete a tag'