Add option to give a review comment when approving / denying inactivity notices / resignations - #179
Add option to give a review comment when approving / denying inactivity notices / resignations#179raadtotheraad wants to merge 10 commits into
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe notices page adds optional per-item reviewer comments for notices and resignations. It submits, clears, and displays those comments. ChangesReview comment handling
Package and tooling updates
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1⚔️ Resolve merge conflicts 💡
🧪 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: 2
🤖 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 `@package.json`:
- Line 85: Align all Prisma dependencies, including `@prisma/client`, prisma, and
`@prisma/adapter-pg`, to the same version across package.json and
package-lock.json; update the resolved lockfile entries and metadata
consistently.
In `@pages/workspace/`[id]/notices/index.tsx:
- Around line 415-416: Replace the page-level resignationReviewComment and
noticeReviewComment values with draft state keyed by each request’s
notice/resignation ID. Update each card’s textarea binding and change handlers
to read and write only that card’s ID entry, and ensure approval/denial handlers
submit the comment associated with their target request.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 6e871a37-9987-4bbe-a717-ff57da570d0f
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.jsonpages/workspace/[id]/notices/index.tsx
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
package.json (1)
85-85: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winMove
@prisma/clienttodependencies.
utils/database.tsimports@prisma/clientat runtime, but it is currently listed underdevDependencieswhilenpm ci --omit=devbehavior keeps the@prisma/adapter-pgruntime dependency independencies. Promote@prisma/clientso production installs can resolve the runtime Prisma client; keep theprismaCLI indevDependencies.🤖 Prompt for 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. In `@package.json` at line 85, Move `@prisma/client` from devDependencies to dependencies in package.json so runtime imports from utils/database.ts resolve in production installs, while keeping the prisma CLI under devDependencies.pages/workspace/[id]/notices/index.tsx (2)
891-898: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve line breaks in displayed review comments. The new textareas accept multiline input, but both display paragraphs collapse newlines.
pages/workspace/[id]/notices/index.tsx#L891-L898: addwhitespace-pre-wrapor equivalent to the notice comment paragraph.pages/workspace/[id]/notices/index.tsx#L943-L951: addwhitespace-pre-wrapor equivalent to the resignation comment paragraph.🤖 Prompt for 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. In `@pages/workspace/`[id]/notices/index.tsx around lines 891 - 898, Preserve multiline formatting in both displayed comment paragraphs by adding whitespace-pre-wrap or an equivalent whitespace-preserving class to the notice review comment at pages/workspace/[id]/notices/index.tsx:891-898 and the resignation comment at pages/workspace/[id]/notices/index.tsx:943-951.
1011-1022: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winAdd item-specific accessible names to the review textareas.
pages/workspace/[id]/notices/index.tsx#L1011-L1022: associate the resignation textarea with an item-specific<label>/idpair oraria-label.pages/workspace/[id]/notices/index.tsx#L1116-L1127: associate the notice textarea with an item-specific<label>/idpair oraria-label.🤖 Prompt for 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. In `@pages/workspace/`[id]/notices/index.tsx around lines 1011 - 1022, Add item-specific accessible names to both review textareas in pages/workspace/[id]/notices/index.tsx: update the resignation textarea near lines 1011-1022 and the notice textarea near lines 1116-1127 with unique IDs paired to descriptive labels or item-specific aria-label values, preserving their existing state and change handlers.
🤖 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.
Outside diff comments:
In `@package.json`:
- Line 85: Move `@prisma/client` from devDependencies to dependencies in
package.json so runtime imports from utils/database.ts resolve in production
installs, while keeping the prisma CLI under devDependencies.
In `@pages/workspace/`[id]/notices/index.tsx:
- Around line 891-898: Preserve multiline formatting in both displayed comment
paragraphs by adding whitespace-pre-wrap or an equivalent whitespace-preserving
class to the notice review comment at
pages/workspace/[id]/notices/index.tsx:891-898 and the resignation comment at
pages/workspace/[id]/notices/index.tsx:943-951.
- Around line 1011-1022: Add item-specific accessible names to both review
textareas in pages/workspace/[id]/notices/index.tsx: update the resignation
textarea near lines 1011-1022 and the notice textarea near lines 1116-1127 with
unique IDs paired to descriptive labels or item-specific aria-label values,
preserving their existing state and change handlers.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 3a8a78f3-46e1-42f3-ac73-98046bb6470d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.jsonpages/workspace/[id]/notices/index.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/workspace/[id]/notices/index.tsx (1)
491-506: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftFull-page reload loses unrelated per-item review drafts.
The new keyed state is correct, but successful approval/denial reloads the page and clears every unsaved draft, not just the submitted item.
pages/workspace/[id]/notices/index.tsx#L491-L506: update the notice collections locally or persistnoticeReviewCommentsacross reloads.pages/workspace/[id]/notices/index.tsx#L551-L565: update the resignation collections locally or persistresignationReviewCommentsacross reloads.🤖 Prompt for 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. In `@pages/workspace/`[id]/notices/index.tsx around lines 491 - 506, Replace the successful non-cancel full-page reload in the notice handler around setAllNotices with a local update to the affected notice collection, preserving unrelated noticeReviewComments drafts; apply the same local-update approach to the resignation handler around setAllResignations and resignationReviewComments. Both affected sites in pages/workspace/[id]/notices/index.tsx (lines 491-506 and 551-565) must avoid reloads that clear other per-item drafts while removing or updating only the submitted item.
🤖 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.
Outside diff comments:
In `@pages/workspace/`[id]/notices/index.tsx:
- Around line 491-506: Replace the successful non-cancel full-page reload in the
notice handler around setAllNotices with a local update to the affected notice
collection, preserving unrelated noticeReviewComments drafts; apply the same
local-update approach to the resignation handler around setAllResignations and
resignationReviewComments. Both affected sites in
pages/workspace/[id]/notices/index.tsx (lines 491-506 and 551-565) must avoid
reloads that clear other per-item drafts while removing or updating only the
submitted item.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8ce6cf21-bc46-4159-a08a-352c78379bd6
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
package.jsonpages/workspace/[id]/notices/index.tsx
BuddyWinte
left a comment
BuddyWinte
left a comment
There was a problem hiding this comment.
The review comment state is shared across all pending notices, so typing a comment on one request also updates every other textarea. Each request should have its own independent review comment state.
|
Coderabbit already noted that and I think I've addressed it in commit d4fe5d7 |
The commit you mentioned is just the push to the newest prisma version |
Wrong commit Check 1d751 |
|
LGTM, sorry for confusion 🫡 |
No problem |
|
@raadtotheraad Please run |
|
Yeah I know the issue I already fixed this some time ago but it seems it doesn't want to apply (GitHub problem? Not sure) I'll resolve this later or you can edit the changes by changing the versions, it's just a prisma version mismatch with the package.json and lockfile |
|
Just revert the package.json back to the old version, I'll update depends later |
|
K will do that once I can |
I see in a commit you updated package.json I updated the package.json to yours and soon once i install the stuff i'll commit the changes |
|
The lockfile should be up to date now I think you set I fixed it bcuz it was erroring out!! |
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 `@package.json`:
- Around line 5-13: Update the CI workflow and Dockerfile dependency
installation and script execution to use Bun consistently with the
packageManager declaration, and reference bun.lockb instead of pnpm’s lockfile.
Ensure all affected install, build, and typecheck commands use Bun without
mixing package managers.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a80b9bbe-2faa-497d-acba-1f4c552d7907
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json
| "packageManager": "bun@1.3.14", | ||
| "scripts": { | ||
| "dev": "next dev", | ||
| "build": "cross-env NODE_OPTIONS=\"--max-old-space-size=4096\" next build --webpack", | ||
| "build": "NODE_OPTIONS=\"--max-old-space-size=4096\" next build --webpack", | ||
| "dev:https": "next dev --experimental-https --webpack", | ||
| "start": "next start", | ||
| "lint": "next lint", | ||
| "lint": "eslint .", | ||
| "typecheck": "tsc --noEmit", | ||
| "prisma:generate": "prisma generate" |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Declared package manager and lockfiles =="
sed -n '1,140p' package.json
fd -HI -t f '^(bun\.lock|bun\.lockb|pnpm-lock\.yaml|package-lock\.json|yarn\.lock)$' . -d 2
echo "== Package-manager commands in affected consumers =="
rg -n -C 2 'pnpm|bun|packageManager|frozen-lockfile|prisma generate' \
package.json .github/workflows/typeCheck.yml DockerfileRepository: PlanetaryOrbit/orbit
Length of output: 6588
Use Bun consistently in CI and Docker.
package.json declares Bun as the package manager, but .github/workflows/typeCheck.yml and Dockerfile still install dependencies and run scripts with pnpm. Use Bun and bun.lockb, or retain pnpm consistently across these consumers.
🤖 Prompt for 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.
In `@package.json` around lines 5 - 13, Update the CI workflow and Dockerfile
dependency installation and script execution to use Bun consistently with the
packageManager declaration, and reference bun.lockb instead of pnpm’s lockfile.
Ensure all affected install, build, and typecheck commands use Bun without
mixing package managers.
There was a problem hiding this comment.
Will start using Bun, im gonna try to integrate it into the existing project
There was a problem hiding this comment.
@raadtotheraad, sounds good. Please update the CI workflow and Dockerfile with the Bun migration so they use the same package manager and lockfile as package.json. The finding will remain open until those changes are present.
You are interacting with an AI system.
…hall bow down to its great aura. btw the bun lockfile is really long idk why
I have integrated Bun and it should work now |
|
Im Done with the stuff including merge problems Should hopefully work |
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
I'll merge this after the API rework, problem is that this still uses pnpm we use bun now. |
I believe you may have not noticed the changes but I may have already switched it to bun Altough I'm not sure if there's still some pnpm included |
2 participants
This feature actually makes the
reviewCommentoption in inactivity notices and resignations have useIt allows reviewers to add an optional comment as a textbox, which then gets displayed with the approved or denied message when checking its status on the Notices page.
When no review comment is added, the box with the review comment doesn't appear
Here are 9 attachments which show each function:
Demo8.mp4
Demo7.mp4
Demo6.mp4
Demo5.mp4
Demo4.mp4
Demo3.mp4
Demo2.mp4
Demo1.mp4
Summary by CodeRabbit
New Features
Bug Fixes