diff --git a/app/controllers/concerns/hyrax/works_controller_behavior.rb b/app/controllers/concerns/hyrax/works_controller_behavior.rb index c5735eefd2..2bf6fe72e1 100644 --- a/app/controllers/concerns/hyrax/works_controller_behavior.rb +++ b/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 }) } diff --git a/app/forms/hyrax/forms/failed_submission_form_wrapper.rb b/app/forms/hyrax/forms/failed_submission_form_wrapper.rb new file mode 100644 index 0000000000..cf827bdf97 --- /dev/null +++ b/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" => , "1" => } + 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 diff --git a/app/helpers/hyrax/embargo_helper.rb b/app/helpers/hyrax/embargo_helper.rb index f4d06daa88..f784fc9c06 100644 --- a/app/helpers/hyrax/embargo_helper.rb +++ b/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? diff --git a/app/helpers/hyrax/lease_helper.rb b/app/helpers/hyrax/lease_helper.rb index ebb6f6abb0..d8bd502555 100644 --- a/app/helpers/hyrax/lease_helper.rb +++ b/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? diff --git a/spec/forms/hyrax/forms/failed_submission_form_wrapper_spec.rb b/spec/forms/hyrax/forms/failed_submission_form_wrapper_spec.rb new file mode 100644 index 0000000000..8e8b34b77d --- /dev/null +++ b/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 diff --git a/spec/helpers/hyrax/embargo_helper_spec.rb b/spec/helpers/hyrax/embargo_helper_spec.rb index 3a32c86e3c..d90ceb0c20 100644 --- a/spec/helpers/hyrax/embargo_helper_spec.rb +++ b/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 diff --git a/spec/helpers/hyrax/lease_helper_spec.rb b/spec/helpers/hyrax/lease_helper_spec.rb index cdf1c2ff18..be1edb0232 100644 --- a/spec/helpers/hyrax/lease_helper_spec.rb +++ b/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