Re-rendering Work params for failed submit

Prior to this commit, when I would submit an invalid work create form,
the re-rendered form would have blank input values.  With this change
all non-file attributes are re-loaded with the submitted values.

Please note, this needs a bit more time to put full polish on, but I
want to make sure its a path worth pursuing.  If this is a reasonable
path, I'm happy to add the additional specs and inline comments to get
this working.

I'm presently dissatisfied with the changes I needed to make to the case
statement, and would appreciate guidance on how to correct that.

Related to #3978

app/controllers/concerns/hyrax/works_controller_behavior.rb

@@ -53,12 +53,17 @@ def new
     end
 
     def create
+      # Caching the original input params in case the form is not valid
+      original_input_params_for_form = params[hash_key_for_curation_concern].deep_dup
       if create_work
         after_create_response
       else
         respond_to do |wants|
           wants.html do
             build_form
+            # Creating a form object that can re-render most of the
+            # submitted parameters
+            @form = Hyrax::Forms::FailedSubmissionFormWrapper.new(form: @form, input_params: original_input_params_for_form, permitted_params: @form.permitted_params)
             render 'new', status: :unprocessable_entity
           end
           wants.json { render_json_response(response_type: :unprocessable_entity, options: { errors: curation_concern.errors }) }

app/forms/hyrax/forms/failed_submission_form_wrapper.rb

@@ -0,0 +1,114 @@
+# frozen_string_literal: true
+module Hyrax
+  module Forms
+    # @deprecated This class should be removed when we switch to
+    #   Valkyrie::ChangeSet instead of Hydra::Forms.
+    #
+    # This object is responsible for exposing the user submitted
+    # params from a failed POST/PUT/PATCH.
+    #
+    # @see https://github.com/samvera/hyrax/issues/3978
+    class FailedSubmissionFormWrapper < SimpleDelegator
+      # @param form
+      # @param input_params [ActionController::Parameters]
+      #
+      # @param permitted_params [Hash] this likely comes from a class
+      #        method on the given form.  It's used for enforcing
+      #        strong parameters.  It's more of a schema for what the
+      #        input parameters should be.  Why not rely on the
+      #        form.class.permitted_params?  This is a violation of
+      #        the Law of Demeter, and due to object delegation, the
+      #        form.class may be a delegate class, and not the one
+      #        that has permitted_params.
+      #
+      # @see Hyrax::WorkForm.build_permitted_params
+      def initialize(form:, input_params:, permitted_params:)
+        @form = form
+        @input_params = input_params
+        @permitted_params = permitted_params
+        @exposed_params = {}
+        super(@form)
+        build_exposed_params!
+      end
+
+      # Yes, I don't want to delegate class to the form, but based on
+      # past experiences, there are times where SimpleForm requests
+      # class.model_name and that had better align with the underlying
+      # form.
+      #
+      # Upon testing, when I don't have this method, the Javascript
+      # for "Requirements" on the new form will not properly
+      # acknowledge that we have re-filled the HTML form with the
+      # submitted non-file fields.
+      def class
+        @form.class
+      end
+
+      def model_name
+        @form.model_name
+      end
+
+      def to_model
+        self
+      end
+
+      def inspect
+        "Hyrax::Forms::FailedSubmissionFormWrapper(#{@form})"
+      end
+
+      def [](key)
+        if @exposed_params.key?(key)
+          @exposed_params.fetch(key)
+        else
+          super
+        end
+      end
+
+      private
+
+      def method_missing(method_name, *args, &block)
+        if @exposed_params.key?(method_name)
+          @exposed_params.fetch(method_name)
+        else
+          super
+        end
+      end
+
+      def respond_to_missing?(method_name, *args)
+        return true if @exposed_params.key?(method_name)
+        super
+      end
+
+      def build_exposed_params!
+        @permitted_params.each do |permitted_param|
+          case permitted_param
+          when ::Symbol
+            @exposed_params[permitted_param] = @input_params.fetch(permitted_param) if @input_params.key?(permitted_param)
+          when ::Hash
+            permitted_param.each do |key, value_schema|
+              build_exposed_param_hash_element!(key: key, value_schema: value_schema)
+            end
+          end
+        end
+      end
+
+      def build_exposed_param_hash_element!(key:, value_schema:)
+        # The input may not include the given key, so don't attempt a fetch.
+        return unless @input_params.key?(key)
+        # I don't have a non-Array example of what this value_schema could be.
+        return unless value_schema.is_a?(::Array)
+        if value_schema.empty?
+          @exposed_params[key] = ::Array.wrap(@input_params.fetch(key))
+        elsif value_schema.is_a?(::Array)
+          # We're expecting nested attributes which will have the form:
+          # { "0" => <Hash with value_schema as keys>, "1" => <Hash with value_schema as keys> }
+          hash = {}
+          @input_params.fetch(key).each_pair do |nested_key, nested_value|
+            hash[nested_key] = nested_value.slice(*value_schema)
+          end
+          @exposed_params[key] = hash
+        end
+      end
+    end
+  end
+end

app/helpers/hyrax/embargo_helper.rb

@@ -20,11 +20,15 @@ def assets_with_deactivated_embargoes
     #
     # @return [Boolean] whether the resource has an embargo that is currently
     #   enforced (regardless of whether it has expired)
+    #
+    # @note Hyrax::Forms::Failedsubmissionformwrapper is a place
+    #   holder until we switch to Valkyrie::ChangeSet instead of Form
+    #   objects
     def embargo_enforced?(resource)
       case resource
       when Hydra::AccessControls::Embargoable
         !resource.embargo_release_date.nil?
-      when HydraEditor::Form
+      when HydraEditor::Form, Hyrax::Forms::FailedSubmissionFormWrapper
         embargo_enforced?(resource.model)
       when Valkyrie::ChangeSet
         Hyrax::EmbargoManager.new(resource: resource.model).enforced?

app/helpers/hyrax/lease_helper.rb

@@ -20,11 +20,15 @@ def assets_with_deactivated_leases
     #
     # @return [Boolean] whether the resource has an lease that is currently
     #   enforced (regardless of whether it has expired)
+    #
+    # @note Hyrax::Forms::Failedsubmissionformwrapper is a place
+    #   holder until we switch to Valkyrie::ChangeSet instead of Form
+    #   objects
     def lease_enforced?(resource)
       case resource
       when Hydra::AccessControls::Embargoable
         !resource.lease_expiration_date.nil?
-      when HydraEditor::Form
+      when HydraEditor::Form, Hyrax::Forms::FailedSubmissionFormWrapper
         lease_enforced?(resource.model)
       when Valkyrie::ChangeSet
         Hyrax::LeaseManager.new(resource: resource.model).enforced?

spec/forms/hyrax/forms/failed_submission_form_wrapper_spec.rb

@@ -0,0 +1,49 @@
+# frozen_string_literal: true
+RSpec.describe Hyrax::Forms::FailedSubmissionFormWrapper do
+  let(:form) { instance_double(Hyrax::Forms::WorkForm, **form_attributes) }
+  let(:form_attributes) { { title: ["Form's Title"], description: ["Form's description"] } }
+  # I would love to use Hyrax::Forms::WorkForm, but I encountered an
+  # exception when I do Hyrax::Forms::WorkForm.permitted_params.  In
+  # this spec, things fail, however in
+  # ./spec/forms/hyrax/forms/work_form_spec.rb it works.
+  let(:permitted_params) do
+    [
+      { title: [] },
+      :representative_id,
+      { description: [] },
+      { based_near_attributes: [:id, :_destroy], member_of_collections_attributes: [:id, :_destroy], work_members_attributes: [:id, :_destroy] },
+      { permissions_attributes: [:type, :name, :access, :id, :_destroy] }
+    ]
+  end
+  let(:input_params) do
+    ActionController::Parameters.new(
+      title: ["Title"],
+      representative_id: "123",
+      obviously_missing_attribute: "One",
+      member_of_collections_attributes: { "0" => { id: "1" }, "2" => { id: "2", _destroy: "1" } }
+    )
+  end
+
+  let(:wrapper) { described_class.new(form: form, input_params: input_params, permitted_params: permitted_params) }
+
+  describe "input param key is part of permitted params" do
+    it "exposes the given input params" do
+      expect(wrapper.representative_id).to eq(input_params.fetch(:representative_id))
+      expect(wrapper[:representative_id]).to eq(input_params.fetch(:representative_id))
+
+      expect(wrapper.title).to eq(input_params.fetch(:title))
+      expect(wrapper[:title]).to eq(input_params.fetch(:title))
+
+      expect(wrapper.member_of_collections_attributes).to eq(input_params.fetch(:member_of_collections_attributes))
+      expect(wrapper[:member_of_collections_attributes]).to eq(input_params.fetch(:member_of_collections_attributes))
+
+      expect { wrapper.obviously_missing_attribute }.to raise_error(NoMethodError)
+    end
+
+    it "delegates to the underlying form when an input param is not given" do
+      expect(wrapper.description).to eq(form.description)
+      allow(form).to receive(:[]).with(:description).and_return(form_attributes.fetch(:description))
+      expect(wrapper[:description]).to eq(form.description)
+    end
+  end
+end

spec/helpers/hyrax/embargo_helper_spec.rb

@@ -110,5 +110,24 @@
         end
       end
     end
+
+    context 'with a Hyrax::Forms::FailedSubmissionFormWrapper' do
+      let(:resource) { Hyrax::Forms::FailedSubmissionFormWrapper.new(form: form, input_params: {}, permitted_params: {}) }
+      let(:form) { Hyrax::GenericWorkForm.new(build(:work), ability, form_controller) }
+      let(:ability) { :FAKE_ABILITY }
+      let(:form_controller) { :FAKE_CONTROLLER }
+
+      it 'returns false' do
+        expect(embargo_enforced?(resource)).to be false
+      end
+
+      context 'when the wrapped work is under embargo' do
+        let(:form) { Hyrax::GenericWorkForm.new(build(:embargoed_work), ability, form_controller) }
+
+        it 'returns true' do
+          expect(embargo_enforced?(resource)).to be true
+        end
+      end
+    end
   end
 end

spec/helpers/hyrax/lease_helper_spec.rb

@@ -108,5 +108,24 @@
         end
       end
     end
+
+    context 'with a Hyrax::Forms::FailedSubmissionFormWrapper' do
+      let(:resource) { Hyrax::Forms::FailedSubmissionFormWrapper.new(form: form, input_params: {}, permitted_params: {}) }
+      let(:form) { Hyrax::GenericWorkForm.new(build(:work), ability, form_controller) }
+      let(:ability) { :FAKE_ABILITY }
+      let(:form_controller) { :FAKE_CONTROLLER }
+
+      it 'returns false' do
+        expect(lease_enforced?(resource)).to be false
+      end
+
+      context 'when the wrapped work is under embargo' do
+        let(:form) { Hyrax::GenericWorkForm.new(build(:leased_work), ability, form_controller) }
+
+        it 'returns true' do
+          expect(lease_enforced?(resource)).to be true
+        end
+      end
+    end
   end
 end