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

Adding Endpoint for Record Validation #112

Closed
wants to merge 2 commits into from

Conversation

Azher2Ali
Copy link

@Azher2Ali Azher2Ali commented Nov 21, 2024

This feature introduces a new endpoint for validating the existence of records within specific contexts, supporting applications that require robust data integrity. By ensuring records exist (or preventing duplicates), this endpoint enhances user experience and maintains data consistency.

Endpoint Details:
Method: GET
Path: /validator/{categoryId}/{entityName}

Query Parameters:

studyId: Identifier of the organization in Lyric, required as a string.
value: The value to validate, required as a string.
categoryId: Numeric ID of the category the record belongs to.
entityName: Name of the entity or analysis type (string) used in Song.

Example Request:
GET http://localhost:3030/validator/{categoryId}/{entityName}?studyId={studyId}&value={value}

Expected Behavior:
Record Exists: Returns 200 OK with an empty body.
Record Not Found: Returns 400 Bad Request with a response body:

{
   "error": "INVALID_VALUE",
   "message": "Description of why the value is invalid"
}

Implemented Changes:

  • Created a new ValidatorController to manage the validation endpoint.

  • Developed a ValidationService to handle logic for checking records within the submitted_data table.

  • Added a Router for routing the new endpoint.

  • Defined data models for ValidationRequest and ValidationResponse.

This endpoint’s design supports configuration-based category and entityName searches, offering flexibility for different validation contexts.

This feature is essential for applications that must verify records' existence within specific categories and organizations, providing a streamlined, reliable approach to validating data before further processing.

Also there are some pending changes related to Database Query, which will be added to this PR with our upcoming changes

@Azher2Ali Azher2Ali requested review from leoraba and joneubank and removed request for leoraba November 21, 2024 15:50
@joneubank joneubank linked an issue Nov 21, 2024 that may be closed by this pull request
Copy link
Contributor

@joneubank joneubank left a comment

Choose a reason for hiding this comment

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

Changes are clear and concise, nice work.

I would like to change two things:

  1. let's have the response handled within the controller instead of throwing an error.
  2. be more specific with the controller function name.

Comment on lines +33 to +35
if (!isValid) {
throw new ValidationError('INVALID_VALUE', 'The specified value was not found.');
}
Copy link
Contributor

Choose a reason for hiding this comment

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

We could just return the error state with a status 400... won't this result in an error 500 response due to our error handler?

Comment on lines +25 to +26
const filterEntityNameSql = eq(submittedData.entityName, entityName);
const filter = eq(submittedData.systemId, studyId); //todo need to change
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are these filter condition separated out from the other where conditions that are defined within the where clause (lines 33, 34)?

const LOG_MODULE = 'VALIDATION_SERVICE';

return {
validateRecord: async ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Feel like our naming could be more specific here. We aren't running validation on a record, we are checking for the existence of a record with a given property value. Should we update this name to be checkValueExists?


const resultCount = await db
.select({ total: count() })
.from(submittedData) //todo need to change
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure what needs to be changed, can you elaborate? Should probably update the PR or leave a more detailed TODO comment (preferably do this work now ;) )

@Azher2Ali Azher2Ali closed this Dec 12, 2024
@Azher2Ali Azher2Ali deleted the feature/external-validation-endpoint branch December 12, 2024 15:22
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.

External validation endpoint
2 participants