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

Access permissions 1: Refactor access to roles #6874

Merged
merged 3 commits into from
Dec 19, 2024

Conversation

lukasbestle
Copy link
Member

@lukasbestle lukasbestle commented Dec 17, 2024

Description

Summary of changes

$app->role() method is mainly a shortcut to $app->user()?->role() for us, but can also be used to directly access a specific role object.

Reasoning

Following PRs would add significantly more uses of $app->user()?->role() that can be shortened and DRYed by this.

Changelog

Features

  • New $app->role() method that works like $app->user() to return a specific role or the role of the current user.

Housekeeping

  • Move $app->roles() method to the AppUsers trait to increase the overview

Breaking changes

None

Docs

None (automatically documented via reference)

Ready?

  • In-code documentation (wherever needed)
  • Unit tests for fixed bug/feature
  • Tests and CI checks all pass

For review team

  • Add lab and/or sandbox examples (wherever helpful)
  • Add changes & docs to release notes draft in Notion

@lukasbestle lukasbestle added type: feature 🎉 Adds a feature (requests → feedback.getkirby.com) type: refactoring ♻️ Is about bad code; cleans up code labels Dec 17, 2024
@lukasbestle lukasbestle self-assigned this Dec 17, 2024
@lukasbestle lukasbestle requested review from a team and removed request for bastianallgeier, afbora and distantnative December 17, 2024 20:39
Copy link
Member

@bastianallgeier bastianallgeier left a comment

Choose a reason for hiding this comment

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

Good cleanup. Do we worry about the coverage? I would merge it otherwise.

@lukasbestle
Copy link
Member Author

The entire view action for the language view is not covered at the moment, this includes the only uncovered line in this changeset. I think at this point it's more important to continue with the permission additions than coverage.

@lukasbestle lukasbestle merged commit ab347fa into v5/develop Dec 19, 2024
11 of 12 checks passed
@lukasbestle lukasbestle deleted the v5/feature/access-permissions-1 branch December 19, 2024 19:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature 🎉 Adds a feature (requests → feedback.getkirby.com) type: refactoring ♻️ Is about bad code; cleans up code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants