Skip to content

Fix model specs to run with flexible metadata enabled#7372

Closed
orangewolf wants to merge 17 commits intosamvera:allinson_test-appfrom
orangewolf:allinson_specs
Closed

Fix model specs to run with flexible metadata enabled#7372
orangewolf wants to merge 17 commits intosamvera:allinson_test-appfrom
orangewolf:allinson_specs

Conversation

@orangewolf
Copy link
Member

Summary

Removes env var overrides that forced flexible metadata OFF in specs, and fixes the test setup so model specs pass with flexible metadata ON.

Changes

  • Add Hyrax::Work, Hyrax::PcdmCollection to M3 profile YAML classes
  • Add Hyrax::FileSet to all property class lists (was only unnamespaced FileSet)
  • Add acts_as_flexible_resource calls for Hyrax::Work and Hyrax::PcdmCollection in spec_helper
  • Remove env var overrides that forced flexible metadata OFF in specs

Results

179 model spec examples now pass with flexible metadata ON (was 18 failures).

laritakr and others added 17 commits March 2, 2026 19:03
Creators on admin sets are set in a transaction
step, and anything used in the form is overridden
by the transaction step. This change removes the
creator requirement from the form, and returns
to previous behavior of allowing the creator to be
set in the transaction step.
The spec was not taking renamed properties into
account. Now that the validator handles renamed
properties, the spec should as well.
- Add Hyrax::Work, Hyrax::PcdmCollection to M3 profile YAML classes
- Add Hyrax::FileSet to all property class lists (was only unnamespaced FileSet)
- Add acts_as_flexible_resource calls for Hyrax::Work and Hyrax::PcdmCollection in spec_helper
- Remove env var overrides that forced flexible metadata OFF in specs

179 model spec examples now pass with flexible metadata ON.
…earch issues

Add check_if_flexible(work_class) to the PcdmObjectIndexer factory method,
mirroring the pattern already present in FileSetIndexer and
PcdmCollectionIndexer. Without this, flexible metadata properties were
never indexed for work classes, causing catalog searches and facet
queries to return empty results.

Also fix FilterByType#work_types to exclude file_set_classes, since
Hyrax::FileSet may be registered as a curation concern in flexible
metadata environments but should never appear in catalog search results.

Add facetable indexing directive to properties in the allinson M3 test
profile (creator, contributor, subject, language, publisher,
resource_type, based_near, rights_statement, keyword) to prevent
FlexibleCatalogBehavior#load_flexible_schema from removing their facet
fields from the blacklight config.

Fixes 5 controller specs: catalog facet search, catalog file metadata
search (2), and file_sets_controller breadcrumb display (2).

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ehavior

When load_flexible_schema encounters an index field that already exists
in the blacklight config, it updates label, itemprop, and helper_method
but was not updating link_to_facet. This caused the facetable keyword
field to lose its link_to_facet value on the second pass through
load_flexible_schema (triggered by controller initialization).

Fixes flexible_catalog_behavior_spec link_to_facet property test.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add return before render :edit in value_or blocks in both
valkyrie_create_detail and valkyrie_update methods. Without the return,
execution continues past the failure handler, causing double render
errors.

Also pre-populate the creator field from current_user before form
validation in valkyrie_create. With flexible metadata, the creator
property has cardinality minimum: 1, causing validation to fail before
the transaction's SetUserAsCreator step can set it.

Fixes 3 admin_sets_controller_spec #create specs.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When flexible metadata is enabled, Flexibility.load coerces nil
admin_set_id to Valkyrie::ID("") via schema.call_unsafe. This empty
ID is truthy so ||= never replaces it with the default admin set,
causing missing permissions and workflow entities.

Change SetDefaultAdminSet, EnsureAdminSet, and the form prepopulator
to use .to_s.blank? checks instead of ||= or truthiness tests.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
* Fix field rendering in file set valkyrie form

This commit will replace the `f.input term` calls with the hyrda-editor
helper `render_edit_field_partial` so it would do a partial lookup by
field name.  This allows the rendering of fields such as
`app/views/records/edit_fields/_license.html.erb` so the license field
will be rendered like how the work and collection forms do.  Also this
will check for multi or single valued fields.

Ref:
- https://github.com/samvera/hydra-editor/blob/main/app/helpers/concerns/records_helper_behavior.rb#L14-L17

* Make location (based_near) field work for file set

This commit will add the based_near field javascript to the file set
form so it looks like the work and collection forms.

* Change file set form cancel link

This proposed change will update the file set form's cancel link to let
the user go back to the file set show page instead of the parent work's
show page.  This is more consistent with the other forms.

* Use missing method pattern for file set presenters

This commit will make the file set presenters use the same missing
method pattern as the work presenters.  The logic has been moved into a
mixin and specs updated.  This sets up a better path for flexible
metadata in file sets and how metadata gets displayed on the show page
for consuming applications.

Ref:
- samvera@a5a0ae9

* Favor the Wings::ModelRegistry.reverse_lookup

This commit will change the way we look up the hydra model, instead of
adding "Resource" string as per convention for works, collections, and
admin sets, we use the `Valkyrie.config.resource_class_resolver`
instead.  This would make it work for models like:
FileSet => Hyrax::FileSet.

Also, switching to use the #klass instead of #name for the dummy models
since it seems more accurate, tested with works and file set.

* Add `break-inside: avoid;` to file set show styles

This will prevent "Characteriaztion" from being split across the two
columns in the file set show page.
The creator= line on valkyrie_create is being replaced by merging main which has a proper fix.
…source

Indexer classes autoload during RSpec file loading (before before(:suite) runs),
so check_if_flexible evaluates when models aren't yet flexible. Re-running it
after acts_as_flexible_resource retroactively includes the M3SchemaLoader module.

Fixes 3 failures in PcdmCollectionIndexer and FileSetIndexer specs.
Spec-only changes to handle both flexible and non-flexible metadata modes:

- Re-run check_if_flexible on PcdmCollectionForm and AdministrativeSetForm
  in spec_helper to fix class-loading timing (same pattern as indexers)
- Use form.model instead of work variable in sync expectations, since
  flexible mode creates a resource copy in ResourceForm#initialize
- Guard basic_metadata includes in anonymous test form classes with
  unless: Hyrax.config.flexible? to prevent missing label errors
- Branch primary_terms, required_fields, and validation expectations
  for flexible mode where M3 profile controls field attributes
- Handle description as array in AdminSetForm specs (M3 data_type: array)
- Accept blank admin_set_id (Valkyrie::ID("") in flexible mode vs nil)
- Add attributes_for stub to M3SchemaLoader mock
- Fix Ruby 3 keyword argument capture in have_received block

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Stub persisted? on resource.model instead of the original model variable,
because flexible metadata's ResourceForm creates a resource copy rather
than using the original model directly.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Address 7 categories of failures when running feature specs with
HYRAX_FLEXIBLE=true in the allinson environment:

spec_helper.rb:
- Re-initialize app-level model classes (Monograph, GenericWork,
  CollectionResource) after FlexibleSchema creation so they get the
  real M3 profile instead of the default fallback schema
- Unregister engine base classes from curation concerns after schema
  validation passes to prevent routing errors (new_hyrax_work_path)
- Re-run check_if_flexible on app-level indexer/form classes
- Exclude hyrax_flexible_schemas table from DatabaseCleaner truncation
  to prevent the allinson profile from being replaced by the generic
  koppie profile between feature specs

create_admin_set_spec.rb:
- Fill in required Creator field when Hyrax.config.flexible?

search_spec.rb:
- Use flexible-aware itemprop value (FlexibleCatalogBehavior changes
  "keywords" to "keyword" on index fields)

homepage_spec.rb:
- Skip tests in flexible mode due to production code bug where
  WorkShowPresenter#define_dynamic_methods redefines depositor method
  without accepting the optional default argument

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Create Gemfile.allinson (dedicated lockfile, same as koppie/sirenia pattern)
- Add allinson to ci_test_app matrix in lint-build-test.yml
- docker-compose-allinson.yml already exists with HYRAX_FLEXIBLE=true
@orangewolf
Copy link
Member Author

Superseded by #7376 (same branch, now pushed directly to samvera/hyrax instead of cross-fork)

@orangewolf orangewolf closed this Mar 5, 2026
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

Successfully merging this pull request may close these issues.

3 participants