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

Feature/edan verifier utility #531

Draft
wants to merge 19 commits into
base: develop
Choose a base branch
from

Conversation

EMaslowskiQ
Copy link
Collaborator

Creation of EDAN verifier utility.

  • Accessed through endpoint: /verifiers/edan
  • Uses Workflows for managing execution
  • Uses WorkflowReports for storing results
  • Merged changes made to 'develop' and tested

* WIP snapshot of EDAN verifier
* compares subject units and identifiers outputting results to CSV
* currently runs as a test case in EdanCollection.test.ts
* WIP commit capturing refactor of EDAN verifier
* lacks cleanup and testing
*  WIP commit for EDAN Verifier utility
* cleaned out legacy code prior to refactor
* limiting returned EDAN records to 1, otherwise throw error
* fix issues with ARK ids not being outputted if also set to preferred
* added basic determination of DPO vs. EDAN subject sources
* support for name comparisons
* support for updating Packrat units from EDAN
* WIP commit for EDAN Verifier utility
* support for name comparisons
* support for updating Packrat units from EDAN
* improved messaging for automatic/manual fixing
* WIP commit for EDAN verifier
* added EDAN MDM column to ouput
* added ability to run a specific subject ID for debugging
* WIP commit for EDAN verifier
* moved EDAN verifier into own class derived from VerifierBase
* cleaned up how EDAN units are collected
* opting for comparing abbreviations when using fetchFromNamedSearch and receiving multiple hits
* WIP commit for EDAN verifier
* removed fallback for checking multiple EDAN units and opting for comparing abbreviations
* verifier now accepts EdanVerifierConfig object with settings
* moved all temporary debug statements to standard LOG
* EDAN Verifier updated to support running from any endpoint (/verifier/edan)
* support query params for basic configuration
* returns a CSV file if given param of: returnFile=true
* using WorkflowEngine for running the EDAN verifier (WorkflowVerifier)
* full support of query params for configuring verifier (accepts: objectId, limit, returnFile, detailedLogs)
* HTTP response includes hyperlink for downloading the report
* added 'Name' property to WorkflowReport to assist in filename creation
* cleaned up Packrat schema. comment prevented rebuilding the DB with initdb
* created utility routine that gathers all SystemObject details (via GraphQL) and returns as NDJSON. Not accessible.
* tested against 'develop' branch
* added cleaner/styled output for response
* updated model graph (mwb) to reflect changes in DB
* fixed eslint error with resolving VerifierBase
Copy link
Collaborator

@jahjedtieson jahjedtieson left a comment

Choose a reason for hiding this comment

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

Your first PR! What a great milestone!

  • The changes to server/tests/jest.config.js must not be committed. Doing so will disable regression tests from being run, by default, by other users, and by GitHub CI.
  • We don't add server/db/sql/models/Packrat.mwb.bak to our source tree.
  • server/utils/verifiers/VerifierBase.ts's replacePackratUnit() is doing the wrong thing. Instead of updating Unit records, it needs to update Subject.idUnit (i.e point the subject at the right unit).
  • I've asked a number of inline questions/provided feedback.

server/db/api/WorkflowReport.ts Show resolved Hide resolved
server/db/api/WorkflowReport.ts Outdated Show resolved Hide resolved
server/http/index.ts Outdated Show resolved Hide resolved
server/http/routes/download.ts Outdated Show resolved Hide resolved
server/http/routes/verifiers.ts Outdated Show resolved Hide resolved
}

// if identifier exists in our database (value & type) then store it
const identifiers: DBAPI.Identifier[] | null = await DBAPI.Identifier.fetchFromIdentifierValue(content);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm confused by the intention of this code. fetchFromIdentifierValue is going to get all identifiers that match the specified value ... this may include matches for identifiers with the same value, but applied to other system objects, and/or of random identifier types. In other words, identifiers are not guaranteed to have a unique value....

server/utils/verifiers/VerifierBase.ts Outdated Show resolved Hide resolved
server/utils/verifiers/EdanVerifier.ts Outdated Show resolved Hide resolved
server/utils/verifiers/EdanVerifier.ts Show resolved Hide resolved
server/http/routes/dataQueries.ts Show resolved Hide resolved
* formatting and cleanup for submission
Copy link
Collaborator

@jahjedtieson jahjedtieson left a comment

Choose a reason for hiding this comment

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

Please remove server/db/sql/models/Packrat.mwb.bak from this PR.

@EMaslowskiQ EMaslowskiQ marked this pull request as draft February 1, 2023 01:54
* changed LOG type top eSYS for better grouping
* clearer definition of VerifierBase types and their use
* comments describing future improvements
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.

2 participants