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

[wings] Avoid special handling for values in ConverterValueMapper #4069

Merged
merged 1 commit into from
Oct 25, 2019

Conversation

no-reply
Copy link
Contributor

@no-reply no-reply commented Oct 8, 2019

Refactor Wings::ConverterValueMapper to use the ::Valkyrie::ValueMapper
chain in place of all prior special handling. Where we used to delete attributes
manually, now we capture and return nil for them during conversion.

This helps clarify the classes of special behavior that we have, and opens the
door to refined handling for edge cases.
Remove the ActiveFedoraAttributes#filter_attributes method.

Changes proposed in this pull request:

  • This is a straight refactor of internal Wings behavior

@samvera/hyrax-code-reviewers

Refactor `Wings::ConverterValueMapper` to use the `::Valkyrie::ValueMapper`
chain in place of all prior special handling. Where we used to delete attributes
manually, now we capture and return `nil` for them during conversion.

This helps clarify the classes of special behavior that we have, and opens the
door to refined handling for edge cases.
Remove the `ActiveFedoraAttributes#filter_attributes` method.
@no-reply no-reply added the wings label Oct 8, 2019
@cjcolvar cjcolvar removed the wings label Oct 24, 2019
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.

👍

@cjcolvar cjcolvar merged commit c75c01d into master Oct 25, 2019
@cjcolvar cjcolvar deleted the wings-converter-refactor branch October 25, 2019 14:51
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