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

♻️ Favor Hyrax.query_service over method chaining #6671

Merged
merged 1 commit into from
Feb 7, 2024

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Feb 6, 2024

NOTE: This is part of the looming Double Combo work for the lazy migration for Hyrax, but I figure having this reviewed separately eases the review of a chunky PR.

See Access Control List (ACL) Considerations in the Hyku wiki.

Prior to this commit we relied on the access_control object to "self convert" via the #valkyrie_resource method. However, that method circumvents the Hyrax.query_service which provides a more up to date version of the permissions.

This is subtle bug is most notable when we have composite adapters; those modeled in the double_combo branch.

How it manifests is as follows:

  • I have a Work and ACL in Fedora.
  • I update the ACL to Postgres.
    • When I fetch the Work, it's in Fedora and grabs the ACL from Fedora which is not the most current.

This relates to:

@samvera/hyrax-code-reviewers

Prior to this commit we relied on the access_control object to "self
convert" via the `#valkyrie_resource` method.  However, that method
circumvents the `Hyrax.query_service` which provides a more up to date
version of the permissions.

This is subtle bug is most notable when we have composite adapters;
those modeled in the `double_combo` branch.

How it manifests is as follows:

- I have a Work and ACL in Fedora.
- I update the ACL to Postgres.
  - When I fetch the Work, it's in Fedora and grabs the ACL from Fedora
    which is not the most current.

This relates to:

- 0de7df6
- #6670
@jeremyf jeremyf force-pushed the adding-query_service-lookup-for-transformer branch from 7bbde49 to f0e99c5 Compare February 6, 2024 20:12
@dlpierce dlpierce merged commit 838b668 into main Feb 7, 2024
6 checks passed
@dlpierce dlpierce deleted the adding-query_service-lookup-for-transformer branch February 7, 2024 19:36
@jeremyf jeremyf added the notes-bugfix Release Notes: Fixed a bug label Feb 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes-bugfix Release Notes: Fixed a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants