fix(web,ci): issue #43 batch P3+P4 — tooling / tests / labeler (7 items) - #62
Conversation
|
Warning Review limit reached
More reviews will be available in 1 hour, 4 minutes, and 7 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughCe PR apporte : la correction de la résolution de chemin dans ChangesTests, config Vitest, migration DB et CI
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
github-actions
Bot
added
type: fix
|
@coderabbitai full review please. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "=== PR `#62` diff ==="
gh pr view 62 --repo InstaZDLL/waveflow-server --json title,body,files
echo ""
echo "=== Full diff ==="
gh pr diff 62 --repo InstaZDLL/waveflow-serverLength of output: 16430
✅
|
| Cas | Vérification |
|---|---|
| Nom vide (whitespace-only) | Déclenche "Display name is required" + signUpEmail non appelé ✓ |
| Mot de passe à MAX+1 (129 chars) | Déclenche le message de borne supérieure + signUpEmail non appelé ✓ |
| Trim whitespace | Payload wire = valeurs trimées ✓ |
Un point à noter : le test « trim » passe ' daisy@example.com ' dans un champ type="email". jsdom n'applique pas la validation HTML5 sur fireEvent.change, donc ce cas fonctionne en test unitaire — mais c'est aussi ce que vous voulez tester (la logique de trim dans le composant avant l'appel réseau).
Aucun bug, problème de logique, ou regression détectés. Le PR peut être mergé.
[approve]
|
@coderabbitai review |
✅ Action performedReview finished.
|
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/labeler.yml:
- Around line 55-61: The glob patterns for sign-in and sign-up in the labeler
configuration do not match the actual .tsx files that exist. The pattern
sign-in{,/**} expands to either sign-in (directory) or sign-in/** (directory
contents) but does not include sign-in.tsx. Update the patterns for both
web/src/routes/sign-in{,/**} and web/src/routes/sign-up{,/**} to also explicitly
match the .tsx file variants so the scope: auth label is correctly applied to
both the individual sign-in.tsx and sign-up.tsx files and any future
subdirectories under those names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 8f8107c2-d5aa-414f-b5fa-222f8496ac97
📒 Files selected for processing (12)
.github/labeler.yml.github/pull_request_template.mdweb/scripts/db-migrate.tsweb/src/routes/-sign-in.test.tsweb/src/routes/_authed.profiles.$profileId.libraries.$libraryId.albums.$albumId.test.tsxweb/src/routes/_authed.profiles.$profileId.libraries.$libraryId.albums.test.tsxweb/src/routes/_authed.profiles.$profileId.libraries.$libraryId.artists.$artistId.test.tsxweb/src/routes/_authed.profiles.$profileId.libraries.$libraryId.artists.test.tsxweb/src/routes/_authed.profiles.$profileId.playlists.$playlistId.test.tsxweb/src/routes/_authed.profiles.$profileId.playlists.test.tsxweb/src/routes/sign-up.test.tsxweb/vitest.config.ts
Follow-up to #62 review. The previous brace expansion `sign-in{,/**}` expanded to `sign-in` (literal — a file or dir NAMED `sign-in`, no extension, doesn't exist) and `sign-in/**` (subtree). Neither shape catches the actual `sign-in.tsx` file at the routes root, so PRs touching `sign-in.tsx` or `sign-up.tsx` were silently missing the `scope: auth` label. `sign-in{.tsx,/**}` is the right union: matches the literal `.tsx` file today AND any future `sign-in/` subtree (e.g. `sign-in.callback.tsx`, `sign-in/index.tsx`). Same fix on `sign-up`. Validation: minimatch eval against the brace expansion gives `sign-in.tsx` and `sign-in/**` — both real-world shapes covered. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
|
@coderabbitai fix appliqué sur commit 8220b9f. Tu avais raison : Switch à Commentaire mis à jour pour expliquer le bug de l'expansion précédente afin d'éviter qu'un futur refactor le réintroduit. |
|
Tip For best results, initiate chat on the files or code changes.
🐇 ʕ •ᴥ•ʔ ✓ |
Follow-up to #62 review. The previous brace expansion `sign-in{,/**}` expanded to `sign-in` (literal — a file or dir NAMED `sign-in`, no extension, doesn't exist) and `sign-in/**` (subtree). Neither shape catches the actual `sign-in.tsx` file at the routes root, so PRs touching `sign-in.tsx` or `sign-up.tsx` were silently missing the `scope: auth` label. `sign-in{.tsx,/**}` is the right union: matches the literal `.tsx` file today AND any future `sign-in/` subtree (e.g. `sign-in.callback.tsx`, `sign-in/index.tsx`). Same fix on `sign-up`. Validation: minimatch eval against the brace expansion gives `sign-in.tsx` and `sign-in/**` — both real-world shapes covered. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
8220b9f to
c7e90a1
Compare
7 findings from CodeRabbit on PR #41 / issue #43 — the P3 tooling batch (6 items) + the single P4 labeler item, bundled because they all touch dev / CI infrastructure rather than runtime code. ## Changes - `web/scripts/db-migrate.ts` — resolve `MIGRATIONS_DIR` relative to the script via `fileURLToPath(import.meta.url)` instead of `process.cwd()`. After the monorepo merge the script can be invoked from the repo root (`bun --cwd=web run db:migrate`); the previous CWD anchor broke silently because it pointed at `<repo>/db/migrations`, which doesn't exist. - `web/vitest.config.ts` — `passWithNoTests: false`. The flag was a bootstrap-era convenience while the suite was empty; now that 230+ tests are landing, a typo in the `include` glob (or an accidental file move that breaks discovery) would silently pass CI under `true`. Failing loud is the right signal. - `web/src/routes/sign-up.test.tsx` — three new cases: - empty / whitespace-only display name → blocked - password at MAX_PASSWORD + 1 (129 chars) → blocked - surrounding whitespace on name + email → trimmed before `signUp.email` call - Six route-adjacent test files (`sign-up.test.tsx`, `_authed.profiles.$profileId.libraries.$libraryId.albums*`, `…artists*`, `…playlists.test.tsx`, `…playlists.$playlistId.test.tsx`) — switch `React.PropsWithChildren` (which referenced the global `React` namespace without an `import * as React`) to a typed `PropsWithChildren` imported via `import type { PropsWithChildren } from 'react'`. The tests passed before only because the JSX transform happens to expose `React` ambient; the explicit import locks the contract and is what every other test file in the suite already does. - `web/src/routes/sign-in.test.ts` → `-sign-in.test.ts` — TanStack Router's `-`-prefix convention excludes the file from the auto-generated route tree. Without the rename the router plugin had been incidentally treating `sign-in.test.ts` as a candidate route module on every dev reload. Kept `.ts` (not `.tsx`) because the test doesn't render JSX (`Link: () => null` mock). - `.github/pull_request_template.md` — first heading `## Summary` → `# Summary`. MD041 (first-line-heading) wants H1; the template was the only file in the repo tripping it. - `.github/labeler.yml` — `web/src/routes/sign-in/**` / `…sign-up/**` only matched DIRECTORIES; the actual routes are `sign-in.tsx` / `sign-up.tsx` files at the root of `routes/`. Switch to `sign-in{,/**}` / `sign-up{,/**}` so a PR touching either form lights up the auth label. ## Validation - `bun run typecheck` clean. - `bun run lint` clean. - `bun run test` — 236 pass across 26 files (was 233, +3 new sign-up boundary cases). - `prettier --check` on the modified files clean. ## Refs Closes 7/24 items of #43. Cumulative: 18/24 across the three batched PRs (#60 P1 + #61 P2 + this). The 4 P0 items + 1 deferred P2 (styles.css OKLCH unification) remain. P0 was tagged in the issue as "ship with the archive PR" — verifying status as a follow-up. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
Follow-up to #62 review. The previous brace expansion `sign-in{,/**}` expanded to `sign-in` (literal — a file or dir NAMED `sign-in`, no extension, doesn't exist) and `sign-in/**` (subtree). Neither shape catches the actual `sign-in.tsx` file at the routes root, so PRs touching `sign-in.tsx` or `sign-up.tsx` were silently missing the `scope: auth` label. `sign-in{.tsx,/**}` is the right union: matches the literal `.tsx` file today AND any future `sign-in/` subtree (e.g. `sign-in.callback.tsx`, `sign-in/index.tsx`). Same fix on `sign-up`. Validation: minimatch eval against the brace expansion gives `sign-in.tsx` and `sign-in/**` — both real-world shapes covered. Signed-off-by: InstaZDLL <github.105mh@8shield.net>
c7e90a1 to
7300e0a
Compare
1 participant
Summary
7 P3+P4 findings from CodeRabbit on PR #41 / issue #43. Bundled
because they all touch dev / CI infrastructure rather than runtime
code.
Changes
web/scripts/db-migrate.tsMIGRATIONS_DIRviafileURLToPath(import.meta.url)instead ofprocess.cwd()so the script works from the repo root post-monorepo mergeweb/vitest.config.tspassWithNoTests: false— bootstrap-era convenience is now a CI footgun with 230+ testsweb/src/routes/sign-up.test.tsxReact.PropsWithChildren(ambient global ref) →import type { PropsWithChildren }(explicit import)web/src/routes/sign-in.test.ts→-sign-in.test.ts--prefix exclusion convention.github/pull_request_template.md## Summary→# Summary(MD041).github/labeler.ymlsign-in/**/sign-up/**→sign-in{,/**}/sign-up{,/**}— match files at the routes rootTest plan
bun run typecheckcleanbun run lintcleanbun run test— 236 pass (was 233, +3 new sign-up cases)prettier --checkon the modified files cleanRefs
Closes 7/24 items of #43. Cumulative: 18/24 across the three batched PRs (#60 P1 + #61 P2 + this).
The 4 P0 items + 1 deferred P2 (styles.css OKLCH unification) remain. P0 was tagged in the issue as "ship with the archive PR" — verifying status as a follow-up.
Summary by CodeRabbit
Notes de version
Bug Fixes
Tests
Chores