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

Provide a Valkyrie changeset factory #4047

Merged
merged 2 commits into from
Oct 3, 2019
Merged

Provide a Valkyrie changeset factory #4047

merged 2 commits into from
Oct 3, 2019

Conversation

no-reply
Copy link
Contributor

For any given Valkyrie model, we want a factory for an appropriate ChangeSet,
with attribute accessors and a #sync method.

We provide this with a Hyrax::ChangeSet.for(my_resource) factory. This builds
the changeset class on the fly, applying the fields from the resource
schema. Then a new instance of the class is initialized to handle the ChangeSet
for the specific object.

As future work, we could consider caching the changeset classes. For now, it
seems best to watch and wait--avoiding premature optimization.

Changes proposed in this pull request:

  • Extends the valkyrie interfaces by adding a ChangeSet factory

@samvera/hyrax-code-reviewers

@cjcolvar cjcolvar added the wings label Sep 27, 2019
@no-reply no-reply force-pushed the hyrax-changesets branch 2 times, most recently from 7468716 to 5cb6f97 Compare October 2, 2019 15:50
@VivianChu VivianChu self-requested a review October 3, 2019 19:28
Tom Johnson added 2 commits October 3, 2019 13:16
For any given Valkyrie model, we want a factory for an appropriate `ChangeSet`,
with attribute accessors and a `#sync` method.

We provide this with a `Hyrax::ChangeSet.for(my_resource)` factory. This builds
the changeset class on the fly, applying the fields from the resource
schema. Then a new instance of the class is initialized to handle the `ChangeSet`
for the specific object.

As future work, we could consider caching the changeset classes. For now, it
seems best to watch and wait--avoiding premature optimization.
Hyrax now has a change set class builder and factory. Use these to construct the
change set instance for `AccessControl` objects in `AccessControlList`.
Copy link
Contributor

@VivianChu VivianChu left a comment

Choose a reason for hiding this comment

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

👍

@no-reply no-reply merged commit 84dad1d into master Oct 3, 2019
@no-reply no-reply deleted the hyrax-changesets branch October 3, 2019 21:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants