[OSDEV-1746] Linked My Claimed Facilities page to SLC#548
[OSDEV-1746] Linked My Claimed Facilities page to SLC#548VadimKovalenkoSNF merged 9 commits intomainfrom
Conversation
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
📝 WalkthroughWalkthroughThis pull request introduces several updates, including a new entry in the Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant CF as ClaimedFacilitiesList Component
participant R as React Router (Link)
participant CP as Contribute Page (SLC)
U ->> CF: Click "Find My Production Location"
CF ->> R: Navigate to contributeProductionLocationRoute?tab=name-address
R ->> CP: Load contribution page
CP -->> U: Render page interface
Possibly related PRs
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 32.85%Your code coverage diff: 0.25% ▴ ✅ All code changes are covered |
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
VadimKovalenkoSNF
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
deployment/terraform/lambda-functions/redirect_to_s3_origin/index.mjs (1)
1-1: Remove redundant 'use strict' directive.The 'use strict' directive is unnecessary in ES modules as they operate in strict mode by default.
-'use strict';🧰 Tools
🪛 Biome (1.9.4)
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.(lint/suspicious/noRedundantUseStrict)
deployment/terraform/lambda.tf (1)
132-146:Details
❓ Verification inconclusive
Consider implementing code signing for Lambda function.
The Lambda function has been properly configured with the necessary attributes for Lambda@Edge, but lacks code signing validation which is recommended for security.
🏁 Script executed:
#!/bin/bash # Check if code signing configuration exists in the account aws lambda list-code-signing-configs --region $(aws configure get region) --query "CodeSigningConfigs[*].{Id:CodeSigningConfigId,Description:Description}" --output tableLength of output: 359
Attention: Manual Verification Needed for Lambda Code Signing
The current Terraform configuration for the Lambda function in
deployment/terraform/lambda.tf(lines 132-146) does not include a code signing configuration. Although the function itself is properly configured for Lambda@Edge, implementing code signing is recommended to enhance security.
- Our initial automated verification using the AWS CLI failed due to the
awscommand not being found. This prevents us from confirming if any code signing configurations exist in the target AWS account.- Please manually verify whether a code signing configuration is present in your account. If none exists, consider integrating code signing for the Lambda function either by adding the appropriate configuration within your Terraform resource or by referencing an existing configuration.
🧰 Tools
🪛 Checkov (3.2.334)
[HIGH] 132-146: Ensure AWS Lambda function is configured to validate code-signing
(CKV_AWS_272)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
deployment/terraform/lambda-functions/redirect_to_s3_origin/redirect_to_s3_origin.zipis excluded by!**/*.zipsrc/react/yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (31)
doc/release/RELEASE-NOTES.md(1 hunks).github/workflows/deploy_to_aws.yml(2 hunks)deployment/terraform/cdn.tf(4 hunks)deployment/terraform/lambda-functions/redirect_to_s3_origin/index.mjs(1 hunks)deployment/terraform/lambda.tf(1 hunks)doc/release/RELEASE-NOTES.md(1 hunks)doc/release/RELEASE-NOTES.md(1 hunks)src/react/src/__tests__/components/ProductionLocationDialog.test.js(2 hunks)src/react/src/components/Contribute/ProductionLocationDialog.jsx(1 hunks)src/react/src/components/Contribute/ProductionLocationDialogFields.jsx(2 hunks)deployment/terraform/database_anonymizer_scheduled_task/docker/requirements.txt(1 hunks)doc/release/RELEASE-NOTES.md(1 hunks)doc/release/RELEASE-NOTES.md(2 hunks)src/django/api/migrations/0167_add_moderationevent_action_reason_text_fields.py(1 hunks)src/django/api/models/moderation_event.py(2 hunks)src/django/api/serializers/v1/moderation_event_update_serializer.py(3 hunks)src/django/api/templates/mail/slc_contribution_rejected_body.html(2 hunks)src/django/api/tests/test_moderation_events_update.py(3 hunks)src/django/api/views/v1/moderation_events.py(1 hunks)src/django/oar/settings.py(2 hunks)src/django/requirements.txt(1 hunks)src/react/package.json(2 hunks)src/react/src/__tests__/components/DashboardContributionRecord.test.js(3 hunks)src/react/src/__tests__/components/RejectModerationEventDialog.test.js(1 hunks)src/react/src/actions/dashboardContributionRecord.js(1 hunks)src/react/src/components/Dashboard/DashboardContributionRecord.jsx(6 hunks)src/react/src/components/Dashboard/RejectModerationEventDialog.jsx(1 hunks)src/react/src/util/styles.js(1 hunks)doc/release/RELEASE-NOTES.md(1 hunks)src/react/src/__tests__/components/SearchByOsIdTab.test.js(1 hunks)src/react/src/components/Contribute/SearchByOsIdTab.jsx(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- deployment/terraform/database_anonymizer_scheduled_task/docker/requirements.txt
- src/react/src/components/Contribute/ProductionLocationDialog.jsx
🚧 Files skipped from review as they are similar to previous changes (4)
- doc/release/RELEASE-NOTES.md
- doc/release/RELEASE-NOTES.md
- doc/release/RELEASE-NOTES.md
- doc/release/RELEASE-NOTES.md
🧰 Additional context used
🧠 Learnings (2)
src/react/src/__tests__/components/ProductionLocationDialog.test.js (2)
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/__tests__/components/ProductionLocationDialog.test.js:66-105
Timestamp: 2025-01-27T07:57:17.370Z
Learning: The ProductionLocationDialog component does not receive error props directly, as errors are not passed down explicitly to this component.
Learnt from: VadimKovalenkoSNF
PR: opensupplyhub/open-supply-hub#470
File: src/react/src/__tests__/components/ProductionLocationDialog.test.js:66-105
Timestamp: 2025-01-27T07:57:17.370Z
Learning: In the Open Supply Hub project, accessibility testing is performed manually rather than through automated tests.
src/django/api/tests/test_moderation_events_update.py (1)
Learnt from: mazursasha1990
PR: opensupplyhub/open-supply-hub#438
File: src/django/api/tests/test_moderation_events_add_production_location.py:21-65
Timestamp: 2024-12-12T14:59:19.694Z
Learning: In `src/django/api/tests/test_moderation_events_add_production_location.py`, the tests have been refactored, and the use of `POST` methods is intentional. Future suggestions to change HTTP methods in these tests may not be necessary.
🪛 Biome (1.9.4)
deployment/terraform/lambda-functions/redirect_to_s3_origin/index.mjs
[error] 1-1: Redundant use strict directive.
The entire contents of JavaScript modules are automatically in strict mode, with no statement needed to initiate it.
Safe fix: Remove the redundant use strict directive.
(lint/suspicious/noRedundantUseStrict)
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
15-15: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
🪛 Checkov (3.2.334)
deployment/terraform/lambda.tf
[HIGH] 132-146: Ensure AWS Lambda function is configured to validate code-signing
(CKV_AWS_272)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-flake8-linter
- GitHub Check: run-fe-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-fe-cov
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-countries-cov
🔇 Additional comments (54)
src/react/src/components/Contribute/SearchByOsIdTab.jsx (1)
33-39: Excellent security enhancement with URL encodingThe change to use
encodeURIComponent()when generating the URL is a critical security improvement. This properly handles special characters in the OS ID, preventing potential URL injection issues and ensuring the application correctly processes IDs with special characters.src/react/src/util/styles.js (1)
1578-1604: Well-structured style definitions for the new reject dialog componentThe new
makeRejectModerationEventDialogStylesfunction is properly implemented with frozen objects for immutability. The style properties cover all necessary UI aspects of the dialog including container dimensions, editor appearance, and button sizing.src/react/package.json (1)
17-18: New dependencies added for rich text editing capabilitiesThe addition of
draft-js,draftjs-to-html, andreact-draft-wysiwygpackages indicates the implementation of rich text editing functionality, likely for the new rejection moderation dialog component. These are appropriate choices for implementing WYSIWYG editing in React.Also applies to: 33-33
src/react/src/actions/dashboardContributionRecord.js (2)
133-138: Function signature properly updated to support rich text rejection reasonsThe updated function signature now accepts the additional parameters
textCleanedandtextRaw, which aligns with the new rich text editing capabilities being added to the application.
145-150: API request properly updated to include rich text dataThe PATCH request now correctly includes the structured text data as
action_reason_text_cleanedandaction_reason_text_rawparameters, enabling the backend to store both the formatted and raw versions of the rejection reason.doc/release/RELEASE-NOTES.md (2)
59-60: Good fix for OS ID encoding issue.This addresses the router redirection issue when OS IDs contain a forward slash by implementing proper encoding with
encodeURIComponent(). This is an important fix as it prevents navigation to unsupported pages.
65-65: Appropriate release note for the new feature.The release note clearly describes the new functionality that links the "My Claimed Facilities" page to SLC when no claimed production locations are found, and also mentions the search button text change. This aligns with the PR objectives.
src/react/src/__tests__/components/DashboardContributionRecord.test.js (3)
3-3: Appropriate import addition for testing user interactions.Adding the
fireEventimport from@testing-library/reactis necessary for simulating user interactions in the new test cases.
34-45: Well-structured mock for the RejectModerationEventDialog component.The mock implementation effectively simulates the dialog's behavior, providing a button to close it when open. This approach allows testing the component's integration without depending on its internal implementation details.
301-320: Comprehensive tests for dialog open/close functionality.These test cases thoroughly verify that:
- Clicking the "Reject Contribution" button opens the dialog
- Clicking the "Close Dialog" button successfully closes the dialog
This ensures the dialog interaction works as expected and improves test coverage.
src/react/src/__tests__/components/SearchByOsIdTab.test.js (1)
71-120: Thorough test for URL encoding of special characters.This test appropriately verifies that special characters in OS IDs are correctly encoded in the URL, testing multiple characters (
/,?,#,$,&,@). This helps ensure the fix for OSDEV-1838 works properly across various scenarios.src/react/src/components/Dashboard/DashboardContributionRecord.jsx (6)
22-22: Good addition of the new dialog component import.The import of
RejectModerationEventDialogis correctly added to support the enhanced moderation rejection workflow.
90-93: Well-structured state management for dialog visibility.The state variable
rejectModerationEventDialogIsOpenand its setter function are properly implemented to manage the dialog's visibility.
169-175: Clear handler functions for dialog management.The implementation of
handleRejectContributionandhandleRejectModerationEventDialogCloseprovides a clean way to open and close the dialog, improving code readability and maintainability.
403-403: Proper update to button click handler.Replacing the inline function with
handleRejectContributionimproves code organization by moving the logic to a separate named function.
431-435: Correct implementation of the dialog component.The
RejectModerationEventDialogcomponent is properly added to the render output with the necessary props for controlling its visibility and behavior.
494-502: Enhanced action dispatching with additional parameters.The
updateModerationEventfunction now acceptstextCleanedandtextRawparameters, allowing for more detailed information to be passed when updating moderation events. This is an important enhancement for the rejection workflow.src/react/src/__tests__/components/RejectModerationEventDialog.test.js (1)
1-144: Well-structured and comprehensive test suite for the RejectModerationEventDialog component.The test file is thoroughly implemented with good coverage for all the component's key functionality:
- Proper rendering verification
- Button interaction testing
- Conditional button state validation
- Event handler validation
- Tooltip behavior verification
The mock implementations for the WYSIWYG editor and tooltip are well-designed, allowing for isolated testing of the component without external dependencies.
src/react/src/components/Dashboard/RejectModerationEventDialog.jsx (1)
1-148: Well-implemented dialog component with appropriate validation and UX considerations.The component is well-structured with:
- Good state management using React hooks
- Appropriate validation for minimum text length
- Clear user feedback through the tooltip
- Well-defined prop types
- Logical separation of concerns
The implementation aligns perfectly with the test coverage seen in the test file. The use of
useMemofor deriving content state is a good optimization..github/workflows/deploy_to_aws.yml (4)
125-125: Job naming updated to better reflect its purpose.The job has been appropriately renamed from
build_and_push_docker_imagetobuild_and_push_react_appto better reflect its responsibility.
160-164: Added step to convert project name to lowercase for consistent resource naming.This change helps ensure consistent resource naming across AWS services by standardizing the project name to lowercase.
166-189: Improved CloudFront distribution management with robust error handling.The updated script includes:
- More robust error handling to detect when no CloudFront distribution is found
- Prevention of ambiguity when multiple distributions match the domain
- Clear error messages for debugging
- Proper environment variable usage for AWS credentials
These improvements will make the deployment process more reliable.
191-205: Added separate job for Docker image building with proper dependencies.The workflow has been restructured to separate concerns:
- Building the React application
- Building and pushing Docker images
This separation improves workflow organization and maintainability by ensuring the React app is built before Docker images are created.
deployment/terraform/lambda-functions/redirect_to_s3_origin/index.mjs (1)
3-11: Lambda function looks good, well-structured for its purpose.This Lambda@Edge function correctly intercepts CloudFront requests and redirects routes without file extensions to the SPA's index.html, which is essential for client-side routing in single-page applications.
deployment/terraform/cdn.tf (6)
1-12: Good practice using locals for bucket naming.Using locals for the frontend bucket name ensures consistency and reuse of the naming pattern throughout the configuration.
14-40: Security best practices applied to S3 bucket configuration.The S3 bucket is properly configured with server-side encryption and appropriate ownership controls, which aligns with security best practices.
42-86: Well-defined IAM policy for secure bucket access.The policy correctly:
- Denies insecure transport (enforcing HTTPS)
- Restricts access to CloudFront using origin access identity
- Uses specific ARNs and actions to follow least privilege principle
117-129: New S3 origin added correctly with proper authentication.The CloudFront distribution has been properly configured with the new S3 origin, including origin access identity and the necessary authentication header.
139-163: Default cache behavior now points to S3 origin with Lambda integration.The default cache behavior has been updated to use the S3 origin and includes the Lambda function association for client-side routing. This is essential for properly serving the SPA from S3.
165-647: Comprehensive ordered cache behaviors for API endpoints.The configuration includes well-defined cache behaviors for various API endpoints and static assets, ensuring they are correctly routed to the ALB origin while the SPA is served from S3. The cache settings are appropriate for each type of content.
deployment/terraform/lambda.tf (3)
95-106: IAM policy follows least privilege principle for Lambda@Edge.The assume role policy correctly specifies both
lambda.amazonaws.comandedgelambda.amazonaws.comas service principals, which is required for Lambda@Edge functions.
108-130: Appropriate logging permissions for Lambda function.The execution role policy provides the minimum necessary permissions for CloudWatch logging, following security best practices.
148-151: Appropriate log retention period configured.The CloudWatch log group has a reasonable retention period of 14 days, which balances storage costs with the need for troubleshooting information.
src/django/requirements.txt (1)
46-46:Details
❓ Verification inconclusive
Added django-bleach dependency for HTML sanitation.
This addition is appropriate for the new BleachField used in the ModerationEvent model. The package provides HTML sanitization capabilities which will help prevent XSS attacks when displaying user-provided content.
🏁 Script executed:
#!/bin/bash # Check if the installed version is the latest available pip index versions django-bleach | head -n 3Length of output: 402
Action Required: Confirm the django-bleach version choice
The added dependency is appropriate for enabling HTML sanitization via the new BleachField in the ModerationEvent model. However, the verification output shows that while the project currently pins to
django-bleach==2.0.0, the latest available version is 3.1.0. Please verify that using version 2.0.0 is an intentional decision (e.g., for compatibility reasons) rather than an oversight, and consider updating if no constraints exist.
- Confirm that BleachField in ModerationEvent is fully supported by version 2.0.0.
- If appropriate, update to the latest version (3.1.0) for additional features and security improvements, or document why the earlier version is chosen.
src/django/api/templates/mail/slc_contribution_rejected_body.html (2)
1-1: Added template tag for HTML sanitization.The bleach_tags are correctly loaded at the top of the template to enable HTML sanitization functionality.
25-25: Improved display of rejection feedback with sanitization.The feedback text is now properly displayed with the bleach filter, which sanitizes the HTML content before rendering it to prevent XSS attacks. This is a good security practice.
src/django/api/models/moderation_event.py (2)
3-3: Added BleachField import for HTML sanitization.This import is needed for the new action_reason_text_raw field that will sanitize HTML content.
151-159: Added fields for storing rejection reasons with proper sanitization.The two new fields serve distinct purposes:
action_reason_text_cleanedstores a version that has been sanitized programmaticallyaction_reason_text_rawuses a BleachField which automatically sanitizes HTML inputThis is a good approach for handling user-provided content that needs to be displayed safely.
src/django/api/migrations/0167_add_moderationevent_action_reason_text_fields.py (1)
1-31: Well-structured migration for adding the new fields.This migration correctly adds the two new fields to the ModerationEvent model with appropriate field types and options. The dependency on the previous migration is properly set.
src/django/api/views/v1/moderation_events.py (1)
130-138: Refined error handling for validation errorsThe error handling now prioritizes non-field errors over field-specific errors, providing more focused error messages for validation issues. This change aligns well with the new validation logic for rejection reasons in the serializer.
src/django/oar/settings.py (2)
134-134: LGTM: Added django_bleach to INSTALLED_APPSThis addition is necessary to enable HTML sanitization for the new moderation event reason text fields.
568-582: Well-configured HTML sanitization settingsThe Django Bleach configuration is properly set up with a good selection of allowed HTML tags and attributes. The settings appropriately restrict potentially dangerous HTML while allowing enough formatting capabilities for rejection reasons.
src/django/api/serializers/v1/moderation_event_update_serializer.py (4)
14-17: LGTM: Added new fields for rejection reasonsGood implementation of the minimum length constant and fields for storing both cleaned and raw versions of the rejection reason text.
40-41: LGTM: Updated Meta.fields to include new fieldsThe Meta.fields list is properly updated to include the new rejection reason text fields.
87-128: Comprehensive validation for rejection reasonsExcellent implementation of validation logic for rejection reasons. The code properly checks for:
- The presence of both cleaned and raw text fields when rejecting
- Minimum length requirements for both fields
- Appropriate error messages that clearly explain the validation requirements
136-141: LGTM: Updated instance with reason text fieldsThe update method correctly assigns the reason text fields to the instance when rejecting a moderation event.
src/django/api/tests/test_moderation_events_update.py (4)
21-24: LGTM: Disconnected signal for cleaner testsGood practice to disconnect the OpenSearch signal during tests to prevent side effects.
111-151: Well-updated test for valid rejectionThe test has been properly renamed and updated to include the new required fields for rejection. The assertions correctly verify that the reason text fields are stored in the model.
153-188: LGTM: Comprehensive test for invalid reason lengthGood test case that verifies the validation errors when the reason text is too short. The assertions properly check the response status, error structure, and that the moderation event remains in the PENDING state.
189-221: LGTM: Comprehensive test for missing reasonExcellent test case that verifies the validation errors when the required reason text fields are missing. The assertions properly check the response status, error structure, and that the moderation event remains in the PENDING state.
src/react/src/__tests__/components/ProductionLocationDialog.test.js (2)
82-82: Test updated to reflect UI terminology changeThe test now expects "Location name" to appear twice in the document instead of checking for "Facility name", aligning with the terminology changes in the component.
92-92: Added assertion for Location Type fieldThis new assertion ensures the "Location Type" label is present in the dialog, which corresponds to the transformation of 'facility_type' to 'location_type' in the ProductionLocationDialogFields component.
src/react/src/components/Contribute/ProductionLocationDialogFields.jsx (2)
75-75: Applied terminology transformation consistentlyThe code correctly implements the terminology transformation in three places:
- Creating a transformed title using the new function
- Using the transformed title as the div's key
- Displaying the formatted transformed title in the UI
This ensures consistent terminology across the UI, changing "Facility Type" to "Location Type" as part of what appears to be a broader terminology update throughout the application.
Also applies to: 81-81, 83-83
12-20:Details
✅ Verification successful
Added temporary label transformation function with TODO comment
The TODO comment appropriately references the related Jira ticket (OSDEV-1657) for future refactoring. The function provides a clean way to transform terminology from "facility" to "location" during this transition phase.
🏁 Script executed:
#!/bin/bash # Check if the referenced Jira ticket exists in other parts of the codebase echo "Checking for references to OSDEV-1657 in the codebase..." rg -i "OSDEV-1657" --type js --type jsxLength of output: 190
Attention: Revised OSDEV-1657 Verification Required
The temporary label transformation function and its associated TODO comment referencing OSDEV-1657 are implemented as expected. However, our initial verification attempt using ripgrep with--type jsxencountered an error (i.e., "unrecognized file type: jsx"). Please run the revised script above—which uses the--glob "*.{js,jsx}"option—to verify that there are no additional occurrences of "OSDEV-1657" in your JavaScript or JSX files.Once the updated search confirms that no extra references exist, the current implementation can be considered valid during this transition phase.
🏁 Script executed:
#!/bin/bash echo "Searching for references to OSDEV-1657 in .js and .jsx files..." rg -i "OSDEV-1657" --glob "*.{js,jsx}"Length of output: 475
Final Verification: Temporary Label Transformation Function is Valid
The transformation function in
src/react/src/components/Contribute/ProductionLocationDialogFields.jsx(lines 12–20) correctly implements a temporary label conversion, as expected for the transition phase referenced in OSDEV-1657. A subsequent search confirmed that the only reference to OSDEV-1657 is this TODO comment, indicating no stray occurrences elsewhere. The implementation is clean and aligned with the intended interim refactoring.
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
doc/release/RELEASE-NOTES.md (1)
62-62: Toggle Switch for Production Location Info Page
A new entry mentions the addition of a toggle switch button to the production location info page that controls whether additional data is sent with the request. Please ensure that the functionality is thoroughly tested and that the UI tooltip or documentation clearly explains the purpose of the toggle.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
doc/release/RELEASE-NOTES.md(1 hunks)doc/release/RELEASE-NOTES.md(1 hunks)doc/release/RELEASE-NOTES.md(1 hunks)doc/release/RELEASE-NOTES.md(1 hunks)doc/release/RELEASE-NOTES.md(2 hunks)doc/release/RELEASE-NOTES.md(1 hunks)doc/release/RELEASE-NOTES.md(1 hunks)doc/release/RELEASE-NOTES.md(1 hunks)src/react/src/Routes.jsx(2 hunks)src/react/src/__tests__/components/AddLocationData.test.js(2 hunks)src/react/src/components/AddLocationData.jsx(0 hunks)src/react/src/util/constants.jsx(0 hunks)
💤 Files with no reviewable changes (2)
- src/react/src/components/AddLocationData.jsx
- src/react/src/util/constants.jsx
🚧 Files skipped from review as they are similar to previous changes (6)
- doc/release/RELEASE-NOTES.md
- doc/release/RELEASE-NOTES.md
- doc/release/RELEASE-NOTES.md
- doc/release/RELEASE-NOTES.md
- doc/release/RELEASE-NOTES.md
- doc/release/RELEASE-NOTES.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
doc/release/RELEASE-NOTES.md
15-15: Trailing punctuation in heading
Punctuation: ':'
(MD026, no-trailing-punctuation)
⏰ Context from checks skipped due to timeout of 90000ms (10)
- GitHub Check: run-flake8-linter
- GitHub Check: run-integration-test-code-quality
- GitHub Check: run-django-code-quality
- GitHub Check: run-dd-code-quality
- GitHub Check: run-countries-code-quality
- GitHub Check: run-contricleaner-code-quality
- GitHub Check: get-base-branch-django-cov
- GitHub Check: get-base-branch-contricleaner-cov
- GitHub Check: get-base-branch-dd-cov
- GitHub Check: get-base-branch-countries-cov
🔇 Additional comments (14)
src/react/src/__tests__/components/AddLocationData.test.js (3)
6-6: Great addition of necessary route constantsThis import adds the required constants for the new tests, ensuring proper verification of button routing.
54-59: Well-structured test for the "Upload Multiple Locations" buttonThis test appropriately verifies that the "Upload Multiple Locations" button is both enabled and correctly links to the
multipleLocationRoute. The test follows best practices by using role-based queries for better accessibility testing.
61-66: Well-structured test for the "Add a Single Location" buttonThis test properly verifies that the "Add a Single Location" button is enabled and correctly links to the SLC flow via
contributeProductionLocationRoute, which aligns with the PR objective of linking to the Single Location Contribution flow.src/react/src/Routes.jsx (3)
51-51: Good addition of multipleLocationRoute importAdding this import correctly supports the route changes implemented below.
162-163: Appropriate route update for the Contribute componentChanging the path from
contributeRoutetomultipleLocationRoutefor the Contribute component ensures the "Upload Multiple Locations" button correctly navigates to this component, which matches the test implemented in AddLocationData.test.js.
167-169: Correct route update for the AddLocationData componentUpdating the AddLocationData component's path to use
contributeRouteproperly implements the routing structure changes needed to support the PR objective of improving navigation to the SLC flow.doc/release/RELEASE-NOTES.md (8)
16-16: Migration Addition for ModerationEvent Fields
The new migration0167_add_moderationevent_action_reason_text_fields.pyadds the fieldsaction_reason_text_cleanedandaction_reason_text_rawto theapi_moderationeventtable. Please verify that this migration handles existing data safely (e.g., defaults or nullability) and that any needed indexes or constraints are added if required by future queries.
19-19: Schema Update for ModerationEvent Fields
New fieldsaction_reason_text_cleanedandaction_reason_text_raware introduced to theapi_moderationeventtable (see OSDEV-1782). Ensure that the schema update is fully aligned with the corresponding business logic and that any necessary data migration or backfilling is accounted for.
22-22: Enhanced Validation for PATCH Endpoint
The release notes now document additional validation for the fieldsaction_reason_text_cleanedandaction_reason_text_rawwhen thePATCH api/v1/moderation-events/{moderation_id}endpoint is used in a 'REJECTED' context. Confirm that:
- The minimum length constraint (30 characters) is implemented in both the serializer and unit tests.
- The validation error messages are clear and user friendly.
23-23: Server-side HTML Sanitization Implementation
Sanitization using theDjango-Bleachlibrary has been applied to theaction_reason_text_rawvalue (in theslc_contribution_rejected_body.htmltemplate). Please review the Bleach configuration (i.e., allowed tags, attributes, and styles) to ensure it meets security standards and that it does not inadvertently strip out needed formatting.
61-61: 'What's New' Section Header Update
The header "### What's new" has been introduced/updated. Ensure that this header and the subsequent entries adhere to our project’s documentation style guidelines so that all users can clearly understand the release highlights.
63-63: Confirmation Dialog for Rejection Flow
A confirmation dialog window featuring a WYSIWYG editor has been added to enforce a minimum input length (30 characters) when rejecting a moderation event. It is important to verify that both the client-side and server-side validations offer consistent feedback to the user. Additionally, check that the dialog disables the 'Reject' button appropriately until the requirement is met.
64-64: Link Update for 'My Claimed Facilities' Page
The release notes now state that the "My Claimed Facilities" page is linked to the SLC (via/contribute/single-location?tab=name-address) when no claimed production locations are found, and that the search button text has been updated. Please ensure that this navigation change has been fully integrated and is verified in user acceptance testing, so the new behavior is consistent with user expectations.
65-65: Formatting and Spacing Consistency
A trailing new line or spacing adjustment appears at line 65. Confirm that these formatting changes are consistent with our markdown styling guidelines and do not affect the rendered output.
4 participants
❗ Merge before SLC final release ❗
Fix OSDEV-1786
/contribute/single-location?tab=name-addressas this is default SLC starting point (see OSDEV-1745)).