Optimistically lock the file_manager

app/assets/javascripts/curation_concerns/file_manager/sorting.es6

@@ -15,18 +15,14 @@ export default class SortManager {
   }
 
   persist() {
-    let params = {}
-    params[this.singular_class_name] = {
-      "ordered_member_ids": this.order
-    }
-    params["_method"] = "PATCH"
     this.element.addClass("pending")
     this.element.removeClass("success")
     this.element.removeClass("failure")
     let persisting = $.post(
       `/concern/${this.class_name}/${this.id}.json`,
-      params
-    ).done(() => {
+      this.params()
+    ).done((response) => {
+      this.element.data('version', response.version)
       this.element.data("current-order", this.order)
       this.element.addClass("success")
       this.element.removeClass("failure")
@@ -39,6 +35,16 @@ export default class SortManager {
     return persisting
   }
 
+  params() {
+    let params = {}
+    params[this.singular_class_name] = {
+      "version": this.version,
+      "ordered_member_ids": this.order
+    }
+    params["_method"] = "PATCH"
+    return params
+  }
+
   get_sort_position(item) {
     return this.element.children().index(item)
   }
@@ -88,6 +94,10 @@ export default class SortManager {
     ).toArray()
   }
 
+  get version() {
+    return this.element.data('version')
+  }
+
   get alpha_sort_button() {
     return $("*[data-action='alpha-sort-action']")
   }

app/controllers/concerns/curation_concerns/curation_concern_controller.rb

@@ -22,6 +22,11 @@ module CurationConcernController
     module ClassMethods
       def curation_concern_type=(curation_concern_type)
         load_and_authorize_resource class: curation_concern_type, instance_name: :curation_concern, except: [:show, :file_manager, :inspect_work]
+
+        # Load the fedora resource to get the etag.
+        # No need to authorize for the file manager, because it does authorization via the presenter.
+        load_resource class: curation_concern_type, instance_name: :curation_concern, only: :file_manager
+
         self._curation_concern_type = curation_concern_type
       end
 
@@ -104,7 +109,7 @@ def destroy
     end
 
     def file_manager
-      presenter
+      @form = Forms::FileManagerForm.new(curation_concern, current_ability)
     end
 
     def inspect_work

app/forms/curation_concerns/forms/file_manager_form.rb

@@ -0,0 +1,27 @@
+module CurationConcerns
+  module Forms
+    class FileManagerForm
+      include HydraEditor::Form
+      self.terms = []
+      delegate :id, :thumbnail_id, :representative_id, :to_s, to: :model
+      attr_reader :current_ability, :request
+      def initialize(work, ability)
+        super(work)
+        @current_ability = ability
+        @request = nil
+      end
+
+      def version
+        model.etag
+      end
+
+      delegate :member_presenters, to: :member_presenter_factory
+
+      private
+
+        def member_presenter_factory
+          MemberPresenterFactory.new(work, ability)
+        end
+    end
+  end
+end

app/models/concerns/curation_concerns/ability.rb

@@ -14,6 +14,7 @@ def curation_concerns_permissions
 
       # user can version if they can edit
       alias_action :versions, to: :update
+      alias_action :file_manager, to: :update
 
       if admin?
         admin_permissions

app/presenters/curation_concerns/member_presenter_factory.rb

@@ -0,0 +1,70 @@
+module CurationConcerns
+  # Creates the presenters of the members (member works and file sets) of a specific object
+  class MemberPresenterFactory
+    class_attribute :file_presenter_class, :work_presenter_class
+    # modify this attribute to use an alternate presenter class for the files
+    self.file_presenter_class = FileSetPresenter
+
+    # modify this attribute to use an alternate presenter class for the child works
+    self.work_presenter_class = WorkShowPresenter
+
+    def initialize(work, ability, request = nil)
+      @work = work
+      @current_ability = ability
+      @request = request
+    end
+
+    delegate :id, to: :@work
+    attr_reader :current_ability, :request
+
+    # @param [Array<String>] ids a list of ids to build presenters for
+    # @param [Class] presenter_class the type of presenter to build
+    # @return [Array<presenter_class>] presenters for the ordered_members (not filtered by class)
+    def member_presenters(ids = ordered_ids, presenter_class = composite_presenter_class)
+      PresenterFactory.build_presenters(ids, presenter_class, *presenter_factory_arguments)
+    end
+
+    # @return [Array<FileSetPresenter>] presenters for the orderd_members that are FileSets
+    def file_set_presenters
+      @file_set_presenters ||= member_presenters(ordered_ids & file_set_ids)
+    end
+
+    # @return [Array<WorkShowPresenter>] presenters for the ordered_members that are not FileSets
+    def work_presenters
+      @work_presenters ||= member_presenters(ordered_ids - file_set_ids, work_presenter_class)
+    end
+
+    private
+
+      # TODO: Extract this to ActiveFedora::Aggregations::ListSource
+      def ordered_ids
+        @ordered_ids ||= begin
+                           ActiveFedora::SolrService.query("proxy_in_ssi:#{id}",
+                                                           rows: 10_000,
+                                                           fl: "ordered_targets_ssim")
+                                                    .flat_map { |x| x.fetch("ordered_targets_ssim", []) }
+                         end
+      end
+
+      # These are the file sets that belong to this work, but not necessarily
+      # in order.
+      # Arbitrarily maxed at 10 thousand; had to specify rows due to solr's default of 10
+      def file_set_ids
+        @file_set_ids ||= begin
+                            ActiveFedora::SolrService.query("{!field f=has_model_ssim}FileSet",
+                                                            rows: 10_000,
+                                                            fl: ActiveFedora.id_field,
+                                                            fq: "{!join from=ordered_targets_ssim to=id}id:\"#{id}/list_source\"")
+                                                     .flat_map { |x| x.fetch(ActiveFedora.id_field, []) }
+                          end
+      end
+
+      def presenter_factory_arguments
+        [current_ability, request]
+      end
+
+      def composite_presenter_class
+        CompositePresenterFactory.new(file_presenter_class, work_presenter_class, ordered_ids & file_set_ids)
+      end
+  end
+end

app/presenters/curation_concerns/work_show_presenter.rb

@@ -4,17 +4,11 @@ class WorkShowPresenter
     include PresentsAttributes
     attr_accessor :solr_document, :current_ability, :request
 
-    class_attribute :collection_presenter_class, :file_presenter_class, :work_presenter_class
+    class_attribute :collection_presenter_class
 
     # modify this attribute to use an alternate presenter class for the collections
     self.collection_presenter_class = CollectionPresenter
 
-    # modify this attribute to use an alternate presenter class for the files
-    self.file_presenter_class = FileSetPresenter
-
-    # modify this attribute to use an alternate presenter class for the child works
-    self.work_presenter_class = self
-
     # Methods used by blacklight helpers
     delegate :has?, :first, :fetch, :export_formats, :export_as, to: :solr_document
 
@@ -41,11 +35,6 @@ def page_title
              :lease_expiration_date, :rights, :source, :thumbnail_id, :representative_id,
              to: :solr_document
 
-    # @return [Array<FileSetPresenter>] presenters for the orderd_members that are FileSets
-    def file_set_presenters
-      @file_set_presenters ||= member_presenters(ordered_ids & file_set_ids)
-    end
-
     def workflow
       @workflow ||= WorkflowPresenter.new(solr_document, current_ability)
     end
@@ -68,24 +57,6 @@ def representative_presenter
         end
     end
 
-    # @return [Array<WorkShowPresenter>] presenters for the ordered_members that are not FileSets
-    def work_presenters
-      @work_presenters ||= member_presenters(ordered_ids - file_set_ids, work_presenter_class)
-    end
-
-    # @param [Array<String>] ids a list of ids to build presenters for
-    # @param [Class] presenter_class the type of presenter to build
-    # @return [Array<presenter_class>] presenters for the ordered_members (not filtered by class)
-    def member_presenters(ids = ordered_ids, presenter_class = composite_presenter_class)
-      PresenterFactory.build_presenters(ids,
-                                        presenter_class,
-                                        *presenter_factory_arguments)
-    end
-
-    def composite_presenter_class
-      CompositePresenterFactory.new(file_presenter_class, work_presenter_class, ordered_ids & file_set_ids)
-    end
-
     # @return [Array<CollectionPresenter>] presenters for the collections that this work is a member of
     def collection_presenters
       PresenterFactory.build_presenters(in_collection_ids,
@@ -109,11 +80,9 @@ def export_as_ttl
       graph.dump(:ttl)
     end
 
-    private
+    delegate :member_presenters, :file_set_presenters, :work_presenters, to: :member_presenter_factory
 
-      def graph
-        GraphExporter.new(solr_document, request).fetch
-      end
+    private
 
       def presenter_factory_arguments
         [current_ability, request]
@@ -126,27 +95,12 @@ def in_collection_ids
                                  .map { |x| x.fetch(ActiveFedora.id_field) }
       end
 
-      # TODO: Extract this to ActiveFedora::Aggregations::ListSource
-      def ordered_ids
-        @ordered_ids ||= begin
-                           ActiveFedora::SolrService.query("proxy_in_ssi:#{id}",
-                                                           rows: 10_000,
-                                                           fl: "ordered_targets_ssim")
-                                                    .flat_map { |x| x.fetch("ordered_targets_ssim", []) }
-                         end
+      def member_presenter_factory
+        MemberPresenterFactory.new(solr_document, current_ability, request)
       end
 
-      # These are the file sets that belong to this work, but not necessarily
-      # in order.
-      # Arbitrarily maxed at 10 thousand; had to specify rows due to solr's default of 10
-      def file_set_ids
-        @file_set_ids ||= begin
-                            ActiveFedora::SolrService.query("{!field f=has_model_ssim}FileSet",
-                                                            rows: 10_000,
-                                                            fl: ActiveFedora.id_field,
-                                                            fq: "{!join from=ordered_targets_ssim to=id}id:\"#{id}/list_source\"")
-                                                     .flat_map { |x| x.fetch(ActiveFedora.id_field, []) }
-                          end
+      def graph
+        GraphExporter.new(solr_document, request).fetch
       end
   end
 end

app/views/curation_concerns/base/_file_manager_member_resource_options.html.erb

@@ -1,10 +1,10 @@
       <div class="form-group radio_buttons member_resource_options">
         <span class="radio">
-          <%= radio_button_tag "thumbnail_id", node.id, @presenter.thumbnail_id == node.id, id: "thumbnail_id_#{node.id}", class: "radio_buttons" %>
+          <%= radio_button_tag "thumbnail_id", node.id, @form.thumbnail_id == node.id, id: "thumbnail_id_#{node.id}", class: "radio_buttons" %>
           <%= label_tag "thumbnail_id_#{node.id}", "Thumbnail" %>
         </span>
         <span class="radio">
-          <%= radio_button_tag "representative_id", node.id, @presenter.representative_id == node.id, id: "representative_id_#{node.id}", class: "radio_buttons" %>
+          <%= radio_button_tag "representative_id", node.id, @form.representative_id == node.id, id: "representative_id_#{node.id}", class: "radio_buttons" %>
           <%= label_tag "representative_id_#{node.id}", "Representative Media" %>
         </span>
       </div>

app/views/curation_concerns/base/_file_manager_members.html.erb

@@ -1,5 +1,13 @@
-<ul id="sortable" data-id="<%= @presenter.id %>" data-class-name="<%= @presenter.model_name.plural %>" data-singular-class-name="<%= @presenter.model_name.singular %>" class="list-unstyled grid clearfix">
- <% @presenter.member_presenters.each do |member| %>
-   <%= render "file_manager_member", node: member %>
- <% end %>
-</ul>
+<%= content_tag :ul,
+                id: "sortable",
+                class: "list-unstyled grid clearfix",
+                data: {
+                  id: @form.id,
+                  "class-name" => @form.model_name.plural,
+                  "singular-class-name" => @form.model_name.singular,
+                  version: @form.version
+                } do %>
+  <% @form.member_presenters.each do |member| %>
+    <%= render "file_manager_member", node: member %>
+  <% end %>
+<% end %>

app/views/curation_concerns/base/_file_manager_resource_form.html.erb

@@ -1,5 +1,5 @@
 <div class="resource-form-container">
-  <%= simple_form_for [main_app, @presenter], remote: true, html: { id: 'resource-form', 'data-type': 'json' } do |f| %>
+  <%= simple_form_for [main_app, @form], remote: true, html: { id: 'resource-form', 'data-type': 'json' } do |f| %>
     <%= f.input :thumbnail_id, as: :hidden, input_html: { data: {member_link: 'thumbnail_id'}} %>
     <%= f.input :representative_id, as: :hidden, input_html: { data: {member_link: 'representative_id'}} %>
   <% end %>

app/views/curation_concerns/base/file_manager.html.erb

@@ -1,11 +1,11 @@
 <h1><%= t("file_manager.link_text") %></h1>
 <ul class="breadcrumb">
   <li>
-    Back to <%= link_to @presenter.to_s, [main_app, @presenter] %>
+    Back to <%= link_to @form.to_s, [main_app, @form] %>
   </li>
 </ul>
 
-<% if !@presenter.member_presenters.empty? %>
+<% if !@form.member_presenters.empty? %>
   <div data-action="file-manager">
     <div class="col-md-3" id="file-manager-tools">
       <h2>Toolbar</h2>

app/views/curation_concerns/base/show.json.jbuilder

@@ -1 +1,2 @@
-json.extract! @curation_concern, *[:id] + @curation_concern.class.fields.select {|f| ![:has_model].include? f}
+json.extract! @curation_concern, *[:id] + @curation_concern.class.fields.select {|f| ![:has_model].include? f}
+json.version @curation_concern.etag

spec/controllers/curation_concerns/generic_works_controller_spec.rb

@@ -341,7 +341,7 @@
     it "is successful" do
       get :file_manager, params: { id: work.id }
       expect(response).to be_success
-      expect(assigns(:presenter)).not_to be_blank
+      expect(assigns(:form)).not_to be_blank
     end
   end
 

spec/presenters/curation_concerns/member_presenter_factory_spec.rb

@@ -0,0 +1,25 @@
+require 'spec_helper'
+
+RSpec.describe CurationConcerns::MemberPresenterFactory do
+  describe "#file_set_presenters" do
+    describe "getting presenters from factory" do
+      let(:solr_document) { SolrDocument.new(attributes) }
+      let(:attributes) { {} }
+      let(:ability) { double }
+      let(:request) { double }
+      let(:factory) { described_class.new(solr_document, ability, request) }
+      let(:presenter_class) { double }
+      before do
+        allow(factory).to receive(:composite_presenter_class).and_return(presenter_class)
+        allow(factory).to receive(:ordered_ids).and_return(['12', '33'])
+        allow(factory).to receive(:file_set_ids).and_return(['33', '12'])
+      end
+
+      it "uses the set class" do
+        expect(CurationConcerns::PresenterFactory).to receive(:build_presenters)
+          .with(['12', '33'], presenter_class, ability, request)
+        factory.file_set_presenters
+      end
+    end
+  end
+end