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

Adds migration and column for doc_type_label and fixes virus scanning logic #424

Merged
merged 14 commits into from
Nov 2, 2023

Conversation

lkemperman-cfa
Copy link
Contributor

@lkemperman-cfa lkemperman-cfa commented Oct 25, 2023

  • Adds logic to create space to set the doc type for a file:
    • Adds doc_type_label to user_files table.
    • Adds form-flow.uploads.default-doc-type-label so one can set the default document type if they want.
    • Updates the readme to include this new field and how to set the default.
  • Adds @DynamicField annotation and separates annotations from their validators (slight package reorg)
  • Fixes an error where files were marked incorrectly if virus scanning was disable. File were getting marked as true when scanning was turned off. The NoOpVirusScanner is used when the virus scanning is disabled, so it was marking the record inaccurately. We revamped the logic around this and made the check a little clearer.

You can test this ticket out by using this branch in the LA Doc Uploader.

@bseeger bseeger changed the title add migration and column for doc type label WIP - don't merge yet - add migration and column for doc type label Oct 25, 2023
@bseeger bseeger force-pushed the add-type-to-user-file branch from dfd6219 to 6001dfa Compare October 30, 2023 14:15
@bseeger bseeger changed the title WIP - don't merge yet - add migration and column for doc type label Adds migration and column for doc_type_label and fixes virus scanning logic Oct 30, 2023
@bseeger bseeger requested a review from spokenbird October 30, 2023 20:31
README.md Outdated Show resolved Hide resolved
@cy-by
Copy link
Contributor

cy-by commented Nov 1, 2023

One last comment and there's a conflict in README.md, then this PR is good to go.

lkemperman-cfa and others added 13 commits November 2, 2023 09:58
Fixes an issue with virus scanning if using the noop scanner (would
always say the files was scanned)

Co-Authored-By: Lauren Kemperman; [email protected]
wouldn't run. This now makes sure there is a default value.
Fixes a typo in the readme and adds a section about the dynamic fields.

Co-Authored-By: Lauren Kemperman; [email protected]
Since the test db doesn't appear to be working correctly, this code is
commented out until we figure out why the DB migrations aren't working
well in test.
@bseeger bseeger force-pushed the add-type-to-user-file branch from 1aef6f9 to 4a2bc04 Compare November 2, 2023 14:01
Copy link
Contributor

@cy-by cy-by left a comment

Choose a reason for hiding this comment

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

Comments addressed, approved.

@bseeger bseeger merged commit 5ca8004 into main Nov 2, 2023
3 checks passed
@bseeger bseeger deleted the add-type-to-user-file branch November 2, 2023 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants