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.

Sign up for GitHub

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

Jump to bottom

Topic/k1ch/ Introduce include_permissions query param for GET /roles APIs #126

Merged
merged 13 commits into from
Oct 17, 2024
Merged

Topic/k1ch/ Introduce include_permissions query param for GET /roles APIs #126

merged 13 commits into from
Oct 17, 2024

Conversation

k1ch
Copy link
Collaborator

@k1ch k1ch commented Oct 7, 2024 •
edited
Loading

Overview

In This PR:

Remaining:

  • Refactor admin-role and view-select-relationships DB layer to use knex
  • Add additional tests
  • Bump version to v2.3.0

Your PR Checklist 🚨

❤️ Please review the guidelines for contributing to this repository. Our goal is to merge PRs fast 💨 .

  • Check your code additions will fail neither code linting checks nor unit tests.
  • Additional units tests have been added to prove code updates work and fixes are effective
  • Additional documentation has been added (if appropriate)
  • Update Assignee field to yourself

Issue Reference

Closes #

Open Questions or Items to Callout

@k1ch k1ch self-assigned this Oct 7, 2024
@k1ch k1ch added enhancement New feature or request component: admin Administration related functionality labels Oct 7, 2024
@k1ch k1ch changed the title Topic/k1ch/roles apis include permissions Topic/k1ch/ Introduce includePermissions query param for GET /roles APIs Oct 7, 2024
@k1ch k1ch force-pushed the topic/k1ch/roles-apis-includePermissions branch from fb04a39 to c37becb Compare October 8, 2024 22:19
@k1ch k1ch force-pushed the topic/k1ch/roles-apis-includePermissions branch from ced52d1 to 3f687d2 Compare October 15, 2024 21:43
@k1ch k1ch changed the title Topic/k1ch/ Introduce includePermissions query param for GET /roles APIs Topic/k1ch/ Introduce include_permissions query param for GET /roles APIs Oct 16, 2024
@k1ch k1ch marked this pull request as ready for review October 16, 2024 20:13
@k1ch k1ch requested a review from mikelax October 16, 2024 20:13
mikelax
mikelax reviewed Oct 17, 2024
View reviewed changes
Copy link
Collaborator

@mikelax mikelax left a comment

Choose a reason for hiding this comment

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

@k1ch overall the changes look good. I left a few trivial comments and answered your question about the listRoles function return.

server/the-usher.js Show resolved Hide resolved
database/layer/admin-permission.js Show resolved Hide resolved
database/layer/admin-permission.js Show resolved Hide resolved
server/src/api_endpoints/clients/roles.js Show resolved Hide resolved
server/src/api_endpoints/endpoint_roles.js Outdated Show resolved Hide resolved
server/src/security_layer/jwt_signature_validator.js Outdated Show resolved Hide resolved
server/test/endpoint_clients.test.js Outdated Show resolved Hide resolved
server/test/endpoint_clients.test.js Outdated Show resolved Hide resolved
database/layer/knex.js Show resolved Hide resolved
server/src/api_endpoints/clients/roles.js Show resolved Hide resolved
@k1ch k1ch requested a review from mikelax October 17, 2024 20:02
@k1ch k1ch merged commit 63dd5c4 into main Oct 17, 2024
4 checks passed
@k1ch k1ch deleted the topic/k1ch/roles-apis-includePermissions branch October 17, 2024 21:32
@mikelax mikelax mentioned this pull request Oct 17, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mikelax mikelax mikelax approved these changes

Assignees

@k1ch k1ch

Labels
component: admin Administration related functionality enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@k1ch @mikelax