From 7d91155d8d591519f62c73e6549839ccda9fd67d Mon Sep 17 00:00:00 2001 From: dananji Date: Wed, 7 Dec 2022 09:12:49 -0500 Subject: [PATCH 01/26] Move collection create to page from modal for better UX --- .../admin/collections_controller.rb | 19 ++- app/views/admin/collections/index.html.erb | 109 +++++++++--------- app/views/admin/collections/new.html.erb | 18 ++- 3 files changed, 88 insertions(+), 58 deletions(-) diff --git a/app/controllers/admin/collections_controller.rb b/app/controllers/admin/collections_controller.rb index 21d24fc029..eec773a64f 100644 --- a/app/controllers/admin/collections_controller.rb +++ b/app/controllers/admin/collections_controller.rb @@ -97,10 +97,25 @@ def create subject: "New collection: #{@collection.name}" ).deliver_later end - render json: {id: @collection.id}, status: 200 + respond_to do |format| + format.html do + redirect_to @collection, notice: 'Collection was successfully created.' + end + format.json do + render json: @collection + end + end else logger.warn "Failed to create collection #{@collection.name rescue ''}: #{@collection.errors.full_messages}" - render json: {errors: ['Failed to create collection:']+@collection.errors.full_messages}, status: 422 + respond_to do |format| + format.html do + flash.now[:error] = @collection.errors.full_messages.to_sentence + end + format.json do + flash[:error] = "Failed to create collection #{@collection.name rescue ''}: #{@collection.errors.full_messages}" + render json: {errors: @collection.errors, flash: flash} + end + end end end diff --git a/app/views/admin/collections/index.html.erb b/app/views/admin/collections/index.html.erb index aa0b01d6c4..f61980cb71 100644 --- a/app/views/admin/collections/index.html.erb +++ b/app/views/admin/collections/index.html.erb @@ -16,69 +16,68 @@ Unless required by applicable law or agreed to in writing, software distributed <% @page_title = t('collections.title', :application_name => application_name) %> <% unless @collections.empty? %> -
-
-

My Collections

+
+
+

My Collections

+
+
+ <%= link_to('Create Collection'.html_safe, new_admin_collection_path) unless cannot? :create, Admin::Collection %> +
-
- <%= button_tag("Create Collection", class: 'btn btn-primary btn-large', data: {toggle:"modal", target:"#new_collection"}) unless cannot? :create, Admin::Collection %> -
-
- - - - - - - - - - - - <% @collections.to_a.each do |collection| %> - - - - - - - - <% end %> - -
TitleItemsManagersDescription
<%= link_to collection.name, admin_collection_path(collection.id) %> - <%= link_to(pluralize(collection.media_object_count, 'item'), search_catalog_path('f[collection_ssim][]' => collection.name)) %> - <% if collection.unpublished_media_object_count > 0 %> - <%=link_to("(#{collection.unpublished_media_object_count} unpublished)", search_catalog_path('f[collection_ssim][]' => collection.name, 'f[workflow_published_sim][]' => "Unpublished")) %> - <% end %> - <%= pluralize(collection.manager_count, 'manager') %> <%= collection.description&.truncate(100, separator: ' ') %> - <% if can?(:destroy, collection) %> -
- <%= link_to('Delete', remove_admin_collection_path(collection.id), class: 'btn btn-danger btn-sm')%> -
- <% else %> -   - <% end %> -
+ + + + + + + + + + + + <% @collections.to_a.each do |collection| %> + + + + + + + + <% end %> + +
TitleItemsManagersDescription
<%= link_to collection.name, admin_collection_path(collection.id) %> + <%= link_to(pluralize(collection.media_object_count, 'item'), search_catalog_path('f[collection_ssim][]' => collection.name)) %> + <% if collection.unpublished_media_object_count > 0 %> + <%=link_to("(#{collection.unpublished_media_object_count} unpublished)", search_catalog_path('f[collection_ssim][]' => collection.name, 'f[workflow_published_sim][]' => "Unpublished")) %> + <% end %> + <%= pluralize(collection.manager_count, 'manager') %> <%= collection.description&.truncate(100, separator: ' ') %> + <% if can?(:destroy, collection) %> +
+ <%= link_to('Delete', remove_admin_collection_path(collection.id), class: 'btn btn-danger btn-sm')%> +
+ <% else %> +   + <% end %> +
<% else %> -
-

You don't have any collections yet

+
+

You don't have any collections yet

- <% if can? :create, Admin::Collection %> -

Would you like to create one?

-
-

- <%= button_tag("Create Collection", class: 'btn btn-primary btn-large', data: {toggle:"modal", target:"#new_collection"}) %> -

- <% else %> -

You'll need to be assigned to one

- <% end %> + <% if can? :create, Admin::Collection %> +

Would you like to create one?

+
+

+ <%= button_tag("Create Collection", class: 'btn btn-primary btn-large', data: {toggle:"modal", target:"#new_collection"}) %> +

+ <% else %> +

You'll need to be assigned to one

+ <% end %> -
+
<% end %> <% @collection = Admin::Collection.new %> -<%= render "form", modal_title: "Create Collection" %> diff --git a/app/views/admin/collections/new.html.erb b/app/views/admin/collections/new.html.erb index 17bc376ab1..1147265a9a 100644 --- a/app/views/admin/collections/new.html.erb +++ b/app/views/admin/collections/new.html.erb @@ -13,4 +13,20 @@ Unless required by applicable law or agreed to in writing, software distributed specific language governing permissions and limitations under the License. --- END LICENSE_HEADER BLOCK --- %> -<%= render :partial => 'form', locals: { modal_title: 'Create Collection' } %> + +<% unless Avalon::ControlledVocabulary.vocabulary[:units] %> + <% raise Avalon::VocabularyNotFound.new "Units vocabulary not found." %> +<% end %> + +<%= bootstrap_form_for @collection, action: 'create' do |f| %> + <%= f.text_field :name %> + <% if @collection.new_record? || can?(:update_unit, @collection)%> + <%= f.select(:unit, Admin::Collection.units, {}, {:class => 'form-control'}) %> + <% end %> + <%= f.text_area :description, rows: 3 %> + <%= f.text_field :contact_email %> + <%= f.text_field :website_url %> + <%= f.text_field :website_label %> + + <%= f.submit class: 'btn btn-primary' %> +<% end %> From c82ef983cbd1df316b8fe65423a6502f26b39780 Mon Sep 17 00:00:00 2001 From: dananji Date: Wed, 7 Dec 2022 14:40:31 -0500 Subject: [PATCH 02/26] Fix failing tests, and display of error flash messages --- .../admin/collections_controller.rb | 6 +- app/views/admin/collections/index.html.erb | 9 ++- app/views/admin/collections/new.html.erb | 8 +- .../capybara_admin_collection_spec.rb | 80 +++++++++++++++++++ spec/features/capybara_navigation_spec.rb | 7 +- 5 files changed, 94 insertions(+), 16 deletions(-) create mode 100644 spec/features/capybara_admin_collection_spec.rb diff --git a/app/controllers/admin/collections_controller.rb b/app/controllers/admin/collections_controller.rb index eec773a64f..b24c4ea846 100644 --- a/app/controllers/admin/collections_controller.rb +++ b/app/controllers/admin/collections_controller.rb @@ -102,7 +102,7 @@ def create redirect_to @collection, notice: 'Collection was successfully created.' end format.json do - render json: @collection + render json: {id: @collection.id}, status: 200 end end else @@ -110,10 +110,10 @@ def create respond_to do |format| format.html do flash.now[:error] = @collection.errors.full_messages.to_sentence + render action: 'new' end format.json do - flash[:error] = "Failed to create collection #{@collection.name rescue ''}: #{@collection.errors.full_messages}" - render json: {errors: @collection.errors, flash: flash} + render json: { errors: ['Failed to create collection:']+@collection.errors.full_messages}, status: 422 end end end diff --git a/app/views/admin/collections/index.html.erb b/app/views/admin/collections/index.html.erb index f61980cb71..61f3d8644c 100644 --- a/app/views/admin/collections/index.html.erb +++ b/app/views/admin/collections/index.html.erb @@ -13,6 +13,11 @@ Unless required by applicable law or agreed to in writing, software distributed specific language governing permissions and limitations under the License. --- END LICENSE_HEADER BLOCK --- %> + +<% unless Avalon::ControlledVocabulary.vocabulary[:units] %> + <% raise Avalon::VocabularyNotFound.new "Units vocabulary not found." %> +<% end %> + <% @page_title = t('collections.title', :application_name => application_name) %> <% unless @collections.empty? %> @@ -21,7 +26,7 @@ Unless required by applicable law or agreed to in writing, software distributed

My Collections

- <%= link_to('Create Collection'.html_safe, new_admin_collection_path) unless cannot? :create, Admin::Collection %> + <%= link_to('Create Collection'.html_safe, new_admin_collection_path) unless cannot? :create, Admin::Collection %>
@@ -69,7 +74,7 @@ Unless required by applicable law or agreed to in writing, software distributed

Would you like to create one?

- <%= button_tag("Create Collection", class: 'btn btn-primary btn-large', data: {toggle:"modal", target:"#new_collection"}) %> + <%= link_to('Create Collection'.html_safe, new_admin_collection_path) %>

<% else %>

You'll need to be assigned to one

diff --git a/app/views/admin/collections/new.html.erb b/app/views/admin/collections/new.html.erb index 1147265a9a..7d7826e996 100644 --- a/app/views/admin/collections/new.html.erb +++ b/app/views/admin/collections/new.html.erb @@ -13,11 +13,9 @@ Unless required by applicable law or agreed to in writing, software distributed specific language governing permissions and limitations under the License. --- END LICENSE_HEADER BLOCK --- %> - -<% unless Avalon::ControlledVocabulary.vocabulary[:units] %> - <% raise Avalon::VocabularyNotFound.new "Units vocabulary not found." %> -<% end %> - +
+

New collection

+
<%= bootstrap_form_for @collection, action: 'create' do |f| %> <%= f.text_field :name %> <% if @collection.new_record? || can?(:update_unit, @collection)%> diff --git a/spec/features/capybara_admin_collection_spec.rb b/spec/features/capybara_admin_collection_spec.rb new file mode 100644 index 0000000000..3d58f0900c --- /dev/null +++ b/spec/features/capybara_admin_collection_spec.rb @@ -0,0 +1,80 @@ +# Copyright 2011-2022, The Trustees of Indiana University and Northwestern +# University. Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software distributed +# under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR +# CONDITIONS OF ANY KIND, either express or implied. See the License for the +# specific language governing permissions and limitations under the License. +# --- END LICENSE_HEADER BLOCK --- + +require 'rails_helper' + +describe 'Admin Collection' do + after { Warden.test_reset! } + it 'checks navigation when create new collection is accessed' do + user = FactoryBot.create(:administrator) + login_as user, scope: :user + visit '/' + click_link('Manage Content') + expect(page).to have_current_path('/admin/collections') + expect(page).to have_link('Create Collection') + click_link('Create Collection') + expect(page).to have_current_path('/admin/collections/new') + expect(page).to have_content('Name') + expect(page).to have_content('Description') + expect(page).to have_content('Unit') + expect(page).to have_content('Default Unit') + expect(page).to have_link('Cancel') + end + it 'is able to create a new collection' do + user = FactoryBot.create(:administrator) + login_as user, scope: :user + visit '/admin/collections/' + click_link('Create Collection') + fill_in('admin_collection_name', with: 'Test Collection') + select('Default Unit', from: 'admin_collection_unit') + click_on('Create Collection') + visit '/admin/collections' + expect(page).to have_content('Test Collection') + expect(page).to have_content('Title') + expect(page).to have_content('Items') + expect(page).to have_content('Managers') + expect(page).to have_content('Description') + expect(page).to have_link('Delete') + end + it 'is able to view collection by clicking on collection name' do + user = FactoryBot.create(:administrator) + login_as user, scope: :user + visit '/admin/collections' + click_link('Create Collection') + fill_in('admin_collection_name', with: 'Test Collection') + select('Default Unit', from: 'admin_collection_unit') + click_on('Create Collection') + visit '/admin/collections' + click_on('Test Collection') + expect(page).to have_content('Test Collection') + expect(page).to have_link('Create An Item') + expect(page).to have_link('List All Items') + expect(page).to have_button('Edit Collection Info') + expect(page).to have_content('Default Unit') + end + it 'is able to delete a collection' do + user = FactoryBot.create(:administrator) + login_as user, scope: :user + visit '/admin/collections' + click_link('Create Collection') + fill_in('admin_collection_name', with: 'Test Collection') + select('Default Unit', from: 'admin_collection_unit') + click_on('Create Collection') + visit '/admin/collections' + click_link('Delete') + expect(page).to have_content('Are you certain you want to remove the collection Test Collection?') + expect(page).to have_button('Yes, I am sure') + expect(page).to have_link('No, go back') + end +end diff --git a/spec/features/capybara_navigation_spec.rb b/spec/features/capybara_navigation_spec.rb index 831e2cf898..5b5a17b302 100644 --- a/spec/features/capybara_navigation_spec.rb +++ b/spec/features/capybara_navigation_spec.rb @@ -31,12 +31,7 @@ expect(page).to have_current_path('/admin/collections') expect(page).to have_content('Skip to main content') expect(page).to have_link('Selected Items (0)') - expect(page).to have_button('Create Collection') - expect(page).to have_content('Name') - expect(page).to have_content('Description') - expect(page).to have_content('Unit') - expect(page).to have_content('Default Unit') - expect(page).to have_link('Cancel') + expect(page).to have_link('Create Collection') end it 'checks naviagtion to Manage Groups' do user = FactoryBot.create(:administrator) From 5cf22150fb4ffce48688890324567d6188af020d Mon Sep 17 00:00:00 2001 From: dananji Date: Mon, 12 Dec 2022 15:51:38 -0500 Subject: [PATCH 03/26] Resize XML editor window to make space in the editor --- app/views/media_objects/_structure.html.erb | 6 +++--- .../assets/javascripts/xmleditor/jquery.xmleditor.js | 6 +++--- vendor/assets/stylesheets/jquery.xmleditor.css | 11 +++-------- 3 files changed, 9 insertions(+), 14 deletions(-) diff --git a/app/views/media_objects/_structure.html.erb b/app/views/media_objects/_structure.html.erb index 17aa5832c7..89a6813162 100644 --- a/app/views/media_objects/_structure.html.erb +++ b/app/views/media_objects/_structure.html.erb @@ -99,15 +99,15 @@ Unless required by applicable law or agreed to in writing, software distributed <% @masterFiles.each_with_index do |section, index| %> <% default_structure="" %> -