Skip to content
New issue

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

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

Already on GitHub? # to your account

Cleaning up specs to pass (via xit) so we can begin to improve our test coverage #103

Merged
merged 5 commits into from
Feb 6, 2023

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Feb 6, 2023

Moving FileSet and NewspaperIssue definition to specs

ac4d3e1

Before this commit, these files were something that all applications
would load into their production code. Ideally, we don't want to do
that for the two reasons:

  • First, not everyone wants a NewspaperIssue model in their application

  • Second the file_set, as implemented, directly inherits from
    ActiveFedora::Base. If a downstream FileSet were to not inherit
    from ActiveFedora:Base we would have a mismatch error.

With this commit, we're saying that the IIIF print gem will require a
FileSet that conforms to the PCDM expectations. But we won't dictate
its implementation details in the downstream application. We instead
dictate those implementation details in the test application.

Also we will not force downstream Hyrax implementations to include a
NewspaperIssue model.

Related to:

Replacing specs with xit todo spec

c8cb50e

These specs use the now non-existent NewspaperPage and need to be
revisited. Also adding near term TODO work to resolve duplication of
effort/knowledge.

Commenting out failing tests, unclear what we need of this

09f352f

Mocking out polymorphic_url behavior

400fe3e

Prior to this commit (and earlier in development) we had routes and
models for this gem. However, without custom models and routes, the
polymorphic_url method failed.

We don't need routes, because the purpose of this gem is to provide
configuration options for downstream Hyrax models (e.g. the gem doesn't
ship with it's own Hyrax work types).

With this commit, we mock the behavior of the polymorphic_url and
resolve 3 broken specs.

Before this commit, these files were something that all applications
would load into their production code.  Ideally, we don't want to do
that for the two reasons:

- First, not everyone wants a NewspaperIssue model in their application

- Second the file_set, as implemented, directly inherits from
  `ActiveFedora::Base`.  If a downstream FileSet were to not inherit
  from `ActiveFedora:Base` we would have a mismatch error.

With this commit, we're saying that the IIIF print gem will require a
FileSet that conforms to the PCDM expectations.  But we won't dictate
its implementation details in the downstream application.  We instead
dictate those implementation details in the test application.

Also we will not force downstream Hyrax implementations to include a
NewspaperIssue model.

Related to:

- #101
These specs use the now non-existent NewspaperPage and need to be
revisited.  Also adding near term TODO work to resolve duplication of
effort/knowledge.
Prior to this commit (and earlier in development) we had routes and
models for this gem.  However, without custom models and routes, the
polymorphic_url method failed.

We don't need routes, because the purpose of this gem is to provide
configuration options for downstream Hyrax models (e.g. the gem doesn't
ship with it's own Hyrax work types).

With this commit, we mock the behavior of the polymorphic_url and
resolve 3 broken specs.
@jeremyf jeremyf changed the title Moving FileSet and NewspaperIssue definition to specs Cleaning up specs to pass (via xit) so we can begin to improve our test coverage Feb 6, 2023
@jeremyf jeremyf merged commit 73a55ab into main Feb 6, 2023
@jeremyf jeremyf deleted the consolidating-models-into-iiif-print-models branch February 6, 2023 21:50
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants