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

don't attempt to save AF::WithMetadata::MetadataNode#file_hash #5320

Merged
merged 2 commits into from Jan 11, 2022

Conversation

no-reply
Copy link
Member

@no-reply no-reply commented Jan 11, 2022

ActiveFedora::WithMetadata::MetadataNode provides a #file_hash attribute,
but writing to it, even with the existing data, prevents all saves.

Wings needs to map data in and out of this class freely, and this attribute
functions as a honeypot. it baits Wings to preserve the data, and then fails
save when the converters try to do keep the existing data. since any attempt to
write this data will break save, ActiveFedora should treat it as read only.

never include MetadataNode#file_hash in changed attributes lists when Wings is
loaded.

i think this is the issue presently blocking #5278.

@no-reply no-reply changed the title WIP: add failing test for Wings file_hash behavior don't attempt to save AF::WithMetadata::MetadataNode#file_hash Jan 11, 2022
@no-reply no-reply marked this pull request as ready for review January 11, 2022 06:20
cjcolvar
cjcolvar previously approved these changes Jan 11, 2022
Copy link
Member

@cjcolvar cjcolvar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Brilliant! ✨

@no-reply
Copy link
Member Author

i'm hoping that the rebase will fix the build.

@no-reply
Copy link
Member Author

@cjcolvar do you think it would be a good idea to open an issue on ActiveFedora?

the error behind this is easily repeatable there. it might be appropriate to port this monkeypatch upstream, or come up with a more principled fix.

tamsin johnson added 2 commits January 11, 2022 09:43
one expectation per test; use named rspec subject.
`ActiveFedora::WithMetadata::MetadataNode` provides a `#file_hash` attribute,
but writing to it, even with the existing data, prevents all saves.

Wings needs to map data in and out of this class freely, and this attribute
functions as a honeypot. it baits Wings to preserve the data, and then fails
save when the converters try to do keep the existing data. since any attempt to
write this data will break save, ActiveFedora should treat it as read only.

never include `MetadataNode#file_hash` in changed attributes lists when Wings is
loaded.
@no-reply no-reply merged commit 64a610a into main Jan 11, 2022
@no-reply no-reply deleted the file-metadata-wings branch January 11, 2022 17:54
@cjcolvar
Copy link
Member

@no-reply An upstream fix would probably be a good idea.

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.

None yet

3 participants