Extended beta feedback: personnel column renames, search filters#247
Conversation
Rename subtable headers for consistency with the outer table and clarity: Salary → Monthly Salary, CBR → Monthly CBR, Funding Effective → Funding Eff. Date, Funding End → Funding End Date. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reason: clicking "All Reports" landed users without AccrualViewer on the empty-state /reports page. Gate the catalog entry on the same condition that decides whether /reports has any visible items. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Reason: project number was rendered inside the displayName cell but not exposed to TanStack's global filter, which only walks accessor values. Concatenate name and number in the displayName accessor so the column search matches on either field while sort still primarily orders by name. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughProject table columns are updated to use accessor functions concatenating displayName and projectNumber for proper search filtering. PersonnelTable column headers now include "Date" and "Monthly" wording, with CBR tooltips referencing monthlyDefinitions. The "All Reports" catalog entry is gated behind CanViewAccruals authorization on the server. Tests validate these behaviors. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
web/client/src/components/project/InternalProjectsTable.tsx (1)
124-127: Clarify the inline rationale.This comment is copied from the sponsored table, but
InternalProjectsTableonly renders the project number in the cell; the display name is used in thetitleand accessor. Updating the wording will keep the explanation accurate.Suggested wording
- // Accessor concatenates name + number so the global filter matches on - // either — the cell visibly renders both, but TanStack only filters - // accessor values, not rendered output. + // Accessor concatenates name + number so the global filter matches on + // either — the cell shows the project number, and TanStack only filters + // accessor values, not rendered output.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@web/client/src/components/project/InternalProjectsTable.tsx` around lines 124 - 127, Update the inline comment above the columnHelper.accessor in InternalProjectsTable to accurately reflect that the cell only visibly renders the project number while the display name is included in the accessor and as the cell title; replace the copied wording about rendering both name + number with a concise explanation that the accessor concatenates displayName and projectNumber so the global filter matches either value, but the visible cell content is just the projectNumber (displayName is used in the title/tooltip).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@web/client/src/components/project/InternalProjectsTable.tsx`:
- Around line 124-127: Update the inline comment above the columnHelper.accessor
in InternalProjectsTable to accurately reflect that the cell only visibly
renders the project number while the display name is included in the accessor
and as the cell title; replace the copied wording about rendering both name +
number with a concise explanation that the accessor concatenates displayName and
projectNumber so the global filter matches either value, but the visible cell
content is just the projectNumber (displayName is used in the title/tooltip).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0ce5844-4b5c-4390-b83d-d16ca2057eb3
📒 Files selected for processing (8)
web/client/src/components/project/InternalProjectsTable.tsxweb/client/src/components/project/PersonnelTable.tsxweb/client/src/components/project/SponsoredProjectsTable.tsxweb/client/src/test/components/PersonnelTable.test.tsxweb/client/src/test/components/project/ProjectsTable.test.tsxweb/client/src/test/components/project/SponsoredProjectsTable.test.tsxweb/server/Controllers/SearchController.csweb/tests/server.tests/Controllers/SearchControllerTests.cs
Summary
Three items from issue #224 (extended beta feedback). Hourly rate work is deferred to a separate issue.
fcd4fb94) — Salary → Monthly Salary, CBR → Monthly CBR (with monthlyCbr tooltip), Funding Effective → Funding Eff. Date, Funding End → Funding End Date.391bc020) — server gates the catalog entry on the same condition that decides whether/reportshas visible items (currentlyCanViewAccruals). Tests added mirroring existing accruals coverage.6d49fff9) —InternalProjectsTableandSponsoredProjectsTablenow match the global filter against project number as well as name. The cell already rendered both, but TanStack only filters accessor values; the displayName accessor now concatenates name + number. Sidebar and command palette already searched by project number.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements