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

Ability and search fixes #6653

Closed
wants to merge 49 commits into from

Conversation

laritakr
Copy link
Contributor

@laritakr laritakr commented Feb 1, 2024

Fixes

Uses config variable for admin set in more places. Allows for creating a custom resource that inherits from Hyrax::AdministrativeSet.
References config in places which were previously only hardcoded with AdminSet and Hyrax::AdministrativeSet.

Remaining specs with failures (dassie 19, dassie-valkyrie 20, koppie 11)

  • spec/features/dashboard/collection_spec.rb
  • spec/controllers/hyrax/admin/admin_sets_controller_spec.rb
  • spec/features/create_admin_set_spec.rb
  • spec/lib/freyja/persister_spec.rb (dassie-valkyrie only)

refs scientist-softserv/hykuup_knapsack#94

Use config variable for admin set in more places.
@laritakr laritakr force-pushed the collections-and-admin-set-resources branch 2 times, most recently from d548dca to e0c1851 Compare February 1, 2024 18:57
@laritakr laritakr added the notes-valkyrie Release Notes: Valkyrie specific label Feb 1, 2024
@laritakr laritakr force-pushed the collections-and-admin-set-resources branch from e0c1851 to 27164f2 Compare February 1, 2024 20:05
@laritakr laritakr force-pushed the collections-and-admin-set-resources branch from 27164f2 to 8b5cf93 Compare February 1, 2024 22:04
laritakr and others added 6 commits February 6, 2024 12:03
The admin set create service cannot create an AdminSet. Changing to use
the config variable causes specs to break, because they are using the
default config value of AdminSet.

This is the first step to locking down the change. The default fallback
value to AdminSet should probably be changed as well.
laritakr and others added 5 commits February 7, 2024 15:45
The config admin set class should not fall back to AdminSet, since
the default admin set service cannot create an AdminSet.
Links for Admin sets in collection type dropdown have changed.
calling wrong method
laritakr and others added 24 commits February 8, 2024 11:51
Shared specs are still failing.
Base class on config for admin_set_model rather than the use_valkyrie flag
* double_combo:
  ☑️ Add stubbing
  🐛 Favor helper method over instance variable
  Extracting a collection models method
  💄 endless and ever appeasing of the coppers
  ♻️ Stub incidental query
  ♻️ Consider all of the curation concerns
Throughout the code we have quite a bit of conditionals regarding what
is a work, collection, file_set, and adminsitrative set.  This model
attempts to provide a common point to interrogate the application.

There are, at present, no refactors to use this model.

Consider how our specs and our application are inconsistent in their
declaration/configuration/stubbing.  This model should help with that.
Instead of the myriad of ways of asking about which models to use, let's
leverage a consolidated central place for information.

This is but one step in addressing other issues.
…n-set-resources

* extract-hyrax-model-registry:
  🐛 Favor helper method over instance variable
  Favor helper method over instance variable
  ♻️ Favor dynamic value
  ♻️ Favor helper method
  ☑️ Fix stubbing
  Adding case statement
  ♻️ Fixing spec
  ♻️ Favor Hyrax::ModelRegistry
  💄 endless and ever appeasing of the coppers
  ♻️ Leverage Hyrax::ModelRegistry for ability tests
  ♻️ Favor Hyrax::ModelRegistry for search builders
  ♻️ Extracting naming container
  🧹 Expose Hyrax.config.use_valkyrie=
@kirkkwang kirkkwang force-pushed the double_combo branch 2 times, most recently from 8aa7ec6 to 5d5e1f9 Compare February 14, 2024 22:36
@laritakr
Copy link
Contributor Author

Relevant work was included elsewhere.

@laritakr laritakr closed this Feb 16, 2024
@jeremyf jeremyf deleted the collections-and-admin-set-resources branch February 16, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes-valkyrie Release Notes: Valkyrie specific
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants