Repository navigation
chore: gate shared package, remove Storybook, document rate-limit env - #649
Ryzenthefirst wants to merge 2 commits into
Conversation
- Run @accensa/shared lint and typecheck from the root scripts (accensa#594) - Remove the unbuilt Storybook config, stories and devDependencies (accensa#595) - Document UPSTASH_REDIS_REST_URL/TOKEN in apps/web/README.md (accensa#589)
|
@Ryzenthefirst Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
|
@Ryzenthefirst is attempting to deploy a commit to the ACCENSA Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info
📝 Walkthrough
Merge Risk: ⚪ Minimal · up to
|
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Out of Scope Changes check |  |
The new apps/web/src/app/api/merchant/settings/notifications/route.test.ts tests are not connected to the supplied linked issues [#594], [#595], or [#589]. They test notification-settings route vali… |
Remove the notification-settings route tests from this pull request, or link the issue that defines them as a direct objective. |
| Docstring Coverage | |
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (3 skipped: 3 … | Write docstrings for the functions missing them to satisfy the coverage threshold. |
| Linked Issues check |  |
The supplied summary supports the implementation for [#594]: root lint and typecheck retain the existing filters and add @accensa/shared. It also supports the deletion path for [#595], including… |
Provide CI results for the root pnpm lint and pnpm typecheck commands, or equivalent evidence that the shared package checks exit zero. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly summarizes the main changes: adding shared-package checks, removing Storybook, and documenting rate-limit environment variables.
Full details: Out of Scope Changes check
-
Explanation
The new
apps/web/src/app/api/merchant/settings/notifications/route.test.tstests are not connected to the supplied linked issues [#594], [#595], or [#589]. They test notification-settings route validation, persistence, and database guards. Thedata-table.tsxdocumentation change supports the Storybook removal and is in scope, but the notification route tests are a separate feature area.Resolution
Remove the notification-settings route tests from this pull request, or link the issue that defines them as a direct objective.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. (3 skipped: 3 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
Full details: Linked Issues check
-
Explanation
The supplied summary supports the implementation for [
#594]: rootlintandtypecheckretain the existing filters and add@accensa/shared. It also supports the deletion path for [#595], including the scripts, Storybook configuration, dependencies, and three story files, plus README reasoning. The README changes support [#589] by naming both variables and consumers, the all-or-nothing test, the per-process fallback, and the multi-instance requirement. The author did not confirmpnpm lintorpnpm typecheck, and no CI results are supplied. Therefore, the required zero exit status for [#594] is not established.Resolution
Provide CI results for the root
pnpm lintandpnpm typecheckcommands, or equivalent evidence that the shared package checks exit zero.
- Fix all pre-merge checks with AI
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
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 help to get the list of available commands.
Covers GET public projection, both DATABASE_URL 500 guards, the two body-shape 400s and each field-level 400 message as its own case (accensa#596).
This branch has not been deployed
1 participant
Closes #596
Closes #594
Closes #595
Closes #589
Changes
lintandtypecheckscripts now also run@accensa/shared's owntsc --noEmit. No existing filter dropped; no CI step changed.storybook/build-storybookscripts,apps/web/.storybook/, the three@storybook/*devDependencies plusstorybook, and the three.stories.tsxfiles (web + shared). Lockfile regenerated. Reasoning recorded inapps/web/README.md.route.test.tsfor the notification settings route; each rejection message is its own named case.apps/web/README.mddocumentingUPSTASH_REDIS_REST_URL/UPSTASH_REDIS_REST_TOKEN, both consumers, the all-or-nothing&&test, the per-process in-memory fallback, and the multi-instance requirement.Verification
prettier --checkon every touched file passes.pnpm lint,pnpm typecheckandpnpm testwere not confirmed locally (a full install took ~47 min on a slow registry); CI will run them.apps/web/src/app/api/merchant/settings/notifications/route.test.ts(11 tests, run in isolation) passes: proves each of the five field-level 400 messages (route.ts:73, :79, :88, :97, :106), the secret-stripping GET projection and the DATABASE_URL 500 guard on both verbs.Summary by CodeRabbit