[#946] Trigger logs view #1075
[#946] Trigger logs view #1075
Conversation
WalkthroughReplaces the legacy Paybutton trigger UI with a new Trigger UI and TriggerLogs; adds a paginated logs API, service exports/types, Pro-aware credit helpers and config normalization; introduces soft-delete for triggers, updates seeds, CSS, page integration, and expands/rewrites tests. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as PaybuttonTrigger UI
participant API as /api/paybutton/triggers/logs
participant Service as triggerService.fetchTriggerLogsForPaybutton
participant DB as Database
rect rgb(245,250,255)
Note over UI,API: User opens Logs tab / Trigger UI mounts
end
User->>UI: open logs / switch tab
UI->>API: GET /api/paybutton/triggers/logs/[id]?page=&pageSize=&orderBy=&orderDesc=&actionType=
activate API
API->>API: validate session & ownership
API->>Service: fetchTriggerLogsForPaybutton(params)
activate Service
Service->>DB: query triggerLog (where paybuttonId & optional actionType, deletedAt = null, order/skip/take)
DB-->>Service: rows
Service->>DB: count total matching rows
DB-->>Service: totalCount
deactivate Service
Service-->>API: { data, totalCount }
API-->>UI: 200 OK { data, totalCount }
deactivate API
UI->>UI: map rows (toPostRow / toEmailRow) & render table
UI-->>User: render paginated table
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (1)
components/Trigger/TriggerLogs.tsx (1)
6-6: Consider importing TriggerLogActionType from services.Instead of defining a local
ActionType, consider importingTriggerLogActionTypefromservices/triggerServiceto ensure type consistency across the codebase and avoid potential drift.-type ActionType = 'PostData' | 'SendEmail' +import { TriggerLogActionType } from 'services/triggerService' +type ActionType = TriggerLogActionType
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
components/Paybutton/PaybuttonTrigger.tsx(0 hunks)components/Paybutton/paybutton.module.css(3 hunks)components/Trigger/PaybuttonTrigger.tsx(1 hunks)components/Trigger/TriggerLogs.tsx(1 hunks)components/Trigger/trigger.module.css(1 hunks)config/index.ts(2 hunks)constants/index.ts(0 hunks)pages/api/paybutton/triggers/logs/[id].ts(1 hunks)pages/button/[id].tsx(2 hunks)services/triggerService.ts(3 hunks)services/userService.ts(2 hunks)tests/integration-tests/api.test.ts(5 hunks)tests/unittests/triggerService2.test.ts(2 hunks)tests/unittests/userService.test.ts(2 hunks)
💤 Files with no reviewable changes (2)
- constants/index.ts
- components/Paybutton/PaybuttonTrigger.tsx
🧰 Additional context used
🧬 Code graph analysis (8)
services/userService.ts (1)
services/triggerService.ts (1)
TriggerLogActionType(236-236)
components/Trigger/PaybuttonTrigger.tsx (3)
services/triggerService.ts (1)
TriggerLogActionType(236-236)services/userService.ts (2)
getUserTriggerCreditsLimit(174-185)getUserRemainingTriggerCreditsLimit(187-192)utils/validators.ts (1)
PaybuttonTriggerPOSTParameters(212-219)
components/Trigger/TriggerLogs.tsx (1)
components/TableContainer/TableContainerGetter.tsx (1)
DataGetterReturn(6-9)
services/triggerService.ts (1)
utils/validators.ts (1)
parseTriggerPostData(262-289)
tests/unittests/triggerService2.test.ts (2)
prisma-local/mockedClient.ts (1)
prismaMock(19-19)services/triggerService.ts (1)
fetchTriggerLogsForPaybutton(75-95)
tests/unittests/userService.test.ts (1)
services/userService.ts (1)
getUserTriggerCreditsLimit(174-185)
tests/integration-tests/api.test.ts (2)
services/addressesOnUserProfileService.ts (1)
fetchAddressWallet(33-49)tests/utils.ts (4)
clearPaybuttonsAndAddresses(25-30)createUserProfile(39-45)createPaybuttonForUser(47-61)testEndpoint(10-19)
pages/api/paybutton/triggers/logs/[id].ts (5)
utils/setSession.ts (1)
setSession(9-24)constants/index.ts (1)
RESPONSE_MESSAGES(8-108)services/paybuttonService.ts (1)
fetchPaybuttonById(278-283)services/triggerService.ts (1)
fetchTriggerLogsForPaybutton(75-95)utils/validators.ts (1)
parseError(65-89)
🪛 Biome (2.1.2)
components/Trigger/trigger.module.css
[error] 19-19: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
border is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🪛 ESLint
pages/button/[id].tsx
[error] 16-16: 'TriggerLogs' is defined but never used.
(@typescript-eslint/no-unused-vars)
🪛 GitHub Actions: Pull Request Tests
config/index.ts
[error] 45-45: Cannot convert undefined or null to object in readConfig: Object.keys(p) where p is undefined (likely config.proSettings is undefined).
🔇 Additional comments (15)
services/userService.ts (3)
169-172: LGTM!The Pro status check is clean and correctly handles null/undefined cases with the loose equality check.
174-185: LGTM!The credit limit calculation correctly branches on user Pro status and action type, returning the appropriate config values.
187-192: LGTM!The remaining credits accessor correctly maps action types to their corresponding user credit fields.
components/Paybutton/paybutton.module.css (3)
149-149: LGTM!The form container height adjustments accommodate the expanded trigger management UI while preserving flexibility for specific form variants.
Also applies to: 248-248
454-477: LGTM!The badge style refactoring effectively reduces duplication while maintaining clear visual distinction between active and inactive states.
479-492: LGTM!The new credits info container provides consistent styling for displaying credit information with appropriate dark theme support.
components/Trigger/TriggerLogs.tsx (2)
27-124: LGTM!The helper functions robustly handle JSON parsing, extract relevant fields with appropriate fallbacks, and gracefully handle errors. The defensive coding ensures the UI remains stable even with malformed data.
242-274: LGTM!The tab interface is well-structured with appropriate accessibility attributes, and the TableContainer integration properly handles dynamic columns and data based on the active tab.
components/Trigger/trigger.module.css (1)
1-17: LGTM!The tab styling provides good visual feedback with smooth transitions and proper dark theme support. The utility classes for code blocks and status text are well-designed and reusable.
Also applies to: 20-73
tests/unittests/userService.test.ts (1)
74-127: LGTM!The test suite provides comprehensive coverage of the new user service functions, including edge cases for Pro status checking and correct limit calculations for both email and post actions.
tests/unittests/triggerService2.test.ts (1)
241-286: LGTM!The test suite properly validates the trigger log fetching functionality, including pagination parameters, sorting, and empty result handling.
config/index.ts (1)
8-16: LGTM!The type changes correctly reflect the runtime normalization behavior, converting the
'Inf'string literal toInfinity(number type) and simplifying themonthsCosttype to a more flexible record.tests/integration-tests/api.test.ts (3)
16-18: LGTM!The new imports support trigger logs testing, and the mock parameter update correctly uses the underscore prefix convention for unused parameters.
Also applies to: 44-44
931-978: LGTM!The wallet update test is now active with updated assertions that properly validate the nested address structure and confirm that old addresses are correctly removed from the original wallet.
1462-1533: LGTM!The new trigger logs API test suite provides comprehensive coverage including ownership validation, pagination support, HTTP method restrictions, and error handling for nonexistent resources.
| {currentPosterTriggerId !== '' && ( | ||
| <button | ||
| type="button" | ||
| onClick={() => setClearModal('poster')} | ||
| className={style.trigger_delete_btn} | ||
| > | ||
| Delete | ||
| </button> | ||
| )} | ||
| <button disabled={disablePosterSubmit} type="submit" className="button_main"> | ||
| {currentPosterTriggerId === undefined ? 'Create Send Request' : 'Update Send Request'} | ||
| </button> | ||
| </div> | ||
| </form> | ||
| </div> | ||
|
|
||
| <div className={style.form_ctn}> | ||
| <div className={style.trigger_header}> | ||
| <h5>Send Email</h5> | ||
| <div> | ||
| {currentEmailTriggerId !== undefined | ||
| ? ( | ||
| <div className={style.active_label}>Active</div> | ||
| ) | ||
| : ( | ||
| <div className={style.inactive_label}>Inactive</div> | ||
| )} | ||
| <div className={style.credits_info}>{renderCreditsInfo(userProfile, 'SendEmail')}</div> | ||
| </div> | ||
| </div> | ||
|
|
||
| <form | ||
| onSubmit={(e) => { void handleSubmitEmailTrigger(getSubmitTriggerHandler('email'))(e) }} | ||
| method="post" | ||
| > | ||
| <label htmlFor="emails">Email</label> | ||
| <input {...registerEmailTrigger('emails')} type="text" id="emails" name="emails" /> | ||
|
|
||
| <div className={style.trigger_btn_row}> | ||
| {emailError !== '' && <div className={style.error_message_}>{emailError}</div>} | ||
| {emailSuccessText !== '' && <div className={style.success_message_}>{emailSuccessText}</div>} | ||
| {currentEmailTriggerId !== '' && ( | ||
| <button | ||
| type="button" | ||
| onClick={() => setClearModal('email')} | ||
| className={style.trigger_delete_btn} | ||
| > | ||
| Delete | ||
| </button> | ||
| )} | ||
| <button disabled={disableEmailSubmit} type="submit" className="button_main"> |
There was a problem hiding this comment.
Hide the delete action when nothing exists to delete
currentPosterTriggerId !== '' (and the email counterpart) is always true for undefined, so the Delete button renders even before a trigger exists. Clicking it sends { triggerId: undefined }, which the backend rejects and shows an error toast. Gate the button on the id actually being defined.
- {currentPosterTriggerId !== '' && (
+ {currentPosterTriggerId != null && (
...
- {currentEmailTriggerId !== '' && (
+ {currentEmailTriggerId != null && (📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {currentPosterTriggerId !== '' && ( | |
| <button | |
| type="button" | |
| onClick={() => setClearModal('poster')} | |
| className={style.trigger_delete_btn} | |
| > | |
| Delete | |
| </button> | |
| )} | |
| <button disabled={disablePosterSubmit} type="submit" className="button_main"> | |
| {currentPosterTriggerId === undefined ? 'Create Send Request' : 'Update Send Request'} | |
| </button> | |
| </div> | |
| </form> | |
| </div> | |
| <div className={style.form_ctn}> | |
| <div className={style.trigger_header}> | |
| <h5>Send Email</h5> | |
| <div> | |
| {currentEmailTriggerId !== undefined | |
| ? ( | |
| <div className={style.active_label}>Active</div> | |
| ) | |
| : ( | |
| <div className={style.inactive_label}>Inactive</div> | |
| )} | |
| <div className={style.credits_info}>{renderCreditsInfo(userProfile, 'SendEmail')}</div> | |
| </div> | |
| </div> | |
| <form | |
| onSubmit={(e) => { void handleSubmitEmailTrigger(getSubmitTriggerHandler('email'))(e) }} | |
| method="post" | |
| > | |
| <label htmlFor="emails">Email</label> | |
| <input {...registerEmailTrigger('emails')} type="text" id="emails" name="emails" /> | |
| <div className={style.trigger_btn_row}> | |
| {emailError !== '' && <div className={style.error_message_}>{emailError}</div>} | |
| {emailSuccessText !== '' && <div className={style.success_message_}>{emailSuccessText}</div>} | |
| {currentEmailTriggerId !== '' && ( | |
| <button | |
| type="button" | |
| onClick={() => setClearModal('email')} | |
| className={style.trigger_delete_btn} | |
| > | |
| Delete | |
| </button> | |
| )} | |
| <button disabled={disableEmailSubmit} type="submit" className="button_main"> | |
| {currentPosterTriggerId != null && ( | |
| <button | |
| type="button" | |
| onClick={() => setClearModal('poster')} | |
| className={style.trigger_delete_btn} | |
| > | |
| Delete | |
| </button> | |
| )} | |
| <button disabled={disablePosterSubmit} type="submit" className="button_main"> | |
| {currentPosterTriggerId === undefined ? 'Create Send Request' : 'Update Send Request'} | |
| </button> | |
| </div> | |
| </form> | |
| </div> | |
| <div className={style.form_ctn}> | |
| <div className={style.trigger_header}> | |
| <h5>Send Email</h5> | |
| <div> | |
| {currentEmailTriggerId !== undefined | |
| ? ( | |
| <div className={style.active_label}>Active</div> | |
| ) | |
| : ( | |
| <div className={style.inactive_label}>Inactive</div> | |
| )} | |
| <div className={style.credits_info}>{renderCreditsInfo(userProfile, 'SendEmail')}</div> | |
| </div> | |
| </div> | |
| <form | |
| onSubmit={(e) => { void handleSubmitEmailTrigger(getSubmitTriggerHandler('email'))(e) }} | |
| method="post" | |
| > | |
| <label htmlFor="emails">Email</label> | |
| <input {...registerEmailTrigger('emails')} type="text" id="emails" name="emails" /> | |
| <div className={style.trigger_btn_row}> | |
| {emailError !== '' && <div className={style.error_message_}>{emailError}</div>} | |
| {emailSuccessText !== '' && <div className={style.success_message_}>{emailSuccessText}</div>} | |
| {currentEmailTriggerId != null && ( | |
| <button | |
| type="button" | |
| onClick={() => setClearModal('email')} | |
| className={style.trigger_delete_btn} | |
| > | |
| Delete | |
| </button> | |
| )} | |
| <button disabled={disableEmailSubmit} type="submit" className="button_main"> |
🤖 Prompt for AI Agents
In components/Trigger/PaybuttonTrigger.tsx around lines 255-305, the Delete
buttons are currently gated by `currentPosterTriggerId !== ''` and the email
counterpart which evaluates true for `undefined`, causing the Delete button to
render and send `{ triggerId: undefined }`. Change the conditional to ensure the
id is actually defined and non-empty (e.g. `currentPosterTriggerId !== undefined
&& currentPosterTriggerId !== ''`) and do the same for `currentEmailTriggerId`,
so the Delete button only appears when an actual trigger id exists.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
Makefile (1)
3-3: Extract long Makefile variable into separate file or use heredoc for maintainability.The
create_test_paybutton_jsonvariable on line 3 is extremely long and difficult to read/maintain. Consider using a heredoc syntax or extracting to a separate JSON file for better clarity and easier future updates.Example using heredoc:
-create_test_paybutton_json = echo { \"priceAPIURL\": \"foo\", ... } > paybutton-config.json +create_test_paybutton_json = cat > paybutton-config.json << 'EOF' +{ + "priceAPIURL": "foo", + "networkBlockchainClients": { "ecash": "chronik", "bitcoincash": "chronik" }, + "networkBlockchainURLs": { "ecash": ["https://xec.paybutton.io"], "bitcoincash": ["https://bch.paybutton.io"] }, + "wsBaseURL": "http://localhost:5000", + "apiDomain": "http://localhost:3000", + "proSettings": { + "enabled": true, + "monthsCost": { "1": 10, "3": 20, "6": 30, "12": 50 }, + "payoutAddress": "ecash:qrf4zh4vgrdal8d8gu905d90w5u2y60djcd2d5h6un", + "standardDailyEmailLimit": 5, + "proDailyEmailLimit": 100, + "standardDailyPostLimit": 5, + "proDailyPostLimit": 100, + "standardAddressesPerButtonLimit": 20, + "proAddressesPerButtonLimit": "Inf" + } +} +EOFAlternatively, store the JSON in a separate
paybutton-config.test.jsonfile and copy it during test setup.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Makefile(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Pull Request Tests
Makefile
[error] 1-1: Command failed: make github-test-unit (exit code 1).
🔇 Additional comments (1)
Makefile (1)
3-3: ****The
"Inf"string value inproAddressesPerButtonLimitis intentional and properly handled. The config normalization logic explicitly converts "Inf" to Infinity:if (p[key] === 'Inf') { p[key] = Infinity }. The frontend display also handles this value correctly, showing "Unlimited" when the limit equals "Inf". The README documents this as an intended type:number | "Inf". The generated JSON is valid and the configuration follows the established pattern used across all proSettings limit fields. No action required.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
config/index.ts (1)
42-51: Guard clause correctly addresses previous pipeline failure.The undefined/null check on line 45 properly resolves the pipeline failure flagged in the previous review where
config.proSettingswas undefined. The normalization logic correctly converts 'Inf' strings to Infinity across all proSettings keys.One minor optional improvement for extra type safety:
const p = config.proSettings as unknown as Record<string, unknown> if (p !== undefined && p !== null) { for (const key of Object.keys(p)) { - if (p[key] === 'Inf') { + if (typeof p[key] === 'string' && p[key] === 'Inf') { p[key] = Infinity } } }This ensures we only attempt the 'Inf' comparison on string values, avoiding potential issues if non-string values are present.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
components/Trigger/PaybuttonTrigger.tsx(1 hunks)components/Trigger/TriggerLogs.tsx(1 hunks)components/Trigger/trigger.module.css(1 hunks)config/index.ts(2 hunks)pages/api/paybutton/triggers/logs/[id].ts(1 hunks)pages/button/[id].tsx(2 hunks)services/triggerService.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- components/Trigger/PaybuttonTrigger.tsx
- components/Trigger/trigger.module.css
- pages/api/paybutton/triggers/logs/[id].ts
- pages/button/[id].tsx
🧰 Additional context used
🧬 Code graph analysis (2)
services/triggerService.ts (1)
utils/validators.ts (1)
parseTriggerPostData(262-289)
components/Trigger/TriggerLogs.tsx (2)
components/Transaction/PaybuttonTransactions.tsx (1)
moment(40-124)components/TableContainer/TableContainerGetter.tsx (1)
DataGetterReturn(6-9)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Tests
🔇 Additional comments (5)
config/index.ts (1)
6-16: Type changes align with runtime normalization.The updated ProSettings types now strictly require
numberinstead ofnumber | 'Inf', which is consistent with the normalization logic that converts any 'Inf' string values toInfinityat runtime. The change toRecord<string, number>formonthsCostis also a good modernization.services/triggerService.ts (2)
363-373: Good fix: parsed variable now has fallback for error logging.The broader-scoped
parsedvariable allows the error handler to reference the parsed value or fall back totrigger.postDatawhen parsing fails. This fixes the bug mentioned in the PR description where unreplaced placeholders like<address>and<amount>were being saved in error logs.
243-269: LGTM: Type exports enable public API consumption.Exporting these types and interfaces allows external consumers (like the new TriggerLogs component) to properly type-check against the trigger log data structures.
components/Trigger/TriggerLogs.tsx (2)
213-240: Pagination fix implemented, but backend has a critical bug.The
actionTypeparameter is now correctly passed to the API (line 224), which addresses the past review comment about client-side filtering breaking pagination. However, the backendfetchTriggerLogsForPaybuttonfunction has a critical bug where the count query doesn't filter byactionType(see review comment inservices/triggerService.ts). Once that backend issue is fixed, this implementation will work correctly.
28-124: Well-structured helper functions with robust error handling.The
pretty(),toEmailRow(), andtoPostRow()transformer functions handle edge cases gracefully with try-catch blocks and fallback values. The logic correctly distinguishes between error and success states, and appropriately extracts data from various field names.
| role="tab" | ||
| tabIndex={0} | ||
| onClick={() => setActiveTab('PostData')} | ||
| className={`${style.tab} ${activeTab === 'PostData' ? style.tabActive : ''}`} | ||
| > | ||
| Requests | ||
| </div> | ||
| <div | ||
| role="tab" | ||
| tabIndex={0} | ||
| onClick={() => setActiveTab('SendEmail')} | ||
| className={`${style.tab} ${activeTab === 'SendEmail' ? style.tabActive : ''}`} | ||
| > | ||
| Emails | ||
| </div> | ||
| </div> |
There was a problem hiding this comment.
Add keyboard navigation support for accessibility.
The tabs are not fully keyboard accessible. They have role="tab" and tabIndex={0} but are missing onKeyDown handlers and proper ARIA attributes. Keyboard-only users can focus the tabs but cannot activate them with Enter or Space keys.
Apply these changes:
- <div className={style.wrapper}>
+ <div className={style.wrapper}>
- <div className={style.tabBar}>
+ <div className={style.tabBar} role="tablist">
<div
role="tab"
tabIndex={0}
+ aria-selected={activeTab === 'PostData'}
onClick={() => setActiveTab('PostData')}
+ onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); setActiveTab('PostData') } }}
className={`${style.tab} ${activeTab === 'PostData' ? style.tabActive : ''}`}
>
Requests
</div>
<div
role="tab"
tabIndex={0}
+ aria-selected={activeTab === 'SendEmail'}
onClick={() => setActiveTab('SendEmail')}
+ onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); setActiveTab('SendEmail') } }}
className={`${style.tab} ${activeTab === 'SendEmail' ? style.tabActive : ''}`}
>
Emails
</div>
</div>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| role="tab" | |
| tabIndex={0} | |
| onClick={() => setActiveTab('PostData')} | |
| className={`${style.tab} ${activeTab === 'PostData' ? style.tabActive : ''}`} | |
| > | |
| Requests | |
| </div> | |
| <div | |
| role="tab" | |
| tabIndex={0} | |
| onClick={() => setActiveTab('SendEmail')} | |
| className={`${style.tab} ${activeTab === 'SendEmail' ? style.tabActive : ''}`} | |
| > | |
| Emails | |
| </div> | |
| </div> | |
| <div className={style.wrapper}> | |
| <div className={style.tabBar} role="tablist"> | |
| <div | |
| role="tab" | |
| tabIndex={0} | |
| aria-selected={activeTab === 'PostData'} | |
| onClick={() => setActiveTab('PostData')} | |
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); setActiveTab('PostData') } }} | |
| className={`${style.tab} ${activeTab === 'PostData' ? style.tabActive : ''}`} | |
| > | |
| Requests | |
| </div> | |
| <div | |
| role="tab" | |
| tabIndex={0} | |
| aria-selected={activeTab === 'SendEmail'} | |
| onClick={() => setActiveTab('SendEmail')} | |
| onKeyDown={(e) => { if (e.key === 'Enter' || e.key === ' ') { e.preventDefault(); setActiveTab('SendEmail') } }} | |
| className={`${style.tab} ${activeTab === 'SendEmail' ? style.tabActive : ''}`} | |
| > | |
| Emails | |
| </div> | |
| </div> |
🤖 Prompt for AI Agents
In components/Trigger/TriggerLogs.tsx around lines 246 to 261, the tab elements
expose role="tab" and tabIndex={0} but lack keyboard activation and proper ARIA
state; add an onKeyDown handler that activates the tab when Enter or Space is
pressed (calling setActiveTab with the same value used by onClick), add
aria-selected={activeTab === 'PostData' / 'SendEmail'}, provide an id for each
tab and aria-controls pointing to the corresponding tabpanel id, and ensure the
tab container has role="tablist" (or verify it exists) so keyboard-only users
can focus and activate tabs correctly.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
services/triggerService.ts(3 hunks)tests/integration-tests/api.test.ts(5 hunks)tests/unittests/triggerService2.test.ts(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unittests/triggerService2.test.ts
🧰 Additional context used
🧬 Code graph analysis (2)
tests/integration-tests/api.test.ts (3)
services/addressesOnUserProfileService.ts (1)
fetchAddressWallet(33-49)tests/utils.ts (4)
clearPaybuttonsAndAddresses(25-30)createUserProfile(39-45)createPaybuttonForUser(47-61)testEndpoint(10-19)constants/index.ts (1)
RESPONSE_MESSAGES(8-108)
services/triggerService.ts (1)
utils/validators.ts (1)
parseTriggerPostData(262-289)
🔇 Additional comments (5)
services/triggerService.ts (3)
67-107: LGTM! Critical pagination fix confirmed.The
actionTypefilter has been correctly added to the count query (line 101), matching thefindManyfilter (line 90). This resolves the critical pagination issue flagged in the previous review wheretotalCountwould reflect all logs regardless of action type.
248-274: LGTM! Type exports align with public API.The exported types and interfaces appropriately support the new
fetchTriggerLogsForPaybuttonpublic API and enable external consumers to work with trigger log data structures.
368-378: Verify type consistency for postedData.The scoping fix correctly addresses the bug where failed requests would save unreplaced placeholders. By declaring
parsedat function scope, the error handler can now access successfully parsed data even when the axios call fails.However, there's a type inconsistency:
PostDataTriggerLog.postedDatais typed asstring(line 259), butparsedisany. SinceparseTriggerPostDatareturns a parsed JSON object, consider explicitly stringifying it on line 375:- data = { postedData: parsed, postedURL: trigger.postURL, responseData: resp.data } + data = { postedData: JSON.stringify(parsed), postedURL: trigger.postURL, responseData: resp.data }This would ensure the logged data structure matches the interface contract and makes the data format consistent between success and error cases.
tests/integration-tests/api.test.ts (2)
16-18: LGTM! Clean import additions and unused parameter handling.The new imports support the trigger logs test suite (line 16) and enhanced wallet validation (line 18). The
_resparameter naming (line 44) correctly indicates an unused parameter, improving code clarity.Also applies to: 44-44
931-979: LGTM! Comprehensive wallet update validation.The test now thoroughly validates wallet address updates by:
- Confirming new addresses are correctly associated (lines 962-970)
- Verifying old addresses are properly dissociated (lines 975-978)
This bidirectional validation ensures the update operation maintains data integrity.
| describe('GET /api/paybutton/triggers/logs/[id]', () => { | ||
| let userId: string | ||
| let otherUserId: string | ||
| let paybuttonId: string | ||
|
|
||
| beforeAll(async () => { | ||
| await clearPaybuttonsAndAddresses() | ||
| const user = await createUserProfile('triggerlog-user') | ||
| const other = await createUserProfile('triggerlog-other') | ||
| userId = user.id | ||
| otherUserId = other.id | ||
| const { paybutton } = await createPaybuttonForUser(userId) | ||
| paybuttonId = paybutton.id | ||
| }) | ||
|
|
||
| it('returns logs data for user-owned paybutton (empty ok)', async () => { | ||
| const req: any = { | ||
| method: 'GET', | ||
| query: { id: paybuttonId, page: '0', pageSize: '10', orderBy: 'createdAt', orderDesc: 'false' }, | ||
| session: { userId } | ||
| } | ||
| const res = await testEndpoint(req, triggerLogsEndpoint) | ||
| expect(res.statusCode).toBe(200) | ||
| const body = res._getJSONData() | ||
| expect(body).toHaveProperty('data') | ||
| expect(body).toHaveProperty('totalCount') | ||
| expect(Array.isArray(body.data)).toBe(true) | ||
| expect(typeof body.totalCount).toBe('number') | ||
| }) | ||
|
|
||
| it('returns 400 if paybutton does not belong to user', async () => { | ||
| const req: any = { | ||
| method: 'GET', | ||
| query: { id: paybuttonId }, | ||
| session: { userId: otherUserId } | ||
| } | ||
| const res = await testEndpoint(req, triggerLogsEndpoint) | ||
| expect(res.statusCode).toBe(400) | ||
| const body = res._getJSONData() | ||
| expect(body.message).toBe(RESPONSE_MESSAGES.RESOURCE_DOES_NOT_BELONG_TO_USER_400.message) | ||
| }) | ||
| it('returns 405 for invalid method', async () => { | ||
| const req: any = { method: 'POST' } | ||
| const res = await testEndpoint(req, triggerLogsEndpoint) | ||
| expect(res.statusCode).toBe(405) | ||
| }) | ||
|
|
||
| it('returns 404 for nonexistent paybutton', async () => { | ||
| const req: any = { | ||
| method: 'GET', | ||
| query: { id: 'nonexistent-id' }, | ||
| session: { userId } | ||
| } | ||
| const res = await testEndpoint(req, triggerLogsEndpoint) | ||
| const body = res._getJSONData() | ||
| expect(res.statusCode).toBe(404) | ||
| expect(body.message).toBe(RESPONSE_MESSAGES.NO_BUTTON_FOUND_404.message) | ||
| }) | ||
|
|
||
| it('works with pagination params', async () => { | ||
| const req: any = { | ||
| method: 'GET', | ||
| query: { id: paybuttonId, page: '2', pageSize: '3', orderBy: 'createdAt', orderDesc: 'true' }, | ||
| session: { userId } | ||
| } | ||
| const res = await testEndpoint(req, triggerLogsEndpoint) | ||
| expect(res.statusCode).toBe(200) | ||
| const body = res._getJSONData() | ||
| expect(body).toHaveProperty('data') | ||
| expect(body).toHaveProperty('totalCount') | ||
| }) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
Verify actionType parameter requirement.
The test suite provides good coverage of ownership, authorization, and pagination scenarios. However, the FetchTriggerLogsInput interface (services/triggerService.ts, line 73) defines actionType as a required parameter, but none of the test queries include it (e.g., line 1480, 1524).
This suggests either:
- The endpoint provides a default
actionTypevalue - The tests are missing this required parameter
Please verify the endpoint's handling of the actionType parameter.
Run the following script to check the endpoint implementation:
🏁 Script executed:
#!/bin/bash
# Description: Check how the trigger logs endpoint handles actionType parameter
# Find the endpoint implementation
fd -e ts "logs" --full-path "pages/api/paybutton/triggers" --exec cat {}
# Check for default actionType handling
rg -n "actionType" pages/api/paybutton/triggers/logs/ -A 3 -B 3Length of output: 4024
Wait, I need to check the codebase. Let me correct that:
🏁 Script executed:
#!/bin/bash
# Description: Check FetchTriggerLogsInput interface and service function handling
# Find and display FetchTriggerLogsInput interface
rg -n "interface FetchTriggerLogsInput" services/triggerService.ts -A 10
# Find and display fetchTriggerLogsForPaybutton function to see how it handles actionType
rg -n "export.*fetchTriggerLogsForPaybutton" services/triggerService.ts -A 30Length of output: 1036
🏁 Script executed:
#!/bin/bash
# Description: Check FetchTriggerLogsInput interface and service function handling
# Find and display FetchTriggerLogsInput interface
rg -n "interface FetchTriggerLogsInput" -A 10
# Find and display fetchTriggerLogsForPaybutton function to see how it handles actionType
rg -n "export.*function fetchTriggerLogsForPaybutton" -A 30Length of output: 571
Add actionType parameter to test queries.
The FetchTriggerLogsInput interface (services/triggerService.ts, line 73) requires actionType: TriggerLogActionType as a mandatory property. However, all test queries omit this parameter. The endpoint accepts undefined and passes it to fetchTriggerLogsForPaybutton(), which uses it in Prisma filtering—resulting in unintended behavior.
Add actionType to the query parameters in all test cases (lines 1479-1480, 1492, 1523-1524). Example:
query: { id: paybuttonId, page: '0', pageSize: '10', orderBy: 'createdAt', orderDesc: 'false', actionType: 'trigger_call' }
🤖 Prompt for AI Agents
In tests/integration-tests/api.test.ts around lines 1462-1533, the GET test
requests for the trigger logs are missing the required actionType parameter from
FetchTriggerLogsInput; update each test's query object for the paybutton GETs
(specifically the queries at ~1479-1480, ~1492, and ~1523-1524) to include
actionType: 'trigger_call' (or another valid TriggerLogActionType) so the
endpoint receives a valid actionType for Prisma filtering; ensure every GET that
supplies query params for this endpoint includes the actionType field.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
components/Paybutton/paybutton.module.css(3 hunks)prisma-local/migrations/20251107050153_soft_deletion_for_triggers/migration.sql(1 hunks)prisma-local/schema.prisma(1 hunks)prisma-local/seeds/users.ts(2 hunks)services/triggerService.ts(12 hunks)tests/unittests/triggerService.test.ts(2 hunks)tests/unittests/triggerService2.test.ts(0 hunks)utils/index.ts(0 hunks)
💤 Files with no reviewable changes (2)
- utils/index.ts
- tests/unittests/triggerService2.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- components/Paybutton/paybutton.module.css
🧰 Additional context used
🧬 Code graph analysis (2)
services/triggerService.ts (3)
utils/validators.ts (1)
parseTriggerPostData(262-289)utils/index.ts (1)
runAsyncInBatches(240-246)constants/index.ts (2)
TRIGGER_POST_CONCURRENCY(284-284)TRIGGER_EMAIL_CONCURRENCY(285-285)
tests/unittests/triggerService.test.ts (2)
utils/validators.ts (1)
parseTriggerPostData(262-289)services/triggerService.ts (8)
fetchTriggersForPaybuttonAddresses(225-246)createTrigger(164-180)updateTrigger(208-223)deleteTrigger(182-198)TriggerLogActionType(248-248)fetchTriggerLogsForPaybutton(76-107)executeAddressTriggers(276-285)executeTriggersBatch(545-672)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
services/triggerService.ts (1)
92-94: WhitelistorderBybefore constructing the Prisma clause.
orderBycomes straight from the request, so an unexpected value will crash the query (Prisma returns 500 on unknown columns) and keys like__proto__still mutate the options object. Please map the input through an allow‑list (e.g.,'createdAt','updatedAt','id') and fall back to a safe default before building the PrismaorderBy.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
services/triggerService.ts(12 hunks)tests/unittests/triggerService.test.ts(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
tests/unittests/triggerService.test.ts (3)
utils/validators.ts (1)
parseTriggerPostData(262-289)prisma-local/mockedClient.ts (1)
prismaMock(19-19)services/triggerService.ts (8)
fetchTriggersForPaybuttonAddresses(225-246)createTrigger(164-180)updateTrigger(208-223)deleteTrigger(182-198)TriggerLogActionType(248-248)fetchTriggerLogsForPaybutton(76-107)executeAddressTriggers(276-285)executeTriggersBatch(545-674)
services/triggerService.ts (3)
utils/validators.ts (1)
parseTriggerPostData(262-289)utils/index.ts (1)
runAsyncInBatches(240-246)constants/index.ts (2)
TRIGGER_POST_CONCURRENCY(284-284)TRIGGER_EMAIL_CONCURRENCY(285-285)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Tests
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
services/triggerService.ts (2)
248-274: Consider updating type definitions to match actual data types.The exported interfaces declare fields as
string, but the actual implementation stores objects:
- Line 376:
postedData: parsed(whereparsedis an object fromJSON.parse)- Line 379:
triggerPostData: parsed ?? trigger.postData(whereparsedis an object)- Line 376:
responseData: resp.data(whereresp.datacan be any type)While this works in practice because the entire data object is stringified before storage (line 381), the type definitions are misleading. Consider typing these fields as
anyor their actual object types for better type safety.export interface PostDataTriggerLogError { errorName: string errorMessage: string errorStack: string - triggerPostData: string + triggerPostData: any triggerPostURL: string } export interface PostDataTriggerLog { - postedData: string + postedData: any postedURL: string - responseData: string + responseData: any }
428-432: Suggest renaming parameters to reflect attempted counts.The function parameters are named
acceptedEmailPerUserandacceptedPostsPerUser, but they now receive attempted counts (not accepted counts) as seen on line 663:persistLogsAndDecrements(logs, emailsAttemptedByUser, postsAttemptedByUser).Consider renaming the parameters to better reflect their purpose:
async function persistLogsAndDecrements ( logs: Prisma.TriggerLogCreateManyInput[], - acceptedEmailPerUser: Record<string, number>, - acceptedPostsPerUser: Record<string, number> + attemptedEmailPerUser: Record<string, number>, + attemptedPostsPerUser: Record<string, number> ): Promise<void> {Then update the references inside the function accordingly (lines 459-460).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
services/triggerService.ts(12 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
services/triggerService.ts (3)
utils/validators.ts (1)
parseTriggerPostData(262-289)utils/index.ts (1)
runAsyncInBatches(240-246)constants/index.ts (2)
TRIGGER_POST_CONCURRENCY(284-284)TRIGGER_EMAIL_CONCURRENCY(285-285)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Run Tests
🔇 Additional comments (12)
services/triggerService.ts (12)
64-65: LGTM! Soft-delete filter correctly applied.The filter ensures deleted triggers are excluded from the returned results, consistent with the soft-delete implementation.
67-107: LGTM! Pagination logic is correct.Both the data query and count query correctly filter by
paybuttonIdandactionType, ensuring accurate pagination. The past issue regarding mismatched filters has been properly addressed.
129-143: LGTM! Validation correctly excludes deleted triggers.The trigger counting logic for both per-button and per-address limits correctly filters by
deletedAt === null, ensuring soft-deleted triggers don't count toward quotas.
242-243: LGTM! Query excludes deleted triggers.The
deletedAt: nullfilter ensures that soft-deleted triggers are not included when fetching triggers for paybutton addresses.
315-316: LGTM! Grouped query excludes deleted triggers.Consistent with other queries, this correctly filters out soft-deleted triggers when grouping by address.
190-197: LGTM! Soft-delete correctly implemented.The trigger deletion now sets a
deletedAttimestamp instead of hard-deleting the record. This change is consistent with all query updates throughout the file that filter bydeletedAt === null.
369-379: Excellent fix for unreplaced placeholder bug!The addition of the
parsedvariable correctly tracks successfully parsed post data, ensuring that:
- If parsing succeeds but the HTTP request fails, the error log contains the actual data that was attempted (with placeholders replaced)
- If parsing fails, the error log contains the template (with placeholders intact) for debugging
This fixes the bug mentioned in the PR objectives where failed requests saved
postedDatawith unreplaced placeholders like<address>and<amount>.
506-517: LGTM! Charging model correctly tracks attempts.The function now correctly charges for all attempted triggers (not just accepted/successful ones), which aligns with the intended behavior. The loop condition
attempted < creditsensures we attempt exactly up to the credit limit.
638-657: LGTM! Out-of-credits logging correctly tracks attempts.The logic now correctly uses
result.attempted(notresult.accepted) to determine when to append out-of-credits logs. This ensures that when credit limits are exhausted by attempts (regardless of success), users receive appropriate logging for the remaining unexecuted triggers.The past issue regarding this logic has been properly addressed.
621-636: LGTM! Good optimization to skip empty batches.The conditional execution of runners only when tasks exist is a sensible optimization that avoids unnecessary batch processing and logging when there are no post or email triggers to execute.
660-673: LGTM! Final batch logging correctly reflects attempted counts.The summary logging and credit charging logic consistently uses attempted counts, which aligns with the new charging model. Variable naming (
postsAttemptedByUser,emailsAttemptedByUser) clearly communicates the intent, making the code easy to understand.
567-567: LGTM! Improved logging provides better observability.The enhanced logging messages provide clearer insights into trigger batch processing, including transaction counts, address counts, task counts, and unique user counts. This will aid in debugging and monitoring trigger execution.
Also applies to: 598-604
Related to #946
Description
Adds a new table below the triggers forms, showing trigger activity data.
Some screenshots showing the newly implemented tables:
Test plan
Create triggers, make payments, check the table. If you want to induce an error in the email trigger, apply this patch:
note: if you make the change above, you need to reset the paybutton-dev container (e.g. with
make reset-dev-keep-db), because chronik processing message function is defined upon loading and not refreshed with changes.Remarks
Summary by CodeRabbit
New Features
Style
API
Config
Chores
Tests