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

Ccap 469 create providerresponse registration tax id ein screen #998

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Ccap 469 create providerresponse registration tax id ein screen #998

wants to merge 10 commits into from

Conversation

enyia21
Copy link
Contributor

@enyia21 enyia21 commented Jan 9, 2025

🔗 Jira ticket

CCAP-469

✍️ Description

📷 Design reference

Figma
Content Manager

✅ Completion tasks

  • Added relevant tests
  • Meets acceptance criteria

@enyia21 enyia21 temporarily deployed to il-gcc-ccap-469-create--2mlvp0 January 9, 2025 16:53 Inactive
@enyia21 enyia21 temporarily deployed to il-gcc-ccap-469-create--2mlvp0 January 9, 2025 19:09 Inactive
@enyia21 enyia21 temporarily deployed to il-gcc-ccap-469-create--2mlvp0 January 9, 2025 19:25 Inactive
@enyia21 enyia21 force-pushed the CCAP-469-Create-providerresponse-registration-tax-id-ein-screen branch from 22b04cc to bbadfd5 Compare January 9, 2025 19:28
@enyia21 enyia21 temporarily deployed to il-gcc-ccap-469-create--2mlvp0 January 9, 2025 19:28 Inactive
@enyia21 enyia21 marked this pull request as ready for review January 9, 2025 19:43
@github-actions github-actions bot requested a review from analoo January 9, 2025 19:43
@enyia21 enyia21 requested review from cram-cfa and removed request for analoo January 9, 2025 19:51
analoo
analoo requested changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@analoo analoo left a comment

Choose a reason for hiding this comment

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

Please add tests: ProviderresponseProviderRegistrationJourneyTest.java

@enyia21 enyia21 temporarily deployed to il-gcc-ccap-469-create--2mlvp0 January 10, 2025 18:36 Inactive
@enyia21 enyia21 requested a review from analoo January 10, 2025 18:42
analoo
analoo requested changes Jan 10, 2025
View reviewed changes
Copy link
Contributor

@analoo analoo left a comment

Choose a reason for hiding this comment

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

Please add tests: ProviderresponseProviderRegistrationJourneyTest.java

analoo
analoo reviewed Jan 10, 2025
View reviewed changes
src/main/java/org/ilgcc/app/submission/actions/ValidateEIN.java Outdated

Locale locale = LocaleContextHolder.getLocale();

String regex = "\\d{9}";
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of creating an action, you can add a validation to the providerresponse input file:

See gcc for an example:
@pattern(regexp = "\d{3}-\d{2}-\d{4}", message = "{errors.invalid-ssn}")
@Encrypted
private String parentSsn;

Copy link
Contributor

Choose a reason for hiding this comment

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

This validation actually forces an EIN to be added but the copy states "This isn't required to register for CCAP, but will be required in order to actually get paid by CCAP."

Doing it as a pattern annotation means that the pattern is checked if the value isn't null.

Copy link
Contributor

Choose a reason for hiding this comment

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

Screenshot 2025-01-10 at 10 57 55 AM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind

analoo
analoo reviewed Jan 10, 2025
View reviewed changes
src/main/java/org/ilgcc/app/submission/actions/ValidateTaxIdSSN.java Outdated
@@ -37,7 +37,7 @@ public Map<String, List<String>> runValidation(FormSubmission formSubmission, Su

if (!requiredSSNMatcher.matches()) {
errorMessages.put(INPUT_NAME,
List.of(messageSource.getMessage("registration-home-provider-ssn.error", null, locale)));
List.of(messageSource.getMessage("registration-tax-id-ssn.error", null, locale)));
Copy link
Contributor

Choose a reason for hiding this comment

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

Same here:

Instead of creating an action, you can add a validation to the providerresponse input file:

See gcc for an example:
@pattern(regexp = "\d{3}-\d{2}-\d{4}", message = "{errors.invalid-ssn}")
@Encrypted
private String parentSsn;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nevermind

@enyia21 enyia21 temporarily deployed to il-gcc-ccap-469-create--2mlvp0 January 10, 2025 19:58 Inactive
@enyia21 enyia21 temporarily deployed to il-gcc-ccap-469-create--2mlvp0 January 10, 2025 22:01 Inactive
@enyia21 enyia21 force-pushed the CCAP-469-Create-providerresponse-registration-tax-id-ein-screen branch from 0eadd40 to f560916 Compare January 10, 2025 22:14
@enyia21 enyia21 temporarily deployed to il-gcc-ccap-469-create--2mlvp0 January 10, 2025 22:14 Inactive
@enyia21 enyia21 temporarily deployed to il-gcc-ccap-469-create--2mlvp0 January 10, 2025 22:33 Inactive
@enyia21 enyia21 requested a review from analoo January 10, 2025 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cram-cfa cram-cfa Awaiting requested review from cram-cfa

@analoo analoo Awaiting requested review from analoo

Requested changes must be addressed to merge this pull request.

Assignees

@enyia21 enyia21

Labels
Ready For Review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@enyia21 @analoo