API ledger client document url updates - #4648
Conversation
Signed-off-by: Doug Lovett <doug@diamante.ca>
kialj876
left a comment
kialj876
left a comment
There was a problem hiding this comment.
Looking pretty good, but I think there's a few minor issues below
| name: str = static_doc.get("name") | ||
| doc_name: str = doc.get("name", "") | ||
| if (doc_name): | ||
| if name.removesuffix(".pdf") == doc_name.removesuffix(".pdf"): |
There was a problem hiding this comment.
this will bail if 'name' is None. Add 'name' to the if?
There was a problem hiding this comment.
Check added above even though the name is the ledger download name and a defect somewhere else if it does not exist.
| # Try partial matching static doc name with default name. | ||
| for key, value in STATIC_DOCUMENTS.items(): | ||
| if ( | ||
| name.startswith(str(key)) and |
There was a problem hiding this comment.
this also looks like it will bail when 'name' is None
There was a problem hiding this comment.
See my previous comment.
| (False, STATIC_URL3, "Test court order", DRS_TEST_DOC, "CORP", "CRTO", "DS0000101951", "Test court order.pdf"), | ||
| (False, STATIC_URL2, "Test court order", DRS_TEST_DOC, "CORP", "CRTO", "DS0000101952", ""), | ||
| (False, STATIC_URL1, "Court Order 1234", DRS_TEST_DOC, "CORP", "COSD", "DS0000101951", ""), | ||
| (False, STATIC_URL1, "Court Order 1234", DRS_TEST_DOC, "CORP", "COSD", "DS0000101951", "Court Order.pdf"), |
There was a problem hiding this comment.
can you add a case here for the 'drs_filename' as None? Is 'None' possible for migrated minio documents without a name or would they evaluate to empty strings?
There was a problem hiding this comment.
Added: the DRS API returns an empty string instead of None if no file name exists.
| if name.removesuffix(".pdf") == doc_name.removesuffix(".pdf"): | ||
| return True | ||
| if ( | ||
| STATIC_DOCUMENTS.get(name) == name and |
There was a problem hiding this comment.
I think this is a dict to str comparison here? It will always be False
There was a problem hiding this comment.
Thanks for catching, fixed.
| name: str = static_doc.get("name") | ||
| if (name and name == doc.get("name")) or ( | ||
| name and STATIC_DOCUMENTS.get(name) and | ||
| doc.get("documentClass") == STATIC_DOCUMENTS[name].get("documentClass") and |
There was a problem hiding this comment.
We're no longer checking against this at all inside 'is_static_doc_match' is that on purpose? I saw you introduce a wild card for this for court order which means this would need to change a bit if it was kept in. Or if we're not going to use 'documentClass' in STATIC_DOCUMENTS for anything can it be removed from the dict?
There was a problem hiding this comment.
Class removed as an unnecessary check.
Signed-off-by: Doug Lovett <doug@diamante.ca>
Signed-off-by: Doug Lovett <doug@diamante.ca>
|
4 participants
Issue #: /bcgov/entity#34436
Description of changes:
Update the filing ledger download URL set up for client documents after switching from Minio to DRS.
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).