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

♻️ Remove conditionals and account for Valkyrie #320

Merged
merged 1 commit into from
Jan 23, 2024

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Jan 23, 2024

The IiifPrint.model_configuration is something we use to create a module that we mixin to a model; either ActiveFedor::Base or Valkyrie::Resource.

As such we know when the model is configured for IiifPrint. It follows that we can remove the iiif_print_config? methods.

Further, to account for Valkyrie indexers favoring the #to_solr method and ActiveFedora indexers favoring #generate_solr_document we introduce a bit of sleuthing to see which type we have. But also concede that if you've implemented either method we're going to duplicate that behavior.

So we get to remove one module and two methods; and remove a potential timing issue regarding when we mixing the model_configuration and when we loop through the work_types to include additional logic.

The `IiifPrint.model_configuration` is something we use to create a
module that we mixin to a model; either `ActiveFedor::Base` or
`Valkyrie::Resource`.

As such we know when the model is configured for IiifPrint.  It follows
that we can remove the `iiif_print_config?` methods.

Further, to account for Valkyrie indexers favoring the `#to_solr` method
and ActiveFedora indexers favoring `#generate_solr_document` we
introduce a bit of sleuthing to see which type we have.  But also
concede that if you've implemented either method we're going to
duplicate that behavior.

So we get to remove one module and two methods; and remove a potential
timing issue regarding when we mixing the model_configuration and when
we loop through the work_types to include additional logic.
@jeremyf jeremyf force-pushed the refactoring-configurations branch from 3b6573b to 1c3ea71 Compare January 23, 2024 17:41
@jeremyf jeremyf merged commit df59edf into main Jan 23, 2024
9 checks passed
@jeremyf jeremyf deleted the refactoring-configurations branch February 23, 2024 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants