[OSDEV-1831] Update text of tooltip on the "Thank you for adding data" pop-up base on the claim status of the production location.#551
Conversation
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
mazursasha1990
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
had a problem deploying
to
Quality Environment
GitHub Actions
Failure
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
React App | Jest test suite - Code coverage reportTotal: 32.88%Your code coverage diff: 0.03% ▴ ✅ All code changes are covered |
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
mazursasha1990
temporarily deployed
to
Quality Environment
GitHub Actions
Inactive
📝 WalkthroughWalkthroughThis pull request updates the tooltip messages in the production location dialog to improve clarity. Specifically, it adjusts the wording for pending reviews and claimed statuses in the Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (13)
🪧 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 (
|
![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 (1)
deployment/terraform/anonymized_database_dump_scheduled_task/docker/dump.sh (1)
20-21: Consider addressing shellcheck warningThere's a potential issue with masking return values when declaring and assigning a variable in the same command.
- export SNAPSHOT_STATUS=$(aws rds describe-db-snapshots --db-snapshot-identifier $ANONYMIZED_SNAPSHOT_ID --query 'DBSnapshots[0].Status' --output text) + SNAPSHOT_STATUS=$(aws rds describe-db-snapshots --db-snapshot-identifier $ANONYMIZED_SNAPSHOT_ID --query 'DBSnapshots[0].Status' --output text) + export SNAPSHOT_STATUS🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 20-20: Declare and assign separately to avoid masking return values.
(SC2155)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (21)
src/react/src/components/Contribute/ProductionLocationDialog.jsx(2 hunks)src/react/src/components/Contribute/ProductionLocationDialog.jsx(1 hunks)src/react/src/components/Contribute/ProductionLocationDialog.jsx(0 hunks)src/react/src/components/Contribute/ProductionLocationDialog.jsx(1 hunks)src/react/src/components/Contribute/ProductionLocationDialog.jsx(1 hunks)src/react/src/components/Contribute/ProductionLocationDialog.jsx(1 hunks)src/react/src/components/Contribute/ProductionLocationDialog.jsx(1 hunks)deployment/environments/terraform-preprod.tfvars(1 hunks)deployment/environments/terraform-production.tfvars(1 hunks)deployment/terraform/anonymized_database_dump_scheduled_task/docker/dump.sh(1 hunks)doc/release/RELEASE-NOTES.md(2 hunks)src/react/src/Routes.jsx(2 hunks)src/react/src/__tests__/components/AddLocationData.test.js(2 hunks)src/react/src/__tests__/components/ClaimedFacilitiesList.test.js(1 hunks)src/react/src/__tests__/components/SearchByOsIdTab.test.js(1 hunks)src/react/src/components/AddLocationData.jsx(0 hunks)src/react/src/components/ClaimedFacilitiesList.jsx(4 hunks)src/react/src/components/Contribute/SearchByOsIdTab.jsx(1 hunks)src/react/src/util/constants.jsx(0 hunks)src/react/src/components/Contribute/ProductionLocationDialog.jsx(1 hunks)doc/release/RELEASE-NOTES.md(1 hunks)
💤 Files with no reviewable changes (2)
- src/react/src/util/constants.jsx
- src/react/src/components/AddLocationData.jsx
🧰 Additional context used
🪛 Shellcheck (0.10.0)
deployment/terraform/anonymized_database_dump_scheduled_task/docker/dump.sh
[warning] 20-20: Declare and assign separately to avoid masking return values.
(SC2155)
🔇 Additional comments (23)
deployment/environments/terraform-production.tfvars (1)
54-54: Memory increase for app_dd_fargate_memoryIncreasing the memory allocation from 12288 to 16384 (4GB more) will provide additional resources for the application, which could help with performance. However, this change should be coordinated with the PR's objective of updating tooltip text, as memory increases typically address performance issues rather than UI text changes.
Could you confirm if this memory increase is directly related to the tooltip updates mentioned in the PR objective, or if it's addressing a separate performance concern?
deployment/environments/terraform-preprod.tfvars (1)
55-55: Consistent memory increase across environmentsGood practice to maintain parity between preprod and production environments by making the same memory increase to 16384 in both. This ensures consistent testing and behavior between environments.
deployment/terraform/anonymized_database_dump_scheduled_task/docker/dump.sh (2)
16-17: Added timeout mechanism for snapshot creationAdding attempt tracking variables with a maximum of 600 attempts provides a proper timeout mechanism rather than waiting indefinitely. With the 60-second sleep interval, this allows for up to 10 hours of waiting, which seems reasonable for large database snapshots.
25-36: Improved error handling with informative messagesThe new implementation provides more detailed status updates and properly handles the case when snapshot creation times out. This is a significant improvement over the previous implementation.
src/react/src/components/Contribute/SearchByOsIdTab.jsx (1)
33-39:Details
❓ Verification inconclusive
Added URL encoding for OS ID in search path
Implementing
encodeURIComponentfor the OS ID when constructing the URL path is an excellent security practice that prevents URL injection and ensures proper handling of special characters.This change aligns with the PR objective by improving how data is handled in the application, which could be related to the tooltip display issues based on claim status.
🏁 Script executed:
#!/bin/bash # Check for any special character handling tests in the codebase rg -A 5 "encodeURIComponent" --glob "*.test.js"Length of output: 47
Approval with Caution: Verify Test Coverage for URL Encoding
The use of
encodeURIComponentin thehandleSearchfunction ofSearchByOsIdTab.jsxcorrectly secures the URL by preventing potential injection issues and ensuring that special characters in the OS ID are properly handled. However, a recent search for tests verifying this behavior did not return any results. Please manually verify that there is adequate test coverage for this functionality or consider adding tests to ensure that special character inputs are encoded correctly.
- File affected:
src/react/src/components/Contribute/SearchByOsIdTab.jsx(Lines 33-39)src/react/src/__tests__/components/AddLocationData.test.js (3)
6-6: Appropriate import added for route constantsAdding specific route constants from the constants file is a good practice to ensure consistent routing across the application.
54-59: Good test coverage for Upload Multiple Locations buttonThis test properly verifies both the enabled state and the correct route for the "Upload Multiple Locations" button, which aligns with the PR objective of ensuring proper routing based on claim status.
61-66: Good test coverage for Add a Single Location buttonThis test properly verifies both the enabled state and the correct route for the "Add a Single Location" button. Having this test is important since it validates changes related to the PR objective.
src/react/src/components/ClaimedFacilitiesList.jsx (3)
3-3: Good imports for improved navigationAdding the Link component and the route constant improves maintainability and makes the code more robust.
Also applies to: 17-17
53-53: Fixed deprecated Typography variantChanging from "body" to "body1" fixes the use of a deprecated variant. This helps future-proof the code.
63-68: Improved navigation and button textThe changes improve user experience by:
- Using the Link component for proper navigation
- Using a more descriptive button text "Find My Production Location" instead of generic "Search"
- Passing the correct query parameter to navigate to the name-address tab
This aligns well with the PR objective to improve UI text based on claim status.
src/react/src/__tests__/components/SearchByOsIdTab.test.js (1)
71-120: Great test for URL encoding of special charactersThis test thoroughly validates that special characters in OS IDs are properly encoded in the URL. Testing multiple special characters (/, ?, #, $, &, @) ensures robust handling of user inputs.
This is an important security practice to prevent URL injection attacks and ensure proper routing.
src/react/src/components/Contribute/ProductionLocationDialog.jsx (3)
78-78: Improved clarity in pending status tooltip messageThe message now more clearly states that the submission is "being reviewed" and that the email will be sent "once the review is complete" instead of "once the production location is live on OS Hub". This provides more accurate expectations to users about when they'll receive follow-up communication.
84-84: Clearer explanation for claimed locationsThe message now directly states that the location "has already been claimed and therefore cannot be claimed again" which is more straightforward than the previous wording. This aligns with the PR objective to improve tooltip text based on claim status.
88-88: More accurate default claim tooltip messageThe updated message now correctly states that claiming will be possible "once the review is complete" rather than tying it to when the OS ID is live. This provides clearer guidance to users about when the claiming functionality will become available.
src/react/src/Routes.jsx (3)
51-51: Import of new route constant looks good.The addition of
multipleLocationRouteto the imports aligns with the PR objective to update routing based on claim status.
162-162: Route path update for Contribute component is appropriate.Changed from
contributeRoutetomultipleLocationRoute, which aligns with the routing strategy changes described in the PR objectives.
167-167:Details
✅ Verification successful
Route path update for AddLocationData component is appropriate.
Changed from
contributeNewRoutetocontributeRoute, which completes the swap of routing paths between these two components.
🏁 Script executed:
#!/bin/bash # Check if there are any references to the old contributeNewRoute that might need updating rg -A 1 -B 1 "contributeNewRoute" --type jsLength of output: 43
Route Update Confirmed – Please Verify Repository-wide
The update in
src/react/src/Routes.jsx(line 167) that changes the route path fromcontributeNewRoutetocontributeRoutefor the AddLocationData component completes the intended swap between the two components. However, note that the initial grep was restricted to.jsfiles and may not have covered.jsxor other file types where references tocontributeNewRoutemight exist.
- File to review:
src/react/src/Routes.jsx(line 167)- Action item: Run a repository-wide search (e.g., using the script above) to ensure no lingering references to
contributeNewRouteremain.Once you’ve manually verified that all instances of
contributeNewRoutehave been successfully replaced, this change can be approved.
🏁 Script executed:
#!/bin/bash # Perform a comprehensive search for 'contributeNewRoute' across the entire repository rg -A 1 -B 1 "contributeNewRoute" .Length of output: 157
Route Path Update Verified – All References Cleared
The update in
src/react/src/Routes.jsxat line 167—changing the path fromcontributeNewRoutetocontributeRoutefor the AddLocationData component—has been verified. A comprehensive search (across both.jsand.jsxfiles) did not reveal any remaining references tocontributeNewRoute, confirming the intended swap between the two components.
- File to review:
src/react/src/Routes.jsx(line 167)- Action: No further references found; the update appears complete.
src/react/src/__tests__/components/ClaimedFacilitiesList.test.js (1)
1-58: New test file looks good with proper test coverage.This test file verifies that when no claimed production locations are available, a button with the correct text and href is rendered. The test setup, mocking, and assertions are well-structured.
The button href points to
/contribute/single-location?tab=name-address, which aligns with the updated routing strategy mentioned in the PR objectives.doc/release/RELEASE-NOTES.md (4)
61-61: Formatting improvement to existing release note entry.Minor formatting change to the entry regarding the update of text from
Facility TypetoLocation TypeandFacility NametoLocation Name.
62-62: Good bug fix documentation.Clear explanation of the fix for router redirection issues when OS ID contains a forward slash, mentioning the implementation of
encodeURIComponent()for encoding the OS ID.
63-63: Clear bug fix documentation.Well-documented fix for the snapshot status checking procedure to prevent crashes when restoring from inaccessible snapshots.
67-67: New feature documentation is clear.This entry clearly describes the linkage of "My Claimed Facilities" page to SLC when no claimed production locations are found, and mentions the search button text update.
…nk-you-for-adding-data-pop-up
|
3 participants
OSDEV-1831 - SLC UAT Bug - Update copy in "Thank you for adding data" pop-up (new OS ID use case).
This PR updates copies of tooltips on the “Thank you for adding data” pop-up. The texts vary depending on the claim status for a particular location.