Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hyrax::Forms::PcdmCollectionForm#prepopulate! fails on shared spec. #6702

Open
jeremyf opened this issue Feb 19, 2024 · 0 comments
Open

Hyrax::Forms::PcdmCollectionForm#prepopulate! fails on shared spec. #6702

jeremyf opened this issue Feb 19, 2024 · 0 comments

Comments

@jeremyf
Copy link
Contributor

jeremyf commented Feb 19, 2024

tl;dr Hyrax::Forms::PcdmCollectionForm changes on prepoluate but does not test it, and the collection generator creates a form that inherits from Hyrax::Forms::PcdmCollectionForm and tests (and fails) that the #prepopulate! method does not change the form.

Error demonstration:

On 715f599 (HEAD of main of of writing this issue), when I adjust the Hyrax::Forms::PcdmCollectionForm spec to use the it_behaves_like 'a Valkyrie::ChangeSet I get the following failed spec:

1) Hyrax::Forms::PcdmCollectionForm behaves like a Valkyrie::ChangeSet #prepopulate! doesn't make it look changed
   Failure/Error: DEFAULT_FAILURE_NOTIFIER = lambda { |failure, _opts| raise failure }
       expected `#<Hyrax::Forms::PcdmCollectionForm:0x0000ffff700c79a0 @fields={"human_readable_type"=>"Collection", "...rs:0x0000ffff71b06000 @base=#<Hyrax::Forms::PcdmCollectionForm:0x0000ffff700c79a0 ...>, @errors=[]>>.changed?` to be falsey, got true
     Shared Example Group: "a Valkyrie::ChangeSet" called from ./spec/forms/hyrax/forms/pcdm_collection_form_spec.rb:8
     # /app/bundle/ruby/3.2.0/gems/valkyrie-3.1.1/lib/valkyrie/specs/shared_specs/change_set.rb:48:in `block (3 levels) in <top (required)>'
     # /app/bundle/ruby/3.2.0/gems/webmock-3.19.1/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>'

The spec is as follows:

describe "#prepopulate!" do
  it "doesn't make it look changed" do
    expect(change_set).not_to be_changed
    change_set.prepopulate!
    expect(change_set).not_to be_changed
  end
end

When we dig a bit deeper, there are three properties that have pre-populators:

  • :version
  • :banner_info
  • :logo_info

And in the above RSpec example, when we ask the change_set what has changed after #prepopulate!, we get the following: {"version"=>true, "banner_info"=>true, "logo_info"=>true}

This is problematic for a few reasons. First Valkyrie does not account for pre-populators. And more vexing, the generator for a collection form and resource has the following (first the form and then the spec):

# frozen_string_literal: true

# Generated via
#  `rails generate hyrax:collection_resource <%= class_name %>`
class <%= class_name %>Form < Hyrax::Forms::PcdmCollectionForm
  include Hyrax::FormFields(:<%= file_name %>)
end
# frozen_string_literal: true

# Generated via
#  `rails generate hyrax:collection_resource <%= class_name %>`
require 'rails_helper'
require 'valkyrie/specs/shared_specs'

RSpec.describe <%= class_name %>Form do
  let(:change_set) { described_class.new(resource: resource) }
  let(:resource)   { <%= class_name %>.new }

  it_behaves_like 'a Valkyrie::ChangeSet'
end
jeremyf added a commit that referenced this issue Feb 19, 2024
jeremyf added a commit that referenced this issue Feb 19, 2024
jeremyf added a commit to scientist-softserv/valkyrie that referenced this issue Feb 19, 2024
This test is fragile in that it does not account for the fact that we
can specify a :prepopulate option for a property.

When you specify the `:prepopulate` option (as in
`Hyrax::Form::PcdmCollection`) and use this shared spec, then the now
deleted spec fails with no recourse.

Related to:

- samvera/hyrax#6702
- samvera/hyrax#6703
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant