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

Initial refactor of revision provider and access #333

Merged
merged 3 commits into from
Jun 25, 2024

Conversation

andybroomfield
Copy link
Contributor

@andybroomfield andybroomfield commented Jun 17, 2024

Fix #323

Refactor the alert banner entitiy to use a more standard approach to revisions, using \Drupal\Core\Entity\Routing\RevisionHtmlRouteProvider instead of custom routes, and using standard revsion routes instead of custom versions in routing.yml.

This also groups all the non view crud permissions into the manage alert banner permission in alertBannerEntityAccessControlHandler.

This is the first part of a refactor for version 1.8.

Fix #323

Refactor the alert banner entitiy to use a more standard approach to revisions,
using \Drupal\Core\Entity\Routing\RevisionHtmlRouteProvider
instead of custom routes, and using standard revsion routes instead of custom
versions in routing.yml.

This also groups all the none view crud permissions into the manage alert
banner permission in alertBannerEntityAccessControlHandler.
Normally not required as the view is in place, but sometimes needed if the
view is disabled.
@andybroomfield andybroomfield marked this pull request as ready for review June 19, 2024 09:39
@andybroomfield
Copy link
Contributor Author

Taking this out of draft. focusing this PR on just removing the history routes and using the standard Drupal provided ones.
Note: I'm targeting 1.8.x branch as this is part of a refactor. @localgovdrupal/maintainers are you happy with this approach as it means bug fixes can still be targeted against 1.x whilst this work is in progress, and I can do smaller PRs for specific issues. If so, do these indivudual ones still want reviews?

Note: this may still get removed later in the refactor, but for now restore
route admin/structure/alert-banner-types
@Adnan-cds
Copy link
Contributor

I will leave it to you Andy :)

@finnlewis
Copy link
Member

Very happy to get the 1.8.x branch into a ready state for review.

If we can get the tests running on merges INTO 1.8.x - that should help.

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