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

feat(four fields): This change add required fields to the submission forms #428

Merged
merged 142 commits into from
Mar 15, 2024

Conversation

benjaminpaige
Copy link
Collaborator

@benjaminpaige benjaminpaige commented Mar 5, 2024

Linked Issues to Close

(for the epic)
https://qmacbis.atlassian.net/browse/OY2-27187

test link: https://d3tv3xhp6q8ov4.cloudfront.net/

Description of Changes

  • Input Fields Addition: Added fields for subject, description, types, and subtypes in SPA WAIVER and Medicaid form submission pages, as per Figma design guidelines.
  • Multiple Select Options: Modified the type and subtype fields to support multiple selections, enabling users to specify more than one option as applicable.
  • Unit testing Added: Added unit testing for the ui/api useGet Types fetch data fucntioinality whcih included mocking responses for both endpoints.
  • SQL Handler Update: Enhanced the submit SQL handler to support the insertion of multiple types into the database effectively.
  • Naming Conventions: Updated variable and function names for clarity and consistency, changing type to authority where relevant to better reflect its purpose.
  • Detail Sidebar Addition: Implemented a new DetailSidebar component that dynamically displays relevant sidebar items on the details page, based on the existing data.
  • Common Components: Added new common components to the submission/common directory to standardize the UI and facilitate reuse.

Impact of Changes

These changes introduce new data collection fields in the form submission process, which may require users to adapt to the updated submission requirements. The database schema and related APIs have been updated to accommodate the new data types and structures. No backward-incompatible changes are introduced, but users should review the updated submission process for SPA WAIVER and Medicaid forms.

Additional Notes

  • Figma Design Document: View Design
    *** We are still waiting on final approval form the business for the changes to the ui**
  • This update requires database reindexing of course
  • Reviewers are encouraged to provide feedback on the UI/UX changes in line with the Figma design specifications.

@benjaminpaige benjaminpaige requested a review from hannasage March 10, 2024 02:07
Copy link
Contributor

@mdial89f mdial89f left a comment

Choose a reason for hiding this comment

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

approved 🚀 You made it look easy, to just casually add a new dimension to our type/subtype handling across the board. this is awesome
Note: Deleting a type and/or subtype in seatool is captured and reflects in our UI as expected. All good there.

SPA_TYPE_NAME: type.SPA_TYPE_NAME.replace(/–|—/g, "-"),
};
}) || null,
subTypes:
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -3,10 +3,8 @@
"version": "0.0.0",
"dependencies": {
"@aws-sdk/client-cognito-identity-provider": "^3.350.0",
"@aws-sdk/client-dynamodb": "^3.281.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

wow 👍

@@ -87,6 +87,7 @@ describe("getNextBusinessDayTimestamp", () => {
expect(nextDate).toEqual(Date.UTC(2024, 0, 16)); // Tuesday, midnight utc
});

// TODO: I dont know if its my time zone but this always fails for me in the MST
Copy link
Contributor

Choose a reason for hiding this comment

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

The test date is created for 3pm local, not explicitly eastern. So, 3pm local for you is 5pm east coast. The logic for getNextBizDay includes "if 5pm or after, go to the next day". 3pm your time is exactly 5pm, and should return the next day. Test isn't accounting for being run in other timezones. Wouldn't hold up merge to fix anything, in addition some of this might be changing or have changed with the unit test effort.

{
match_phrase: {
name: {
query: "Do Not Use",
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

maximumRetryAttempts: 0
environment:
region: ${self:provider.region}
osDomain: ${param:osDomain}
events:
- http:
path: /getSeaTypes
path: /getTypes
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

Copy link

@hannasage hannasage left a comment

Choose a reason for hiding this comment

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

This is a hefty one, glad we got it done but we should think about ways to better approach 4300 line change PRs 😰

@benjaminpaige benjaminpaige added the status: TESTING PR is being tested by QA label Mar 11, 2024
@mdial89f mdial89f self-requested a review March 14, 2024 23:44
@benjaminpaige benjaminpaige merged commit f9489a7 into master Mar 15, 2024
16 checks passed
Copy link
Contributor

🎉 This PR is included in version 1.5.0-val.15 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
released on @val status: READY PR is ready for review status: TESTING PR is being tested by QA type: FEAT Submit new features type: REFACTOR Restructure existing code while preserving its external behavior
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants