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

2681 s1 enhancements #2817

Merged
merged 38 commits into from
Mar 26, 2024
Merged

2681 s1 enhancements #2817

merged 38 commits into from
Mar 26, 2024

Conversation

elipe17
Copy link

@elipe17 elipe17 commented Jan 23, 2024

Summary of Changes

  • Updated WORK_PART_STATUS
  • Added missing cat3 validator
  • Updated FUNDING_STREAM validators
  • Updated GENDER validators
  • Updated OTHER_AMOUNT & TRANSITION_SERVICES_AMOUNT validators
  • Updated DISPOSITION validators

Note: The FIPS/Tribe code update already exists because of the relation in the database. Accessing those fields within the record has also been covered in #2805 by adding STT as a queryable field in the search indices.

Pull request closes #2681

How to Test

List the steps to test the PR
These steps are generic, please adjust as necessary.

cd tdrs-frontend && docker-compose up
cd tdrs-backend && docker-compose up
pytest -k test_parse.py

Deliverables

More details on how deliverables herein are assessed included here.

Deliverable 1: Accepted Features

Checklist of ACs:

  • validation checks noted above are accounted for
  • Testing Checklist has been run and all tests pass
  • README is updated, if necessary

Deliverable 2: Tested Code

  • Are all areas of code introduced in this PR meaningfully tested?
    • If this PR introduces backend code changes, are they meaningfully tested?
    • If this PR introduces frontend code changes, are they meaningfully tested?
  • Are code coverage minimums met?
    • Frontend coverage: [insert coverage %] (see CodeCov Report comment in PR)
    • Backend coverage: [insert coverage %] (see CodeCov Report comment in PR)

Deliverable 3: Properly Styled Code

  • Are backend code style checks passing on CircleCI?
  • Are frontend code style checks passing on CircleCI?
  • Are code maintainability principles being followed?

Deliverable 4: Accessible

  • Does this PR complete the epic?
  • Are links included to any other gov-approved PRs associated with epic?
  • Does PR include documentation for Raft's a11y review?
  • Did automated and manual testing with iamjolly and ttran-hub using Accessibility Insights reveal any errors introduced in this PR?

Deliverable 5: Deployed

  • Was the code successfully deployed via automated CircleCI process to development on Cloud.gov?

Deliverable 6: Documented

  • Does this PR provide background for why coding decisions were made?
  • If this PR introduces backend code, is that code easy to understand and sufficiently documented, both inline and overall?
  • If this PR introduces frontend code, is that code easy to understand and sufficiently documented, both inline and overall?
  • If this PR introduces dependencies, are their licenses documented?
  • Can reviewer explain and take ownership of these elements presented in this code review?

Deliverable 7: Secure

  • Does the OWASP Scan pass on CircleCI?
  • Do manual code review and manual testing detect any new security issues?
  • If new issues detected, is investigation and/or remediation plan documented?

Deliverable 8: User Research

Research product(s) clearly articulate(s):

  • the purpose of the research
  • methods used to conduct the research
  • who participated in the research
  • what was tested and how
  • impact of research on TDP
  • (if applicable) final design mockups produced for TDP development

Copy link

codecov bot commented Jan 23, 2024

Codecov Report

Attention: Patch coverage is 79.31034% with 12 lines in your changes are missing coverage. Please review.

Project coverage is 93.58%. Comparing base (fa65a68) to head (efc1d31).
Report is 5 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #2817      +/-   ##
===========================================
- Coverage    93.63%   93.58%   -0.06%     
===========================================
  Files          266      268       +2     
  Lines         6083     6185     +102     
  Branches       513      526      +13     
===========================================
+ Hits          5696     5788      +92     
- Misses         294      305      +11     
+ Partials        93       92       -1     
Flag Coverage Δ
dev-backend 93.76% <79.31%> (-0.08%) ⬇️
dev-frontend 92.62% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
...-backend/tdpservice/parsers/schema_defs/tanf/t1.py 100.00% <ø> (ø)
...-backend/tdpservice/parsers/schema_defs/tanf/t2.py 100.00% <ø> (ø)
tdrs-backend/tdpservice/parsers/validators.py 89.95% <79.31%> (-3.34%) ⬇️

... and 3 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 55b2470...efc1d31. Read the comment docs.

@elipe17 elipe17 added the raft review This issue is ready for raft review label Jan 23, 2024
- add safety to custom validators where type exceptions could occur
@elipe17 elipe17 requested review from jtimpe and raftmsohani January 26, 2024 18:16
Copy link

@raftmsohani raftmsohani left a comment

Choose a reason for hiding this comment

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

LGTM!

@elipe17 elipe17 added QASP Review and removed raft review This issue is ready for raft review labels Jan 26, 2024
@elipe17 elipe17 requested a review from ADPennington January 26, 2024 18:44
@ADPennington ADPennington added the Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI label Mar 20, 2024
- update test file
@ADPennington ADPennington removed the Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI label Mar 21, 2024
@elipe17 elipe17 requested a review from ADPennington March 21, 2024 14:43
@ADPennington ADPennington added Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI and removed Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI labels Mar 21, 2024
@ADPennington ADPennington added Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI and removed Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI labels Mar 22, 2024
Copy link

@atrimpe atrimpe left a comment

Choose a reason for hiding this comment

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

👍

@ADPennington ADPennington added the Blocked Label for Pull Requests that are currently blocked by a dependency label Mar 25, 2024
@elipe17 elipe17 removed the Blocked Label for Pull Requests that are currently blocked by a dependency label Mar 25, 2024
@ADPennington
Copy link
Collaborator

@elipe17 approval here is pending your input on the test notes here, specifically: when AGE is invalid (i.e. DOB==20 0114), WEI == 11, and HOH == 1: the following errors are raised:
20 0114 does not have exactly 8 digits ✔️
20 must be larger than year 1900 ✔️
If WORK_ELIGIBLE_INDICATOR == 11 and AGE < 19, then RELATIONSHIP_HOH != 1 ⚠️ (unsure why this would be raised since age is invalid)

@elipe17
Copy link
Author

elipe17 commented Mar 26, 2024

@elipe17 approval here is pending your input on the test notes here, specifically: when AGE is invalid (i.e. DOB==20 0114), WEI == 11, and HOH == 1: the following errors are raised: 20 0114 does not have exactly 8 digits ✔️ 20 must be larger than year 1900 ✔️ If WORK_ELIGIBLE_INDICATOR == 11 and AGE < 19, then RELATIONSHIP_HOH != 1 ⚠️ (unsure why this would be raised since age is invalid)

@ADPennington that error arrises because of the exception handling in the new validator. If you would prefer to return a true case during an exception that is a quick change. Let me know your thoughts.

@elipe17 elipe17 added the Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI label Mar 26, 2024
@@ -412,8 +412,8 @@ def dateDayIsValid():
def olderThan(min_age):
"""Validate that value is larger than min_age."""
return make_validator(
lambda value: date.today().year - int(str(value)[:4]) > min_age,
lambda value: (f"{str(value)[:4]} must be less than or equal to {date.today().year - min_age} "
lambda value: datetime.date.today().year - int(str(value)[:4]) > min_age,
Copy link
Collaborator

Choose a reason for hiding this comment

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

just noting for reference that this validator is not needed.

Copy link
Collaborator

@ADPennington ADPennington left a comment

Choose a reason for hiding this comment

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

thanks for the quick fix @elipe17 👍🏾

test notes updated here

Also, I noticed some updates to the olderThan validator, but to my knowledge, that validator isn't needed since we dont calculate age relative to today's date.

@ADPennington ADPennington added Ready to Merge and removed QASP Review Deploy with CircleCI-qasp Deploy to https://tdp-frontend-qasp.app.cloud.gov through CircleCI labels Mar 26, 2024
@andrew-jameson andrew-jameson merged commit de1600c into develop Mar 26, 2024
14 checks passed
@andrew-jameson andrew-jameson deleted the 2681-s1-enhancements branch March 26, 2024 16:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TANF Section 1 validation clean-up
6 participants