🦺 server: return ok for non-processed persona templates#891
Conversation
🦋 Changeset detectedLatest commit: f9f0d0a The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the server's Persona webhook handler to gracefully acknowledge and respond to certain Persona inquiry templates that do not require active processing. By explicitly identifying and returning an 'ok' status for these templates, the system avoids unnecessary processing, improves efficiency, and prevents potential errors from unhandled webhook events, ensuring a smoother integration with Persona. Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces functionality to handle specific Persona templates as 'ignored', returning an 'ok' status without further processing. The changes include updating the Valibot schema to recognize these templates using a picklist and adding a conditional check in the persona hook to bypass processing for them. Comprehensive test cases have been added to verify that these ignored templates correctly return an 'ok' response and do not trigger downstream services like panda.createUser or persona.addDocument. Existing tests were also updated to reflect the new validation error messages resulting from the schema changes. The implementation is clean, well-tested, and directly addresses the stated objective of the pull request.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #891 +/- ##
==========================================
+ Coverage 71.24% 71.34% +0.10%
==========================================
Files 212 212
Lines 8381 8428 +47
Branches 2741 2755 +14
==========================================
+ Hits 5971 6013 +42
- Misses 2132 2134 +2
- Partials 278 281 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
WalkthroughThis PR adds support for non-processed persona templates that return an "ok" response without further processing. It exports additional template constants from the persona utilities module and implements an early-return code path for templates marked as "ignored", along with corresponding test coverage. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can use Trivy to scan for security misconfigurations and secrets in Infrastructure as Code files.Add a .trivyignore file to your project to customize which findings Trivy reports. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e97bf2e5-0380-4d9a-b5c1-6e121acdb828
📒 Files selected for processing (3)
.changeset/spotty-tigers-tease.mdserver/hooks/persona.tsserver/test/hooks/persona.test.ts
| describe("ignored template", () => { | ||
| beforeEach(() => { | ||
| vi.resetAllMocks(); | ||
| }); | ||
|
|
||
| it("returns ok for address template", async () => { | ||
| const response = await appClient.index.$post({ | ||
| header: { "persona-signature": "t=1,v1=sha256" }, | ||
| json: ignoredPayload("itmpl_FTHNSXqJjoMvUTBc85QECGHogrZx"), | ||
| }); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| await expect(response.json()).resolves.toStrictEqual({ code: "ok" }); | ||
| expect(panda.createUser).not.toHaveBeenCalled(); | ||
| expect(persona.addDocument).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns ok for cryptomate template", async () => { | ||
| const response = await appClient.index.$post({ | ||
| header: { "persona-signature": "t=1,v1=sha256" }, | ||
| json: ignoredPayload("itmpl_8uim4FvD5P3kFpKHX37CW817"), | ||
| }); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| await expect(response.json()).resolves.toStrictEqual({ code: "ok" }); | ||
| expect(panda.createUser).not.toHaveBeenCalled(); | ||
| expect(persona.addDocument).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("returns ok for manteca extra fields template", async () => { | ||
| const response = await appClient.index.$post({ | ||
| header: { "persona-signature": "t=1,v1=sha256" }, | ||
| json: ignoredPayload("itmpl_gjYZshv7bc1DK8DNL8YYTQ1muejo"), | ||
| }); | ||
|
|
||
| expect(response.status).toBe(200); | ||
| await expect(response.json()).resolves.toStrictEqual({ code: "ok" }); | ||
| expect(panda.createUser).not.toHaveBeenCalled(); | ||
| expect(persona.addDocument).not.toHaveBeenCalled(); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider using exported template constants instead of hardcoded IDs.
The tests hardcode template IDs directly (e.g., "itmpl_FTHNSXqJjoMvUTBc85QECGHogrZx") rather than importing and using ADDRESS_TEMPLATE, CRYPTOMATE_TEMPLATE, and MANTECA_TEMPLATE_EXTRA_FIELDS from ../../utils/persona. Using the constants ensures tests stay in sync if template IDs change.
♻️ Proposed fix
+import {
+ ADDRESS_TEMPLATE,
+ CRYPTOMATE_TEMPLATE,
+ MANTECA_TEMPLATE_EXTRA_FIELDS,
+} from "../../utils/persona";Then in tests:
it("returns ok for address template", async () => {
const response = await appClient.index.$post({
header: { "persona-signature": "t=1,v1=sha256" },
- json: ignoredPayload("itmpl_FTHNSXqJjoMvUTBc85QECGHogrZx"),
+ json: ignoredPayload(ADDRESS_TEMPLATE),
});
// ...
it("returns ok for cryptomate template", async () => {
const response = await appClient.index.$post({
header: { "persona-signature": "t=1,v1=sha256" },
- json: ignoredPayload("itmpl_8uim4FvD5P3kFpKHX37CW817"),
+ json: ignoredPayload(CRYPTOMATE_TEMPLATE),
});
// ...
it("returns ok for manteca extra fields template", async () => {
const response = await appClient.index.$post({
header: { "persona-signature": "t=1,v1=sha256" },
- json: ignoredPayload("itmpl_gjYZshv7bc1DK8DNL8YYTQ1muejo"),
+ json: ignoredPayload(MANTECA_TEMPLATE_EXTRA_FIELDS),
});
2 participants
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests