PSP-9287 | Document type purpose / definition #4407
Conversation
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4407 |
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4407 |
2 similar comments
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4407 |
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4407 |
| @@ -55,6 +55,14 @@ export const SelectedDocumentHeader: React.FunctionComponent<ISelectedDocumentHe | |||
| [documentTypes], | |||
| ); | |||
|
|
|||
| const selectedDocumentType = useMemo(() => { | |||
| const documentType = documentTypes.find(d => Number(document.documentTypeId) === d.id); | |||
There was a problem hiding this comment.
unrelated to these changes, but isn't the documentTypeId a number? at least I think it is on the backend.
There was a problem hiding this comment.
it is, the problem is that on the form is a string.
| @@ -55,6 +55,14 @@ export const SelectedDocumentHeader: React.FunctionComponent<ISelectedDocumentHe | |||
| [documentTypes], | |||
| ); | |||
|
|
|||
| const selectedDocumentType = useMemo(() => { | |||
There was a problem hiding this comment.
nit: This variable appears to contain the purpose and not the document type
| document_types = [] | ||
|
|
||
| # Get the document Type | ||
| for i in range(2, doctype_sheet.nrows): |
There was a problem hiding this comment.
nit: magic number(2)
| doc_type_name = doctype_sheet.cell_value(i, 2) | ||
| doc_type_purpose = doctype_sheet.cell_value(i, 3) | ||
| # display_order = int(doctype_sheet.cell_value(i, 3)) | ||
| display_order = 8 |
| metadata_types = [] | ||
|
|
||
| # Get the document Type | ||
| for i in range(3, metadata_sheet.nrows): |
There was a problem hiding this comment.
nit: magic number(3)
| print("--------------" + str(i)) | ||
| current_row_name = sheet.cell_value(i, 1) | ||
|
|
||
| if current_row_name != "": |
There was a problem hiding this comment.
why is this necessary?
There was a problem hiding this comment.
Sometimes the document definition takes multiple rows, leaving the row name empty
|
|
||
| doctype_sort_order = [] | ||
|
|
||
| for i in range(3, sheet.nrows): |
There was a problem hiding this comment.
nit: magic number(3)
|
|
||
| def load_document_definitions(): | ||
| # Give the location of the file | ||
| doctype_location = ("xml_definitions/doctype_definitions.xlsx") |
There was a problem hiding this comment.
nit: perhaps a common function to get a file, open the workbook, extract the sheet, and possibly index to the first valid row? that would cut down on some of this duplicated code.
| if current_row_name != "": | ||
| doc_type_name = current_row_name | ||
|
|
||
| if doc_type_name not in doctype_sort_order: |
There was a problem hiding this comment.
Hmm, we may want to throw here instead, I feel like I'd rather fail fast then accidentally generate a json with missing data.
| @@ -0,0 +1,220 @@ | |||
|
|
|||
There was a problem hiding this comment.
please either add some doc for this, or update the make file.
There was a problem hiding this comment.
Thanks for adding this!
There was a problem hiding this comment.
good idea. I will add a new pr with a readme
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4407 |
1 similar comment
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4407 |
FuriousLlama
changed the title
No description provided.