Skip to content

Conversation

@EthanEnigma
Copy link
Collaborator

@EthanEnigma EthanEnigma commented Nov 25, 2025

This pull request updates the putKnockValidation route to improve its logic and error handling by integrating the TokenStoreService and returning more informative responses. The main changes focus on fetching, updating, and persisting validation payloads, as well as handling missing tokens more gracefully.

Enhancements to validation logic and error handling:

  • Injected TokenStoreService into the route handler and used it to fetch and update the token payload for a given knockId. [1] [2]
  • Added error handling with YHTTPError to throw an error if the token payload for the specified knockId is not found. [1] [2]
  • Modified the response to return the updated payload in the response body instead of an empty object.

@Degalax Degalax requested a review from devZenta November 25, 2025 13:49
@devZenta devZenta added documentation Improvements or additions to documentation enhancement New feature or request labels Nov 25, 2025
@devZenta devZenta changed the title Feat: inject tokenStore, get payload and put payload back to store Feature/inject tokenStore, get payload and put payload back to store Nov 25, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR implements the validation workflow for knock requests by integrating the token store service. The implementation retrieves stored knock payloads, marks them as validated, and persists the updated state back to the token store.

  • Injected TokenStoreService dependency into the route handler
  • Implemented token retrieval, validation marking, and persistence logic
  • Updated response to return the validated payload instead of an empty object

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const payload = await tokenStore.get(knockId);

if (!payload) {
throw new YHTTPError;
Copy link

Copilot AI Nov 25, 2025

Choose a reason for hiding this comment

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

The YHTTPError constructor is called without any arguments. According to the pattern used in other routes (e.g., postCronRun.ts, putEcho.ts), YHTTPError should be instantiated with at least a status code and error code. For a missing token/payload, this should likely be a 404 error.

Example:

throw new YHTTPError(404, 'E_KNOCK_NOT_FOUND', knockId);
Suggested change
throw new YHTTPError;
throw new YHTTPError(404, 'E_KNOCK_NOT_FOUND', knockId);

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nfroidure nfroidure force-pushed the feat/knock_Validation branch 2 times, most recently from a5e1b7a to 0de6bf2 Compare November 25, 2025 15:01
@nfroidure nfroidure force-pushed the feat/knock_Validation branch from 0de6bf2 to 83bb980 Compare November 25, 2025 15:05
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 6 out of 7 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@nfroidure nfroidure merged commit 854441a into main Nov 25, 2025
8 checks passed
@nfroidure nfroidure deleted the feat/knock_Validation branch November 25, 2025 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants