[2023-10-20] main -> prod #2565
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* Fixes logic in check_federal_program_total The check was being applied against the federal_program_total line. However, it was summing all expenditures and expecting to find the sum of everything in `amount_expended` to appear in `federal_program_total`. What the check wanted to do was to compare against `total_federal_expenditures.` I suspect this is because I did not ticket the issue clearly. This makes the check do the right thing. * Fixes my fix. I was confused. I forgot that an outcome from earlier was that we needed to fix the workbook generator. Therefore, I confused myself on the check_federal_program_total code. I reverted my adventure there (leaving it as-is from HSMD's work), and instead fixed the workbook generator to now create/insert the `cfda_key` column. I regenerated 100010 to verify that this now works. And, the end-to-end code now works as well (again), because it does the right thing. test_commands is a casualty. It is testing a fixture command that is aging, and I think that end-to-end and the workbook generator will replace it. * That was too easy. I spent too much time with date formats. * Linting. * Before main sync * Addes auditor cert, fixes E2E regression The text on the table changed, so the regression had to change. I've switched to a regex instead of the full contents of the table caption. This also makes the E2E workbook test code actually move audits into the "Accepted" table. This preps the E2E code for use in actuall regression testing. * Linting. * Apply suggestions from code review Accepting cleanup suggestions. Co-authored-by: Phil Dominguez <[email protected]> * Removing a test. --------- Co-authored-by: Phil Dominguez <[email protected]>
…rantee validation failures (#2548)
|
Terraform plan for production No changes. Your infrastructure matches the configuration.✅ Plan applied in Deploy to Production Environment #24 |
|
Terraform plan for staging No changes. Your infrastructure matches the configuration.✅ Plan applied in Deploy to Staging Environment #71 |
sambodeme
approved these changes
Oct 20, 2023
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 92dec12 |
* This... validated two workbooks... Huh. That worked. * Fixes an extraction bug When a cell has a None value, it should become "" in the JSON. * Broke things apart. This made no changes other than to take the previous work and break it into semantically meaningful files/pieces. Now, each workbook has a file. The IR work has its own file. * Small improvements, merging main. * Passes regression tests. * Passes all regressions, demonstrates new checks This commit now passes all workbooks through the IR. The README is started, but not complete. It demonstrates the authoring of checks in keeping with the cross-validations. The errors generated pass through to the frontend correctly, and visualize like all other workbook upload errors. A no-op check is provided. It always passes. A check for a missing UEI is provided. It correctly stops empty UEIs. If a UEI is present, but does not match the regex, it passes through the IR check, and is caught by the schema validation. * Fixes state/other cluster errors This takes a recent helpdesk ticket and demonstrates how to improve the error messages. In this case, a user had N/A N/A N/A for cluster name, state cluster name, and other cluster name. This causes horrible JSON Schema validation errors. Now, we tell them exactly what they need to do in order to fix the workbook. Once the instructions are followed, the user's workbook is fixed, and it passes JSON Schema validation.: * is_direct and passthrough names This adds two checks: 1. The user failed to include the is_direct value(s) 2. The user failed to include a passthrough name when is_direct is `N`. * Adds a loan balance check The JSON Schema validation for this was confusing. When N, we expect nothing in the balance column. When Y, we expect a balance. This adds a check that provides a clear error message. * This checks that the workbook is right This adds up-front checks for: 1. Making sure we are getting a FAC workbook (it checks for a Coversheet) 2. It checks to see if it is the right workbook for a given validation section Now runs notes-to-sefa-specific checks. * Ran black. However, I might have run the wrong version. * Fixing import, removing excel. Not needed anymore, and confuses the linter. * Handles the is_major and audit_report_type This Y/N pair breaks in JSON Schemas. Better error reporting on this pair of columns. * Adds an award finding check Disambinguates the Y/N for prior reference numbers. * Checks for empty rows in the middle of the data. How do people do these things? * Checks award numbers 1. Makes sure they are all unique 2. Makes sure they're in the right sequence Removed some logging from no_major_program_type * Checking for blank fields 1. Cannot have blank findings counts 2. Cannot have blank cluster names When these are blank, they sometimes get through to the schema, and then really bad errors come back. * Moving import to a better place w.r.t. the list. All of these probably have to go to the top of the file for the linter. it made sense to me to put them near the lists I was building for now. * Added other half of passthrough names Handles more workbook cases now. * Replaced some print statements Can't chase down an error with a weird... Excel issue where fonts are involved. * Ready to fix tests... Changing to the IR broke a lot of tests. * Fixing more missing columns ZD 114 had many fields missing; so many that it would make it through the improved checks and still fail the schema. Now, 114 would be guided through the submission by these errors. Their workbook can be started in the errored state, and the error messages will guide them to a valid workbook. * Adds transforms The first transform is on Notes to SEFA. We have an invisible column called seq_number. Users somehow manage to delete things in a way that the cell ends up with a `REF!` instead of a number. The solution is to replace the column with values that are generated computationally on the server side. It is not clear that we use this value? * Using section names, adding prechecks Broke the general checks out, so they can run before transforms. Some transforms may have to run before. (I found one in notes-to-sefa.) So, we need to make sure we're in the right workbook. Then we can clean up the data. Then we can begin checking it for semantics. Then we can rewrite it into a JSON doc. So. Those changes are in this commit, and some tightening on the passthrough names. All inspired by the same workbook... * Adding the Audit Finding grid check As a backstop to the schemas, adding in the allowed grid from the UG. Makes sure the Y/N combos are allowed. * Minor cleanups/removals Comments and printlns that made it through. * Bringing in workbook E2E code. * Adds workbooks to validate with, tests This now tests many workbooks. It runs them through the new generic importer and the JSON validator. If they validate, it is good. If not, it fails. Next is to add explicit failing tests. * Adding a failure test This walks workbooks that are constructed to fail. The test runs all of them, and counts the failures. It should be the case that every workbook in the directory fails. Therefore, we count the workbooks and count the failures. If they come out different, clearly, we did not fail everywhere we expected. * Confirmed fails correctly with... correct wb Placing a correct workbook in the directory does the job. * Adding a breadcrumb The naming in the fail directory structure matters. * Adding another breadcrumb. * Removing print statements * Adds full check paths everywhere This adds the full check path everywhere. It also adds a new failure check for CAP, to make sure the general validations are running at the start. * Linting. * More linting. * Linting. * These are needed for tests in the tree. The linter says they have to go. * Removing more prints There's one I can't find. Also, I'm not going to be able to satisfy the linter. I give up for now. * Some unit tests, removing an unused function From some tests off to the side earlier. * Linting. My version of black does not match the team's. Our docs do not make clear to me how I should be running the linter so as to match the online environment. * This runs end-to-end with cross-val I didn't realize cross-val was baked into the `sac` object. This now runs cross-validation on the workbooks when the E2E code is run. * Trying to work into unit tests Can't quite, because it wants to write to the real DB (vs. a mocked DB). For now, this will have to be a future thing. * Updates from intial review. Expanding variable names, adding comments. TBD some more unit tests. * Fixing test books. 1 * Fixing error introduced through simplification Forgot that the *range* is needed for error message construction, not just the *values* from the range. * Fixed. * Removing a period from an error message. * Updated workbook versions and updated federalAwards template formula in column J to prevent endless non-zero values * Linting * Necessary change to prevent award reference like this AWARD-0001-EXTRA to pass * Necessary change to prevent check_sequential_award_numbers from cratching * More linting * Linting ... * Code update to make mypy happy * Adding a new transform for the EINs They all need to be strings. * #2499 Added Wrap Text and Updated Workbook Version and Removed seq_number from NotesToSefa * #2499 Updated code to set row height * #2499 Updated row height in workbook templates * #2499 Removed obsolete code * #2499 seq_number was removed from NotesToSefa section by mistake and was reverted back via this commit * #2499 Renamed AdditionalNotes sheet and added patch to ensure backwards compatibility with workbook templates 1.0.0 and 1.0.1 * #2499 Updated output --------- Co-authored-by: Matt Jadud <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR checklist: submitters
maininto your branch shortly before creating the PR. (You should also be mergingmaininto your branch regularly during development.)git status | grep migrations. If there are any results, you probably need to add them to the branch for the PR. Your PR should have only one new migration file for each of the component apps, except in rare circumstances; you may need to delete some and re-runpython manage.py makemigrationsto reduce the number to one. (Also, unless in exceptional circumstances, your PR should not delete any migration files.)PR checklist: reviewers
make docker-clean; make docker-first-run && docker compose up; then rundocker compose exec web /bin/bash -c "python manage.py test"The larger the PR, the stricter we should be about these points.