From 4aed12cd1bffbcd88aedcf1be8a65f0e71365185 Mon Sep 17 00:00:00 2001
From: Matt Barnett <matt@sixtyodd.com>
Date: Wed, 31 Jan 2018 14:03:23 -0700
Subject: [PATCH 1/4] Working multiple community collection selection

---
 app/assets/javascripts/jupiter/item_draft.js  | 49 +++++++++++-
 app/assets/javascripts/jupiter/items.js       | 60 --------------
 app/controllers/admin/items_controller.rb     |  1 -
 app/controllers/items/draft_controller.rb     | 19 ++++-
 app/controllers/items_controller.rb           | 80 +------------------
 app/models/draft_item.rb                      | 28 ++++++-
 .../_community_collection_input.html.erb      | 27 -------
 app/views/items/_form.html.erb                | 63 ---------------
 .../_community_collection_input.html.erb      | 45 +++++++++++
 app/views/items/draft/describe_item.html.erb  | 50 ++++--------
 .../draft/review_and_deposit_item.html.erb    | 12 +--
 app/views/items/edit.html.erb                 |  6 --
 app/views/items/new.html.erb                  |  6 --
 config/locales/en.yml                         | 12 +--
 config/routes.rb                              |  3 +-
 .../items/draft_controller_test.rb            |  6 +-
 test/fixtures/draft_items.yml                 |  8 +-
 test/models/draft_item_test.rb                | 12 +--
 test/system/authentication_test.rb            |  6 +-
 test/system/deposit_item_test.rb              |  4 +-
 20 files changed, 183 insertions(+), 314 deletions(-)
 delete mode 100644 app/assets/javascripts/jupiter/items.js
 delete mode 100644 app/views/items/_community_collection_input.html.erb
 delete mode 100644 app/views/items/_form.html.erb
 create mode 100644 app/views/items/draft/_community_collection_input.html.erb
 delete mode 100644 app/views/items/edit.html.erb
 delete mode 100644 app/views/items/new.html.erb

diff --git a/app/assets/javascripts/jupiter/item_draft.js b/app/assets/javascripts/jupiter/item_draft.js
index bedd4d064..7a405d9df 100644
--- a/app/assets/javascripts/jupiter/item_draft.js
+++ b/app/assets/javascripts/jupiter/item_draft.js
@@ -3,6 +3,8 @@ var unsavedChanges = false;
 $(document).on('turbolinks:load', function() {
   unsavedChanges = false;
 
+  toggle_remove_visibility();
+
   $('#js-license-accordion .card').on('hidden.bs.collapse', toggleIcon);
   $('#js-license-accordion .card').on('shown.bs.collapse', toggleIcon);
 
@@ -23,10 +25,18 @@ $(document).on('turbolinks:load', function() {
     e.preventDefault();
   });
 
-  // bring over community/collection select from items (tweaked a bit)
-  // (we only need to handle one pairing for time being)
-  $('form.js-deposit-item .js-community-select').change(function() {
-    var $collectionSelect = $('.js-collection-select');
+  $('form.js-deposit-item .add-community-collection').click(function(e) {
+    e.preventDefault();
+    add_community_collection_input();
+  });
+
+  $('form.js-deposit-item').on('click', '.remove-community-collection', function() {
+      event.preventDefault();
+      remove_community_collection_input($(this));
+    });
+
+  $('form.js-deposit-item').on('change', '.js-community-select', function() {
+    var $collectionSelect = collection_select($(this));
     var id =  $(this).find('option:selected').val();
     if (!id) {
       $collectionSelect.prop('disabled', true).empty();
@@ -72,6 +82,37 @@ $(window).bind('beforeunload', function(event) {
   }
 });
 
+// Find collection select
+function collection_select($element) {
+  var $root = $element.hasClass('.community-collection') ? $element : $element.closest('.community-collection');
+  return $root.find('.js-collection-select');
+}
+
+function add_community_collection_input() {
+  var $new_input = $("div.community-collection").first().clone();
+  // Clear selections and disable collection select
+  $new_input.find('.js-community-select').val(null);
+  collection_select($new_input).attr('disabled', true).val(null);
+
+  $new_input.appendTo('.communities-collections-list');
+  toggle_remove_visibility();
+}
+
+function remove_community_collection_input($link) {
+  if ($('div.community-collection').length > 1) {
+    $link.closest('div.community-collection').remove();
+    toggle_remove_visibility();
+  }
+}
+
+function toggle_remove_visibility() {
+  if ($('div.community-collection').length > 1) {
+    $('.remove-community-collection').show();
+  } else {
+    $('.remove-community-collection').hide();
+  }
+}
+
 function toggleIcon(e) {
   $(e.target).prev('.card-header')
              .find('.js-more-less')
diff --git a/app/assets/javascripts/jupiter/items.js b/app/assets/javascripts/jupiter/items.js
deleted file mode 100644
index a68442fae..000000000
--- a/app/assets/javascripts/jupiter/items.js
+++ /dev/null
@@ -1,60 +0,0 @@
-$(document).on('turbolinks:load', function() {
-  $('form')
-    .on('change', '.community-select', function() {
-      update_collection_select($(this));
-    })
-    .on('click', '.add-community-collection', function() {
-      event.preventDefault();
-      add_community_collection_input();
-    })
-    .on('click', '.remove-community-collection', function() {
-      event.preventDefault();
-      remove_community_collection_input($(this));
-    })
-    .on('click', '.add-file', function(event){
-      event.preventDefault();
-      add_file_input();
-    });
-});
-
-function update_collection_select($community_select) {
-  var $collection_select = collection_select($community_select);
-  var id =  $community_select.find('option:selected').val();
-  // Bad value? Disable the collection select
-  if (!id) {
-    $collection_select.attr('disabled', true).val(null);
-    return;
-  }
-  $.getJSON('/communities/' + id + '.json').done(function(data) {
-    var items = "";
-    $.each(data.collections, function(idx, item) {
-      items += '<option value="' + item.id + '">' + item.title + '</option>';
-    });
-    $collection_select.empty().removeAttr('disabled').append(items);
-  });
-}
-
-// Find collection select
-function collection_select($element) {
-  var $root = $element.hasClass('.community-collection') ? $element : $element.closest('.community-collection');
-  return $root.find('.collection-select');
-}
-
-function add_community_collection_input() {
-  var $new_input = $("div.community-collection").first().clone();
-  // Clear selections and disable collection select
-  $new_input.find('.community-select').val(null);
-  collection_select($new_input).attr('disabled', true).val(null);
-
-  $new_input.appendTo('.communities-collections-list');
-}
-
-function remove_community_collection_input($link) {
-  if ($('div.community-collection').length > 1) {
-    $link.closest('div.community-collection').remove();
-  }
-}
-
-function add_file_input() {
-  $("div.item_file").first().clone().appendTo('.file_upload');
-}
diff --git a/app/controllers/admin/items_controller.rb b/app/controllers/admin/items_controller.rb
index 51d02fab3..7790d0f20 100644
--- a/app/controllers/admin/items_controller.rb
+++ b/app/controllers/admin/items_controller.rb
@@ -1,6 +1,5 @@
 class Admin::ItemsController < Admin::AdminController
 
-  # TODO: Implement me
   def index
     @items = Item.limit(10).sort(:record_created_at, :desc)
   end
diff --git a/app/controllers/items/draft_controller.rb b/app/controllers/items/draft_controller.rb
index f350c2c6b..e09dcf6cb 100644
--- a/app/controllers/items/draft_controller.rb
+++ b/app/controllers/items/draft_controller.rb
@@ -2,6 +2,8 @@ class Items::DraftController < ApplicationController
 
   include Wicked::Wizard
 
+  before_action :initialize_communities
+
   steps(*DraftItem.wizard_steps.keys.map(&:to_sym))
 
   def show
@@ -36,10 +38,15 @@ def update
     when :describe_item
       params[:draft_item][:status] = DraftItem.statuses[:active]
 
-      # TODO: Need to handle multiple community/collection pairs down the road
-      community_id = params[:draft_item].delete :community_id
-      collection_id = params[:draft_item].delete :collection_id
-      @draft_item.member_of_paths = { 'community_id': community_id, 'collection_id': collection_id }
+      @draft_item.member_of_paths = { community_id: [], collection_id: [] }
+      communities = params[:draft_item].delete :community_id
+      communities.each_with_index do |community_id, idx|
+        next if community_id.blank?
+        @draft_item.member_of_paths['community_id'] << community_id
+        collection_id = params[:draft_item]['collection_id'][idx]
+        @draft_item.member_of_paths['collection_id'] << collection_id
+      end
+      params[:draft_item].delete :collection_id
 
       @draft_item.update_attributes(permitted_attributes(DraftItem))
 
@@ -83,4 +90,8 @@ def destroy
     redirect_back(fallback_location: root_path, notice: t('.successful_deletion'))
   end
 
+  def initialize_communities
+    @communities = Community.all
+  end
+
 end
diff --git a/app/controllers/items_controller.rb b/app/controllers/items_controller.rb
index 6e8e7a56c..e699d94f9 100644
--- a/app/controllers/items_controller.rb
+++ b/app/controllers/items_controller.rb
@@ -1,85 +1,11 @@
 class ItemsController < ApplicationController
 
-  before_action :load_item, only: [:show, :edit, :update]
-  before_action :initialize_communities_and_collections, only: [:new, :edit]
-
-  def show; end
-
-  def new
-    # TODO: Remove for Draft Item Controller
-    @item = Item.new_locked_ldp_object
-    authorize @item
-  end
-
-  def create
-    # TODO: Remove for Draft Item Controller
-    communities = params[:item].delete :community
-    collections = params[:item].delete :collection
-
-    @item = Item.new_locked_ldp_object(permitted_attributes(Item))
-    authorize @item
-
-    # TODO: add validations?
-    @item.unlock_and_fetch_ldp_object do |unlocked_item|
-      unlocked_item.owner = current_user.id
-      unlocked_item.add_communities_and_collections(communities, collections)
-      unlocked_item.add_files(params[:item][:file])
-      unlocked_item.save!
-
-      if unlocked_item.save
-        redirect_to @item, notice: t('.created')
-      else
-        initialize_communities_and_collections
-        render :new, status: :bad_request
-      end
-    end
-  end
-
-  def edit
-    # TODO: Remove for Draft Item Controller
-  end
-
-  def update
-    # TODO: Remove for Draft Item Controller
-    authorize @item
-
-    communities = params[:item].delete :community
-    collections = params[:item].delete :collection
-
-    @item.unlock_and_fetch_ldp_object do |unlocked_item|
-      unlocked_item.update_attributes(permitted_attributes(@item))
-      unlocked_item.add_communities_and_collections(communities, collections)
-      unlocked_item.add_files(params[:item][:file])
-
-      if unlocked_item.save
-        redirect_to @item, notice: t('.updated')
-      else
-        initialize_communities_and_collections
-        render :edit, status: :bad_request
-      end
-    end
-  end
-
-  private
-
-  def load_item
+  def show
     @item = JupiterCore::LockedLdpObject.find(params[:id], types: [Item, Thesis])
     authorize @item
   end
 
-  def initialize_communities_and_collections
-    @communities = Community.all
-
-    @item_communities = []
-    @item_collections = []
-    @collection_choices = []
-
-    return if @item.nil?
-    @item.each_community_collection do |community, collection|
-      @item_communities << community
-      @item_collections << collection
-      @collection_choices << community.member_collections
-    end
-  end
+  # TODO: this is just a temp hack to give edit_item_path calls in templates somewhere to point until draft editing lands
+  def edit; end
 
 end
diff --git a/app/models/draft_item.rb b/app/models/draft_item.rb
index 0fec31893..af90acb12 100644
--- a/app/models/draft_item.rb
+++ b/app/models/draft_item.rb
@@ -54,6 +54,21 @@ class DraftItem < ApplicationRecord
 
   validates :files, presence: true, if: :validate_upload_files?
 
+  def communities
+    return unless member_of_paths.present? && member_of_paths['community_id']
+    member_of_paths['community_id'].map do |cid|
+      Community.find(cid)
+    end
+  end
+
+  def each_community_collection
+    return unless member_of_paths && member_of_paths['community_id'].present?
+    member_of_paths['community_id'].each_with_index do |community_id, idx|
+      collection_id = member_of_paths['collection_id'][idx]
+      yield Community.find(community_id), collection_id.present? ? Collection.find(collection_id) : nil
+    end
+  end
+
   def thumbnail
     if thumbnail_id.present?
       # TODO: Weird bug with activestorage when walking the assocation to all the attachments
@@ -124,7 +139,11 @@ def ingest_into_fedora
       source: source,
       related_link: related_item
     ).unlock_and_fetch_ldp_object do |unlocked_obj|
-      unlocked_obj.add_to_path(member_of_paths['community_id'], member_of_paths['collection_id'])
+
+      each_community_collection do |community, collection|
+        unlocked_obj.add_to_path(community.id, collection.id)
+      end
+
       unlocked_obj.add_files(map_activestorage_files_as_file_objects)
       unlocked_obj.save!
     end
@@ -137,10 +156,13 @@ def ingest_into_fedora
   # TODO: validate if community/collection ID's are actually in Fedora?
   def communities_and_collections_validations
     return if member_of_paths.blank? # caught by presence check
-    # member_of_paths.each do |path| # TODO eventually this will be an array of hashes
     errors.add(:member_of_paths, :community_not_found) if member_of_paths['community_id'].blank?
     errors.add(:member_of_paths, :collection_not_found) if member_of_paths['collection_id'].blank?
-    # end
+    return unless member_of_paths['community_id'].present? && member_of_paths['collection_id'].present?
+    member_of_paths['community_id'].each_with_index do |community_id, idx|
+      errors.add(:member_of_paths, :community_not_found) if community_id.blank?
+      errors.add(:member_of_paths, :collection_not_found) if member_of_paths['collection_id'][idx].blank?
+    end
   end
 
   def validate_describe_item?
diff --git a/app/views/items/_community_collection_input.html.erb b/app/views/items/_community_collection_input.html.erb
deleted file mode 100644
index 9aced4a18..000000000
--- a/app/views/items/_community_collection_input.html.erb
+++ /dev/null
@@ -1,27 +0,0 @@
-<div class="community-collection">
-  <div class="d-flex justify-content-between mt-3">
-    <div class="card p-3 w-100 mr-3">
-      <%= # The 'tap' prevents the first community from getting selected when no community_id
-          f.input :community,
-                  { as: :unread_select,
-                    collection: @communities,
-                    label_method: :title,
-                    value_method: :id,
-                    required: true,
-                    input_html: { name: 'item[community][]', id: nil, class: 'community-select' }
-                  }.tap { | options| options[:selected] = community_id if community_id }
-      %>
-      <%= f.input :collection,
-          as: :unread_select,
-          disabled: !collections.present?,
-          collection: collections,
-          selected: collection_id,
-          required: true,
-          input_html: { name: 'item[collection][]', id: nil, class: 'collection-select' }
-      %>
-    </div>
-    <div>
-      <%= link_to fa_icon('remove'), '#', title: t(:remove), class: 'btn btn-danger remove-community-collection' %>
-    </div>
-  </div>
-</div>
diff --git a/app/views/items/_form.html.erb b/app/views/items/_form.html.erb
deleted file mode 100644
index 182883b7b..000000000
--- a/app/views/items/_form.html.erb
+++ /dev/null
@@ -1,63 +0,0 @@
-<%= simple_form_for item, html: {multipart: true} do |f| %>
-  <% item.display_attributes.each do |prop, _| %>
-    <% next if ['visibility', 'owner', 'record_created_at', 'doi', 'embargo_end_date'].include? prop %>
-    <% if ['creator', 'contributor'].include? prop %>
-      <%# Multi-valued, handle it the easy way for now %>
-      <%= f.input prop, input_html: { multiple: true } %>
-    <% else %>
-      <%= f.input prop %>
-    <% end %>
-  <% end %>
-  <%= f.input :visibility,
-    as: :unread_select,
-    collection: Item::VISIBILITIES,
-    selected: item.visibility,
-    input_html: { name: 'item[visibility]' }
-  %>
-
-  <div class="communities-collections card p-3 mb-3">
-    <div class="card-block">
-      <%= f.label :member_of_paths, label: t('communities.title'),
-          class: 'card-title', required: true %>
-      <%= help_tooltip(t('.community_collection_tooltip')) %>
-      <%= f.error(:member_of_paths) %>
-      <div class="communities-collections-list">
-        <% if @item_communities.any? %>
-          <% @item_communities.each_with_index do |community, index| %>
-            <%= render partial: 'community_collection_input',
-                locals: { f: f, community_id: community.id, collection_id: @item_collections[index].id,
-                collections: @collection_choices[index] } %>
-          <% end %>
-        <% else %>
-          <%= render partial: 'community_collection_input',
-              locals: { f: f, community_id: nil, collection_id: nil, collections: [] } %>
-        <% end %>
-      </div>
-      <div class="d-flex flex-row-reverse">
-        <div class="p-3">
-          <%= link_to "#{fa_icon('plus')} #{t(:add_another)}".html_safe,
-              '#', class: 'btn btn-secondary add-community-collection' %>
-        </div>
-      </div>
-    </div>
-  </div>
-
-  <div class="file_upload">
-    <div class="row">
-      <div class="col-6">
-        <%= f.input :file,
-          as: :file,
-          label: t('items.file_label'),
-          input_html: {name: 'item[file][]'}
-        %>
-      </div>
-      <div class="col-6">
-        <%= link_to t('items.add_another_file'),
-          '#',
-          class: 'btn btn-secondary add-file'
-        %>
-      </div>
-    </div>
-  </div>
-  <%= f.button :submit %>
-<% end %>
diff --git a/app/views/items/draft/_community_collection_input.html.erb b/app/views/items/draft/_community_collection_input.html.erb
new file mode 100644
index 000000000..f69d529f0
--- /dev/null
+++ b/app/views/items/draft/_community_collection_input.html.erb
@@ -0,0 +1,45 @@
+<div class="community-collection">
+  <div class="form-group">
+    <label>
+      <span class="text-danger font-weight-bold">*</span>
+      <%= t('.community') %>
+    </label>
+
+    <%= select_tag 'draft_item[community_id][]',
+                    options_from_collection_for_select(
+                      Community.all,
+                      :id,
+                      :title,
+                      community.present? ? community.id : nil ),
+                    prompt: t('.community_placeholder'),
+                    class: 'form-control js-community-select' %>
+  </div>
+  <div class="form-group">
+    <label>
+      <span class="text-danger font-weight-bold">*</span>
+      <%= t('.collection') %>
+    </label>
+    <% if community.present? %>
+          <%= select_tag 'draft_item[collection_id][]',
+                         options_from_collection_for_select(
+                           community.member_collections,
+                           :id,
+                           :title,
+                           collection.present? ? collection.id : nil),
+                         prompt: t('.collection_placeholder'),
+                         data: { placeholder: t('.collection_placeholder')},
+                         class: 'form-control js-collection-select' %>
+    <% else %>
+      <%= select_tag 'draft_item[collection_id][]',
+                      nil,
+                      data: { placeholder: t('.collection_placeholder')},
+                      class: 'form-control js-collection-select',
+                      disabled: true %>
+    <% end %>
+    <div class="d-flex flex-row-reverse">
+      <div class="p-3">
+        <%= link_to fa_icon('remove'), '#', title: t(:remove), class: 'btn btn-danger remove-community-collection' %>
+      </div>
+    </div>
+  </div>
+</div>
diff --git a/app/views/items/draft/describe_item.html.erb b/app/views/items/draft/describe_item.html.erb
index c4f3f02e0..3a557c2c5 100644
--- a/app/views/items/draft/describe_item.html.erb
+++ b/app/views/items/draft/describe_item.html.erb
@@ -122,45 +122,25 @@
         <% end %>
       </ul>
     <% end %>
-    <div class="form-group">
-      <label>
-        <span class="text-danger font-weight-bold">*</span>
-        <%= t('.community') %>
-      </label>
 
-      <% @community = Community.find(@draft_item.member_of_paths['community_id']) if @draft_item.member_of_paths && @draft_item.member_of_paths['community_id'].present? %>
-      <%= select_tag 'draft_item[community_id]',
-                      options_from_collection_for_select(
-                        Community.all,
-                        :id,
-                        :title,
-                        @community&.id ),
-                      prompt: t('.community_placeholder'),
-                      class: 'form-control js-community-select' %>
-    </div>
-    <div class="form-group">
-      <label>
-        <span class="text-danger font-weight-bold">*</span>
-        <%= t('.collection') %>
-      </label>
-      <% if @community.present? %>
-            <%= select_tag 'draft_item[collection_id]',
-                           options_from_collection_for_select(
-                             @community.member_collections,
-                             :id,
-                             :title,
-                             @draft_item.member_of_paths['collection_id']),
-                           prompt: t('.collection_placeholder'),
-                           data: { placeholder: t('.collection_placeholder')},
-                           class: 'form-control js-collection-select' %>
+    <div class="communities-collections-list">
+      <% if @draft_item.communities.present? %>
+
+        <% @draft_item.each_community_collection do |community, collection| %>
+          <%= render partial: 'community_collection_input',
+              locals: { community: community, collection: collection } %>
+        <% end %>
       <% else %>
-        <%= select_tag 'draft_item[collection_id]',
-                        nil,
-                        data: { placeholder: t('.collection_placeholder')},
-                        class: 'form-control js-collection-select',
-                        disabled: true %>
+        <%= render partial: 'community_collection_input',
+            locals: { community: nil, collection: nil} %>
       <% end %>
     </div>
+    <div class="d-flex flex-row-reverse">
+      <div class="p-3">
+        <%= link_to "#{fa_icon('plus')} #{t(:add_another_community_collection)}".html_safe,
+            '#', class: 'btn btn-secondary add-community-collection' %>
+      </div>
+    </div>
   </fieldset>
 
   <div id="js-additional-fields-accordion" role="tablist">
diff --git a/app/views/items/draft/review_and_deposit_item.html.erb b/app/views/items/draft/review_and_deposit_item.html.erb
index 202f21d14..de9c67ac1 100644
--- a/app/views/items/draft/review_and_deposit_item.html.erb
+++ b/app/views/items/draft/review_and_deposit_item.html.erb
@@ -141,11 +141,13 @@
             <%=  t('items.draft.describe_item.community_and_collection') %>
           </h6>
           <ul>
-            <li>
-              <%= Community.find(@draft_item.member_of_paths['community_id']).title %>
-              /
-              <%= Collection.find(@draft_item.member_of_paths['collection_id']).title %>
-            </li>
+            <% @draft_item.each_community_collection do |community, collection| %>
+              <li>
+                <%= community.title %>
+                /
+                <%= collection.title %>
+              </li>
+            <% end %>
           </ul>
 
           <% if @draft_item.contributors.present? %>
diff --git a/app/views/items/edit.html.erb b/app/views/items/edit.html.erb
deleted file mode 100644
index 2aa49342d..000000000
--- a/app/views/items/edit.html.erb
+++ /dev/null
@@ -1,6 +0,0 @@
-<% page_title( t('.header')) %>
-
-<h1 class="mt-3"><%= t('.header') %></h1>
-<div id='item-form'>
-  <%= render partial: 'form', locals: {item: @item} %>
-</div>
diff --git a/app/views/items/new.html.erb b/app/views/items/new.html.erb
deleted file mode 100644
index 2aa49342d..000000000
--- a/app/views/items/new.html.erb
+++ /dev/null
@@ -1,6 +0,0 @@
-<% page_title( t('.header')) %>
-
-<h1 class="mt-3"><%= t('.header') %></h1>
-<div id='item-form'>
-  <%= render partial: 'form', locals: {item: @item} %>
-</div>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 0db0baa71..8ce9a0a7b 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -33,7 +33,7 @@ en:
   site_name: 'ERA'
 
   # Generic terms
-  add_another: 'Add another'
+  add_another_community_collection: 'Add another Community/Collection'
   all: 'All'
   browse_all: 'Browse all'
   cancel: 'Cancel'
@@ -417,6 +417,12 @@ en:
       delete_confirm: 'Are you sure you want to delete draft item "%{title}"?'
       successful_deletion: 'Draft Item was successfully deleted!'
 
+      community_collection_input:
+        community: 'Community'
+        community_placeholder: 'Select a community'
+        collection: 'Collection'
+        collection_placeholder: 'Select a collection'
+
       describe_item:
         header: 'Describe Item'
         header_icon: 'pencil'
@@ -443,10 +449,6 @@ en:
         community_and_collection: Community and Collection
         community_and_collection_help_html: 'Choose the Community and Collection that best match your item(s).
           If you need a new collection, email <a href="mailto:erahelp@ualberta.ca">erahelp@ualberta.ca</a>'
-        community: 'Community'
-        community_placeholder: 'Select a community'
-        collection: 'Collection'
-        collection_placeholder: 'Select a collection'
         additional_fields: 'Additional fields'
         contributors: 'Additional contributors'
         contributors_placeholder: 'Enter one or multiple contributors'
diff --git a/config/routes.rb b/config/routes.rb
index 065daaf1a..661b2040c 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -2,7 +2,8 @@
 require_dependency 'admin_constraint'
 
 Rails.application.routes.draw do
-  resources :items do
+  post '/items/create_draft' => 'items/draft#create'
+  resources :items, only: [:show, :edit] do
     collection do
       post :create_draft, controller: 'items/draft', action: :create
     end
diff --git a/test/controllers/items/draft_controller_test.rb b/test/controllers/items/draft_controller_test.rb
index caf4b7dfe..c22967607 100644
--- a/test/controllers/items/draft_controller_test.rb
+++ b/test/controllers/items/draft_controller_test.rb
@@ -61,8 +61,8 @@ class Items::DraftControllerTest < ActionDispatch::IntegrationTest
           subjects: ['Best Seller', 'Adventure'],
           date_created: Date.current,
           description: 'Really random description about this random book',
-          community_id: 'random-uuid-123',
-          collection_id: 'random-uuid-abc'
+          community_id: ['random-uuid-123'],
+          collection_id: ['random-uuid-abc']
         }
       }
 
@@ -155,7 +155,7 @@ class Items::DraftControllerTest < ActionDispatch::IntegrationTest
 
       draft_item = draft_items(:completed_choose_license_and_visibility_step)
 
-      draft_item.member_of_paths = { 'community_id': community.id, 'collection_id': collection.id }
+      draft_item.member_of_paths = { 'community_id': [community.id], 'collection_id': [collection.id] }
 
       file_fixture = fixture_file_upload('/files/image-sample.jpeg', 'image/jpeg')
       image_file = ActiveStorage::Blob.create_after_upload!(
diff --git a/test/fixtures/draft_items.yml b/test/fixtures/draft_items.yml
index 413590a30..8746a3915 100644
--- a/test/fixtures/draft_items.yml
+++ b/test/fixtures/draft_items.yml
@@ -18,8 +18,8 @@ completed_describe_item_step:
     romance, and adventure fill the pages of this magnificent saga, the first volume in an epic
     series sure to delight fantasy fans everywhere.'
   member_of_paths: {
-    community_id: 'random-uuid-for-book-community-123',
-    collection_id: 'random-uuid-for-fantasy-books-collection-abc'
+    community_id: ['random-uuid-for-book-community-123'],
+    collection_id: ['random-uuid-for-fantasy-books-collection-abc']
   }
 
 completed_choose_license_and_visibility_step:
@@ -38,8 +38,8 @@ completed_choose_license_and_visibility_step:
      A Clash of Kings transports us to a world of revelry and revenge,
      wizardry and warfare unlike any you have ever experienced.'
   member_of_paths: {
-    community_id: 'random-uuid-for-books-community-123',
-    collection_id: 'random-uuid-for-fantasy-books-collection-abc'
+    community_id: ['random-uuid-for-books-community-123'],
+    collection_id: ['random-uuid-for-fantasy-books-collection-abc']
   }
   license: <%= DraftItem.licenses[:license_text] %>
   license_text_area: 'Copyright license goes here'
diff --git a/test/models/draft_item_test.rb b/test/models/draft_item_test.rb
index c31e046db..f5897f7d1 100644
--- a/test/models/draft_item_test.rb
+++ b/test/models/draft_item_test.rb
@@ -43,7 +43,7 @@ class DraftItemTest < ActiveSupport::TestCase
         subjects: ['Best Seller', 'Adventure'],
         date_created: Date.current,
         description: 'Really random description about this random book',
-        member_of_paths: { community_id: 'random-uuid-123', collection_id: 'random-uuid-abc' }
+        member_of_paths: { community_id: ['random-uuid-123'], collection_id: ['random-uuid-abc'] }
       )
       assert draft_item.valid?
     end
@@ -62,7 +62,7 @@ class DraftItemTest < ActiveSupport::TestCase
         subjects: ['Best Seller', 'Adventure'],
         date_created: Date.current,
         description: 'Really random description about this random book',
-        member_of_paths: { community_id: 'random-uuid-123', collection_id: 'random-uuid-abc' },
+        member_of_paths: { community_id: ['random-uuid-123'], collection_id: ['random-uuid-abc'] },
         # technically a draft_item is valid in this wizard step already, since license/visibility are given defaults
         # but this case let's force the validation to fail
         license: nil,
@@ -96,7 +96,7 @@ class DraftItemTest < ActiveSupport::TestCase
         subjects: ['Best Seller', 'Adventure'],
         date_created: Date.current,
         description: 'Really random description about this random book',
-        member_of_paths: { community_id: 'random-uuid-123', collection_id: 'random-uuid-abc' }
+        member_of_paths: { community_id: ['random-uuid-123'], collection_id: ['random-uuid-abc'] }
       )
       refute draft_item.valid?
 
@@ -125,7 +125,7 @@ class DraftItemTest < ActiveSupport::TestCase
         subjects: ['Best Seller', 'Adventure'],
         date_created: Date.current,
         description: 'Really random description about this random book',
-        member_of_paths: { community_id: 'random-uuid-123', collection_id: 'random-uuid-abc' },
+        member_of_paths: { community_id: ['random-uuid-123'], collection_id: ['random-uuid-abc'] },
         license: DraftItem.licenses[:license_text]
       )
 
@@ -152,7 +152,7 @@ class DraftItemTest < ActiveSupport::TestCase
         subjects: ['Best Seller', 'Adventure'],
         date_created: Date.current,
         description: 'Really random description about this random book',
-        member_of_paths: { community_id: 'random-uuid-123', collection_id: 'random-uuid-abc' },
+        member_of_paths: { community_id: ['random-uuid-123'], collection_id: ['random-uuid-abc'] },
         visibility: DraftItem.visibilities[:embargo]
       )
 
@@ -187,7 +187,7 @@ class DraftItemTest < ActiveSupport::TestCase
       assert_equal "Collection can't be blank", draft_item.errors.messages[:member_of_paths].last
 
       draft_item.assign_attributes(
-        member_of_paths: { community_id: 'random-uuid-123', collection_id: 'random-uuid-abc' }
+        member_of_paths: { community_id: ['random-uuid-123'], collection_id: ['random-uuid-abc'] }
       )
       assert draft_item.valid?
     end
diff --git a/test/system/authentication_test.rb b/test/system/authentication_test.rb
index 425c29d42..59b9fdc7d 100644
--- a/test/system/authentication_test.rb
+++ b/test/system/authentication_test.rb
@@ -53,7 +53,9 @@ class AuthenticationTest < ApplicationSystemTestCase
 
   context 'when visiting a protected page' do
     should 'get redirected to login then back to page, if user is authorized' do
-      visit new_item_url
+      skip "there's no obvious place to send this now, as it won't redirect to the POST action for a draft item..."
+      visit root_url
+      click_link 'Deposit'
 
       assert_text I18n.t('authorization.user_not_authorized_try_logging_in')
       assert_selector 'h1', text: I18n.t('sessions.new.header')
@@ -74,7 +76,7 @@ class AuthenticationTest < ApplicationSystemTestCase
       assert_text I18n.t('login.success', kind: 'saml')
 
       # TODO: fix this view and i18n this
-      assert_text I18n.t('items.new.header')
+      assert_text I18n.t('items.draft.describe_item.header')
     end
 
     # TODO: Nothing currently exist to nicely test this behaviour, Comment this out for time being
diff --git a/test/system/deposit_item_test.rb b/test/system/deposit_item_test.rb
index 8990791aa..b61b7e8e6 100644
--- a/test/system/deposit_item_test.rb
+++ b/test/system/deposit_item_test.rb
@@ -43,8 +43,8 @@ def before_all
 
       fill_in I18n.t('items.draft.describe_item.description'), with: 'A Dance with Dragons Description Goes Here!!!'
 
-      select @community.title, from: 'draft_item[community_id]'
-      select @collection.title, from: 'draft_item[collection_id]'
+      select @community.title, from: 'draft_item[community_id][]'
+      select @collection.title, from: 'draft_item[collection_id][]'
 
       click_on I18n.t('items.draft.save_and_continue')
 

From e0ff616631cd579bb299ba5a38f492d3296c6d3d Mon Sep 17 00:00:00 2001
From: Matt Barnett <matt@sixtyodd.com>
Date: Wed, 31 Jan 2018 15:30:53 -0700
Subject: [PATCH 2/4] Remove a leftover hack

---
 config/routes.rb | 1 -
 1 file changed, 1 deletion(-)

diff --git a/config/routes.rb b/config/routes.rb
index 661b2040c..c3f2444ad 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -2,7 +2,6 @@
 require_dependency 'admin_constraint'
 
 Rails.application.routes.draw do
-  post '/items/create_draft' => 'items/draft#create'
   resources :items, only: [:show, :edit] do
     collection do
       post :create_draft, controller: 'items/draft', action: :create

From b74429952392d10b3138ecc840eb11f5f0007e36 Mon Sep 17 00:00:00 2001
From: Matt Barnett <matt@sixtyodd.com>
Date: Wed, 31 Jan 2018 16:17:52 -0700
Subject: [PATCH 3/4] Feedback

---
 app/assets/javascripts/jupiter/item_draft.js  | 20 +++++++++----------
 app/controllers/items/draft_controller.rb     |  2 +-
 .../_community_collection_input.html.erb      |  4 ++--
 app/views/items/draft/describe_item.html.erb  |  4 ++--
 test/system/authentication_test.rb            |  6 ++----
 tmp/.keep                                     |  0
 tmp/pids/.keep                                |  0
 7 files changed, 17 insertions(+), 19 deletions(-)
 delete mode 100644 tmp/.keep
 delete mode 100644 tmp/pids/.keep

diff --git a/app/assets/javascripts/jupiter/item_draft.js b/app/assets/javascripts/jupiter/item_draft.js
index 7a405d9df..668f86d40 100644
--- a/app/assets/javascripts/jupiter/item_draft.js
+++ b/app/assets/javascripts/jupiter/item_draft.js
@@ -25,12 +25,12 @@ $(document).on('turbolinks:load', function() {
     e.preventDefault();
   });
 
-  $('form.js-deposit-item .add-community-collection').click(function(e) {
+  $('form.js-deposit-item .js-add-community-collection').click(function(e) {
     e.preventDefault();
     add_community_collection_input();
   });
 
-  $('form.js-deposit-item').on('click', '.remove-community-collection', function() {
+  $('form.js-deposit-item').on('click', '.js-remove-community-collection', function() {
       event.preventDefault();
       remove_community_collection_input($(this));
     });
@@ -84,32 +84,32 @@ $(window).bind('beforeunload', function(event) {
 
 // Find collection select
 function collection_select($element) {
-  var $root = $element.hasClass('.community-collection') ? $element : $element.closest('.community-collection');
+  var $root = $element.hasClass('.js-community-collection') ? $element : $element.closest('.js-community-collection');
   return $root.find('.js-collection-select');
 }
 
 function add_community_collection_input() {
-  var $new_input = $("div.community-collection").first().clone();
+  var $new_input = $("div.js-community-collection").first().clone();
   // Clear selections and disable collection select
   $new_input.find('.js-community-select').val(null);
   collection_select($new_input).attr('disabled', true).val(null);
 
-  $new_input.appendTo('.communities-collections-list');
+  $new_input.appendTo('.js-communities-collections-list');
   toggle_remove_visibility();
 }
 
 function remove_community_collection_input($link) {
-  if ($('div.community-collection').length > 1) {
-    $link.closest('div.community-collection').remove();
+  if ($('div.js-community-collection').length > 1) {
+    $link.closest('div.js-community-collection').remove();
     toggle_remove_visibility();
   }
 }
 
 function toggle_remove_visibility() {
-  if ($('div.community-collection').length > 1) {
-    $('.remove-community-collection').show();
+  if ($('div.js-community-collection').length > 1) {
+    $('.js-remove-community-collection').show();
   } else {
-    $('.remove-community-collection').hide();
+    $('.js-remove-community-collection').hide();
   }
 }
 
diff --git a/app/controllers/items/draft_controller.rb b/app/controllers/items/draft_controller.rb
index e09dcf6cb..03383335c 100644
--- a/app/controllers/items/draft_controller.rb
+++ b/app/controllers/items/draft_controller.rb
@@ -2,7 +2,7 @@ class Items::DraftController < ApplicationController
 
   include Wicked::Wizard
 
-  before_action :initialize_communities
+  before_action :initialize_communities, only: [:show, :edit]
 
   steps(*DraftItem.wizard_steps.keys.map(&:to_sym))
 
diff --git a/app/views/items/draft/_community_collection_input.html.erb b/app/views/items/draft/_community_collection_input.html.erb
index f69d529f0..f0a3db055 100644
--- a/app/views/items/draft/_community_collection_input.html.erb
+++ b/app/views/items/draft/_community_collection_input.html.erb
@@ -1,4 +1,4 @@
-<div class="community-collection">
+<div class="js-community-collection">
   <div class="form-group">
     <label>
       <span class="text-danger font-weight-bold">*</span>
@@ -38,7 +38,7 @@
     <% end %>
     <div class="d-flex flex-row-reverse">
       <div class="p-3">
-        <%= link_to fa_icon('remove'), '#', title: t(:remove), class: 'btn btn-danger remove-community-collection' %>
+        <%= link_to fa_icon('remove'), '#', title: t(:remove), class: 'btn btn-danger js-remove-community-collection' %>
       </div>
     </div>
   </div>
diff --git a/app/views/items/draft/describe_item.html.erb b/app/views/items/draft/describe_item.html.erb
index 3a557c2c5..25ac797c6 100644
--- a/app/views/items/draft/describe_item.html.erb
+++ b/app/views/items/draft/describe_item.html.erb
@@ -123,7 +123,7 @@
       </ul>
     <% end %>
 
-    <div class="communities-collections-list">
+    <div class="js-communities-collections-list">
       <% if @draft_item.communities.present? %>
 
         <% @draft_item.each_community_collection do |community, collection| %>
@@ -138,7 +138,7 @@
     <div class="d-flex flex-row-reverse">
       <div class="p-3">
         <%= link_to "#{fa_icon('plus')} #{t(:add_another_community_collection)}".html_safe,
-            '#', class: 'btn btn-secondary add-community-collection' %>
+            '#', class: 'btn btn-secondary js-add-community-collection' %>
       </div>
     </div>
   </fieldset>
diff --git a/test/system/authentication_test.rb b/test/system/authentication_test.rb
index 59b9fdc7d..05cfa66b6 100644
--- a/test/system/authentication_test.rb
+++ b/test/system/authentication_test.rb
@@ -53,9 +53,7 @@ class AuthenticationTest < ApplicationSystemTestCase
 
   context 'when visiting a protected page' do
     should 'get redirected to login then back to page, if user is authorized' do
-      skip "there's no obvious place to send this now, as it won't redirect to the POST action for a draft item..."
-      visit root_url
-      click_link 'Deposit'
+      visit profile_url
 
       assert_text I18n.t('authorization.user_not_authorized_try_logging_in')
       assert_selector 'h1', text: I18n.t('sessions.new.header')
@@ -76,7 +74,7 @@ class AuthenticationTest < ApplicationSystemTestCase
       assert_text I18n.t('login.success', kind: 'saml')
 
       # TODO: fix this view and i18n this
-      assert_text I18n.t('items.draft.describe_item.header')
+      assert_text I18n.t('admin.users.created')
     end
 
     # TODO: Nothing currently exist to nicely test this behaviour, Comment this out for time being
diff --git a/tmp/.keep b/tmp/.keep
deleted file mode 100644
index e69de29bb..000000000
diff --git a/tmp/pids/.keep b/tmp/pids/.keep
deleted file mode 100644
index e69de29bb..000000000

From 8611cbb65ed403d34aa91df750d7ae59528535be Mon Sep 17 00:00:00 2001
From: Matt Barnett <matt@sixtyodd.com>
Date: Wed, 31 Jan 2018 16:20:18 -0700
Subject: [PATCH 4/4] COME BACK

---
 tmp/.keep      | 0
 tmp/pids/.keep | 0
 2 files changed, 0 insertions(+), 0 deletions(-)
 create mode 100644 tmp/.keep
 create mode 100644 tmp/pids/.keep

diff --git a/tmp/.keep b/tmp/.keep
new file mode 100644
index 000000000..e69de29bb
diff --git a/tmp/pids/.keep b/tmp/pids/.keep
new file mode 100644
index 000000000..e69de29bb