Fix all specs to run with flexible metadata enabled#7376
Open
orangewolf wants to merge 37 commits intomainfrom
Open
Fix all specs to run with flexible metadata enabled#7376orangewolf wants to merge 37 commits intomainfrom
orangewolf wants to merge 37 commits intomainfrom
Conversation
- 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>
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
Test Results 17 files + 4 17 suites +4 3h 19m 35s ⏱️ + 21m 48s For more details on these failures, see this check. Results for commit 59a0826. ± Comparison against base commit da62e1e. This pull request removes 367 and adds 461 tests. Note that renamed tests count towards both.♻️ This comment has been updated with latest results. |
Fix style/lint issues across 6 files: block layout, indentation, lambda syntax, modifier conditionals, numeric predicates, rescue alignment, and spacing in specs. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This reverts commit 0840163.
Allow FlexibleProfileSeeder.generate_seeds to be called in the spec, since allinson's flexible? config causes it to be invoked before the collection type and collection seeders. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove label: from monograph factory calls in show_document_list_row specs — Monograph has no label attribute in flexible metadata mode - Stub label method on GenericWork in admin_sets show_document_list_row - Stub new_record and contexts on AdminSet form spec - Stub persisted? on ResourceForm in file_sets permission spec — needed because ResourceForm copies the model, losing stub_model stubs - Stub persisted? on form in form_files spec for same reason - Stub AdminSetCreateService and agreement_accepted in form_progress spec to avoid hitting Solr during prepopulate! Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Stub AdminSetCreateService.find_or_create_default_admin_set in the top-level before block so that ResourceForm#prepopulate! does not attempt to connect to Solr during view specs. The "when the work has been saved before" context already had this stub, but the "for a new object" and "additional sections" contexts did not. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- work_show_presenter: Make dynamic methods accept *args so presenter methods like depositor(default) work when called with arguments from _featured_fields.html.erb. Also wrap multi-value fields in Array.wrap so date_created.each works correctly in show field partials. - file_set_update_spec: Add creator to file_set factory since flexible metadata requires creator on FileSets. - _index_list_default spec: Stub render_optionally? on view since flexible_catalog_behavior sets this as an index field condition. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…llution - index_list_default spec: Use define_singleton_method instead of RSpec stub for render_optionally? so Blacklight's Object#method lookup works when FlexibleCatalogBehavior adds :if conditions to index fields. - ResourceForm: Add respond_to? cleanup to the deprecated constructor path, mirroring the resource: keyword path. This removes form definitions for attributes the model doesn't support, preventing NoMethodError when test ordering causes property definitions to accumulate in class-level definitions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- EditPermissionsService: Use safe navigation (&.) when looking up PermissionTemplate by source_id, since it may not exist in test contexts (fixes _currently_shared permissions spec in dassie shard 0) - _index_list_default spec: Define render_optionally? on controller instead of view, because Blacklight::Configuration::Context wraps the controller object (via spec/support/controller_level_helpers.rb) and calls context.method(:render_optionally?) for :if evaluation (fixes allinson shard 2) - AdminSetsController: Restore creator pre-population before form validation in valkyrie_create, since flexible metadata requires creator to be present for validation to pass (fixes 3 create tests in allinson shard 0) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…y_shared deadlock - Set creator on form before validation in admin_sets_controller#valkyrie_create to prevent "Creator can't be blank" error when flexible metadata requires creator - Pre-set admin_set_id before prepopulate! in _currently_shared spec to avoid triggering find_or_create_default_admin_set which caused PG deadlocks Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
dlpierce
approved these changes
Mar 13, 2026
Member
dlpierce
left a comment
dlpierce
left a comment
There was a problem hiding this comment.
Looks good, thanks! Having more conditionals in the specs is unfortunate, but moves us in the right direction for the future.
Comment on lines
10
to
12
| # Controlling Flexible Metadata is done per test application, but the test suite needs | ||
| # to disable it for now because disabling the inclusion of core/basic metadata has a huge | ||
| # impact on the suite. It can be explicitly enabled or mocked where appropriate. |
Member
There was a problem hiding this comment.
Is this comment outdated now with the ENV settings below removed?
Comment on lines
+7
to
+12
| # In flexible mode, WorkShowPresenter#define_dynamic_methods redefines | ||
| # depositor on SolrDocument::OrderedMembers without accepting arguments. | ||
| # The homepage templates call depositor(default_message) with one arg, | ||
| # causing ArgumentError. This is a production code bug to be fixed in | ||
| # WorkShowPresenter; skip these tests until then. | ||
| skip("Flexible metadata: depositor method redefined without args by WorkShowPresenter#define_dynamic_methods") if Hyrax.config.flexible? |
Member
There was a problem hiding this comment.
Has an issue been created to capture this?
Contributor
randalldfloyd
left a comment
randalldfloyd
left a comment
There was a problem hiding this comment.
The suite passes for me locally when running against the allinson app.
randalldfloyd
approved these changes
Mar 13, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes env var overrides that forced flexible metadata OFF in specs, and fixes test setup so the full spec suite passes with flexible metadata ON (
HYRAX_FLEXIBLE=true,HYRAX_DISABLE_INCLUDE_METADATA=true).All changes are spec-only — no production code was modified.
Spec Results
WorkShowPresenter#define_dynamic_methods(redefinesdepositorwithout accepting the optional default argument)Changes
spec/spec_helper.rb
ENV['HYRAX_FLEXIBLE'],ENV['HYRAX_DISABLE_INCLUDE_METADATA'],ENV['HYRAX_FLEXIBLE_CLASSES']overridesacts_as_flexible_resourcecalls for engine base classescheck_if_flexibleon indexer and form classes after flexible setup (fixes class-loading timing issue)FlexibleSchema.createso they use the real allinson profilehyrax_flexible_schemastable from DatabaseCleaner truncationspec/fixtures/files/m3_profile-allinson.yaml
Hyrax::Work,Hyrax::PcdmCollection,Hyrax::FileSet) to M3 profilerecord_infoproperty for MonographForm specs
form.modelinstead ofworkfor sync tests (flexible mode creates resource copies)include Hyrax::FormFields(:basic_metadata)in test form classesattributes_forFeature specs
keywordvskeywords)WorkShowPresenterdepositor bugOther spec fixes
persisted?onform.modelCI
allinsonto the CI test matrix inlint-build-test.ymlGemfile.allinsonfor dedicated lockfileProduction code fixes (from earlier commits on this branch)
check_if_flexibletoPcdmObjectIndexerfilter_by_type.rbto exclude file_set_classes fromwork_typesfacetableto keyword + propagatelink_to_facetinFlexibleCatalogBehaviorreturnbeforerender :editin admin_sets_controllerValkyrie::IDforadmin_set_idon flexible metadata works