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

Attempt to address Issue-191. #224

Merged
merged 2 commits into from
Jan 15, 2025
Merged

Conversation

markpbaggett
Copy link
Member

@markpbaggett markpbaggett commented Jan 9, 2025

What Does this Do

This attempts to address Issue-191. That issue describes a problem where homepage props are dropped from Manifests in a Collection if the manifest was created via the make_ manifest() helper. Oddly, the issue doesn't exist if manifests are created and added to the Collection in other ways.

The issue appears to be that the helper uses the Manifest class rather than ManifestRef. If we switch the classes, things seem to work as expected.

Todo: what if items prop exists in incoming kwargs?

Any potential regressions

I don't think so. I've added ManifestRef to the AutoIdConfig helper incase someone was using make_manifest() without adding an id property since that worked on Manifest but not ManifestRef. I've also had to add ManifestRef to the AutoLang helper. For some reason, this doesn't work on ManifestRef even though Reference is listed.

@markpbaggett markpbaggett marked this pull request as ready for review January 10, 2025 14:29
@glenrobson
Copy link
Contributor

I don't think Manifests can be embedded in Collections so I think this change is correct. I'm not sure why it was breaking homepage though.

@glenrobson glenrobson merged commit 465fadb into iiif-prezi:main Jan 15, 2025
7 checks passed
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.

2 participants