More cat 4 #2879
More cat 4 #2879
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #2879 +/- ##
===========================================
+ Coverage 92.97% 93.10% +0.12%
===========================================
Files 272 272
Lines 6965 7049 +84
Branches 572 590 +18
===========================================
+ Hits 6476 6563 +87
+ Misses 395 392 -3
Partials 94 94
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
jtimpe
force-pushed
the
2842-cat-4-remaining-s2-validators
branch
from
bebaf2d to
d979685
Compare
| field='RPT_MONTH_YEAR', | ||
| msg=( | ||
| f'There should only be one {t1_model_name} record ' | ||
| f'for a RPT_MONTH_YEAR and CASE_NUMBER.' |
There was a problem hiding this comment.
| f'for a RPT_MONTH_YEAR and CASE_NUMBER.' | |
| f'per RPT_MONTH_YEAR and CASE_NUMBER.' |
| field='RPT_MONTH_YEAR', | ||
| msg=( | ||
| f'There should only be one {t4_model_name} record ' | ||
| f'for a RPT_MONTH_YEAR and CASE_NUMBER.' |
There was a problem hiding this comment.
| f'for a RPT_MONTH_YEAR and CASE_NUMBER.' | |
| f'per RPT_MONTH_YEAR and CASE_NUMBER.' |
|
|
||
| if closure_reason == '01': | ||
| num_errors += self.__validate_case_closure_employment(t4, t5s, ( | ||
| 'At least one person on the case must have employment status = 1:Yes in the same month.' |
There was a problem hiding this comment.
@jtimpe suggested language for this one from the DIGIT team:
At least one person on the case must have employment status = 1:Yes in the same RPT_MONTH_YEAR since CLOSURE_REASON = 1:Employment/excess earnings.
| )) | ||
| elif closure_reason == '99' and not is_ssp: | ||
| num_errors += self.__validate_case_closure_ftl(t4, t5s, ( | ||
| 'At least one person who is HoH or spouse of HoH on case must have FTL months >=60.' |
There was a problem hiding this comment.
@jtimpe do we mean closure_reason == '03' and not is_ssp?
ADPennington
removed
the
Deploy with CircleCI-qasp
| record, | ||
| field='REC_SSI', | ||
| msg=( | ||
| f'{t5_model_name} People in territories must have a valid value for 19E.' |
There was a problem hiding this comment.
| f'{t5_model_name} People in territories must have a valid value for 19E.' | |
| f'{t5_model_name} People in territories must have value = 2:No for 19E.' |
| record, | ||
| field='REC_AID_TOTALLY_DISABLED', | ||
| msg=( | ||
| f'{t5_model_name} People in states shouldn\'t have a value of 1.' |
There was a problem hiding this comment.
| f'{t5_model_name} People in states shouldn\'t have a value of 1.' | |
| f'{t5_model_name} People in states should not have a value of 1.' |
ADPennington
added
the
Deploy with CircleCI-qasp
| record, | ||
| field='REC_SSI', | ||
| msg=( | ||
| f'{t5_model_name} People in states must have a valid value.' |
There was a problem hiding this comment.
@jtimpe can we add the following? I just noticed that the other columns in the error report dont make reference to which field or row this error message is associated with. thats probably another ticket, so in the meantime, we should reference field in the error message:
f'{t5_model_name}: People in states must have a valid value for {field}'
There was a problem hiding this comment.
per our discussion, i've hard-coded the field names into the string for now, and created #2996 to capture a dynamic approach to including the field/friendly name
| record, | ||
| field='REC_AID_TOTALLY_DISABLED', | ||
| msg=( | ||
| f'{t5_model_name} People in states should not have a value of 1.' |
There was a problem hiding this comment.
| f'{t5_model_name} People in states should not have a value of 1.' | |
| f'{t5_model_name} People in states should not have a value of 1 for {field}' |
suggestion, assuming {field} can be inserted. else, need the field name.
| record, | ||
| field='REC_SSI', | ||
| msg=( | ||
| f'{t5_model_name} People in territories must have value = 2:No for 19E.' |
There was a problem hiding this comment.
| f'{t5_model_name} People in territories must have value = 2:No for 19E.' | |
| f'{t5_model_name} People in territories must have value = 2:No for {field}.' |
| record, | ||
| field='REC_AID_TOTALLY_DISABLED', | ||
| msg=( | ||
| f'{t5_model_name} Adults in territories must have a valid value for 19C.' |
There was a problem hiding this comment.
| f'{t5_model_name} Adults in territories must have a valid value for 19C.' | |
| f'{t5_model_name} Adults in territories must have a valid value for {field}.' |
ADPennington
added
Deploy with CircleCI-qasp
| )) | ||
| elif closure_reason == '03' and not is_ssp: | ||
| num_errors += self.__validate_case_closure_ftl(t4, t5s, ( | ||
| 'At least one person who is HoH or spouse of HoH on case must have FTL months >=60.' |
There was a problem hiding this comment.
| 'At least one person who is HoH or spouse of HoH on case must have FTL months >=60.' | |
| 'At least one person who is head-of-household or spouse of head-of-household on case must have countable months toward time limit >= 60 since CLOSURE_REASON = 03: federal 5 year time limit.' |
ADPennington
removed
the
Deploy with CircleCI-qasp
There was a problem hiding this comment.
lgtm @jtimpe 🚀 had one more language suggestion before this merges. i do not need to re-test.
Summary of Changes
Pull request closes #2842. Completes the implementation of cat4 validations with the following
How to Test
Deliverables
More details on how deliverables herein are assessed included here.
Deliverable 1: Accepted Features
Checklist of ACs:
lfrohlichand/oradpenningtonconfirmed that ACs are met.Deliverable 2: Tested Code
CodeCov Reportcomment in PR)CodeCov Reportcomment in PR)Deliverable 3: Properly Styled Code
Deliverable 4: Accessible
iamjollyandttran-hubusing Accessibility Insights reveal any errors introduced in this PR?Deliverable 5: Deployed
Deliverable 6: Documented
Deliverable 7: Secure
Deliverable 8: User Research
Research product(s) clearly articulate(s):