Open
Conversation
25c93b4 to
92b2f65
Compare
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces reason templates for access requests to standardize and enforce required fields in user-provided reasons. Key changes include:
- Adding new configuration options (REASON_TEMPLATE and REASON_TEMPLATE_REQUIRED) in the frontend and backend.
- Implementing validation logic on both API and UI to prevent submission of templated or incomplete reasons.
- Creating tests to cover various invalid and valid reason scenarios.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_reason_template.py | Adds tests to validate correct and incorrect reason template behaviors. |
| src/pages/role_requests/Create.tsx | Updates the UI to use the REASON_TEMPLATE as a placeholder. |
| src/pages/requests/Create.tsx | Enhances form validation to check for both templated and missing fields. |
| src/config/loadAccessConfig.js | Loads new config options ensuring consistency between template options. |
| src/config/accessConfig.ts | Updates type definitions to include reason template options. |
| api/views/schemas/access_requests.py | Implements API validation logic for the reason field using the new config. |
| api/operations/constraints/check_for_reason.py | Updates constraint logic to reject unmodified or incomplete reason entries. |
| api/access_config.py | Updates access configuration parsing to include the new template options. |
Comments suppressed due to low confidence (2)
api/views/schemas/access_requests.py:30
- Consider aggregating missing required fields into a comprehensive error message (e.g., listing all fields) instead of raising an error on the first missing field, to provide clear guidance to the API consumer.
for required_field in access_config.reason_template_required:
api/operations/constraints/check_for_reason.py:58
- Consider aggregating and reporting all missing required fields in the reason instead of returning after detecting the first missing field, to align with the frontend validation logic and improve error messaging.
for required_field in access_config.reason_template_required:
92b2f65 to
d0cedff
Compare
d0cedff to
37e1166
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This adds the ability to define 'Reason' templates.
It allows organizations to define expected fields for requesting access to standardize requests across teams.
Two new fields are added:
REASON_TEMPLATE: The template to use and is the default value."Project: [Project Name]\nTicket: [Ticket ID]\nJustification: [Why is this access needed?]",REASON_TEMPLATE_REQUIRED: The fields that must be present in the reason. This allows the template to define helpful context while this is the hard required text that must exist in the reason.["Project", "Ticket", "Justification"]Screenshots of the feature:

View:
Error screen:
