feat(estimates): show the real estimate value in view and module list rows - #299
Merged
Merged
Conversation
… rows The estimate column in the project view and module work-item lists rendered a hardcoded placeholder. Both pages now load the project's estimates and resolve each work item's estimate_point_id to its point value (mirroring the issue detail estimate picker), falling back to the placeholder when a work item has no estimate. Closes Devlaner#127 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthrough
ChangesEstimate display
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (3)
apps/web/src/pages/ModuleDetailPage.tsx (2)
496-501: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPre-build an estimate point lookup map for O(1) resolution.
estimateValuecallsestimates.flatMap((e) => e.points).find(...)on every issue in every render, allocating a new array each time. For lists with many issues this is wasteful. AuseMemothat builds aMap<pointId, value>once perestimateschange gives O(1) lookups.♻️ Suggested refactor
+ const estimatePointMap = useMemo(() => { + const m = new Map<string, string>(); + for (const e of estimates) for (const p of e.points) m.set(p.id, p.value); + return m; + }, [estimates]); const estimateValue = (issue: IssueApiResponse) => { if (!issue.estimate_point_id) return '—'; - return ( - estimates.flatMap((e) => e.points).find((p) => p.id === issue.estimate_point_id)?.value ?? '—' - ); + return estimatePointMap.get(issue.estimate_point_id) ?? '—'; };🤖 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 `@apps/web/src/pages/ModuleDetailPage.tsx` around lines 496 - 501, Update the estimate lookup near estimateValue to use a useMemo-built Map keyed by estimate point ID, recomputing only when estimates changes. Then have estimateValue return the mapped value with the existing '—' fallback, removing the per-call flatMap and find allocation.
496-501: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the
estimateValuehelper to a shared utility.The same
estimateValuelogic is duplicated inViewDetailPage.tsx(lines 857-862) and a similar flatMap-and-find pattern exists inIssueDetailPage.tsx. Extracting a sharedbuildEstimatePointMap(estimates)orresolveEstimateValue(estimates, issue)utility would eliminate triplication and ensure consistent fallback behavior.🤖 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 `@apps/web/src/pages/ModuleDetailPage.tsx` around lines 496 - 501, Extract the duplicated estimate lookup logic from estimateValue in ModuleDetailPage and its counterparts in ViewDetailPage and IssueDetailPage into a shared utility, preferably a resolveEstimateValue or buildEstimatePointMap helper. Update all callers to use it, preserving the existing em-dash fallback for missing estimate_point_id or unmatched points and consistent estimate resolution.apps/web/src/pages/ViewDetailPage.tsx (1)
857-862: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPre-build an estimate point lookup map for O(1) resolution.
Same as
ModuleDetailPage.tsx—estimateValueallocates a new flat array per call per issue. AuseMemoMap eliminates the repeatedflatMap.♻️ Suggested refactor
+ const estimatePointMap = useMemo(() => { + const m = new Map<string, string>(); + for (const e of estimates) for (const p of e.points) m.set(p.id, p.value); + return m; + }, [estimates]); const estimateValue = (issue: IssueApiResponse) => { if (!issue.estimate_point_id) return '—'; - return ( - estimates.flatMap((e) => e.points).find((p) => p.id === issue.estimate_point_id)?.value ?? '—' - ); + return estimatePointMap.get(issue.estimate_point_id) ?? '—'; };🤖 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 `@apps/web/src/pages/ViewDetailPage.tsx` around lines 857 - 862, Update the estimate lookup logic near estimateValue in ViewDetailPage to build a memoized Map of estimate point IDs to values with useMemo, rather than calling estimates.flatMap(...).find(...) for each issue. Have estimateValue resolve issue.estimate_point_id directly from this map while preserving the existing '—' fallback for missing IDs or values.
🤖 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.
Nitpick comments:
In `@apps/web/src/pages/ModuleDetailPage.tsx`:
- Around line 496-501: Update the estimate lookup near estimateValue to use a
useMemo-built Map keyed by estimate point ID, recomputing only when estimates
changes. Then have estimateValue return the mapped value with the existing '—'
fallback, removing the per-call flatMap and find allocation.
- Around line 496-501: Extract the duplicated estimate lookup logic from
estimateValue in ModuleDetailPage and its counterparts in ViewDetailPage and
IssueDetailPage into a shared utility, preferably a resolveEstimateValue or
buildEstimatePointMap helper. Update all callers to use it, preserving the
existing em-dash fallback for missing estimate_point_id or unmatched points and
consistent estimate resolution.
In `@apps/web/src/pages/ViewDetailPage.tsx`:
- Around line 857-862: Update the estimate lookup logic near estimateValue in
ViewDetailPage to build a memoized Map of estimate point IDs to values with
useMemo, rather than calling estimates.flatMap(...).find(...) for each issue.
Have estimateValue resolve issue.estimate_point_id directly from this map while
preserving the existing '—' fallback for missing IDs or values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d1209fcf-8c12-47b9-a1b4-e5df6a53344a
📒 Files selected for processing (2)
apps/web/src/pages/ModuleDetailPage.tsxapps/web/src/pages/ViewDetailPage.tsx
martian56
approved these changes
Jul 12, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Feature summary
The estimate column in the project view and module work-item lists rendered a hardcoded placeholder instead of the work item's real estimate. Both now resolve and display the actual estimate point value. This finishes the last user-facing gap in estimates (the systems + points + issue-detail picker landed in #222/#223).
Linked issues / discussion
Closes #127
User-facing behavior
In a project View and a Module's work-item list, when the Estimate display column is enabled, each row now shows the work item's estimate (e.g.
5) instead of a dash. Work items with no estimate still show—.What changed
apps/web/src/pages/ViewDetailPage.tsxandapps/web/src/pages/ModuleDetailPage.tsx: load the project's estimates alongside the other project data, and add anestimateValue(issue)helper that resolvesissue.estimate_point_idagainst the loaded estimate points (the same resolution the issue-detail estimate picker uses). The estimate column renders that value.Why this design
The issue-detail page already resolves an estimate point as
estimates.flatMap(e => e.points).find(p => p.id === estimate_point_id).value. Reusing that exact approach keeps the list rows consistent with the detail view and avoids a new endpoint (estimates are already a per-project list).Test plan
npm run typecheck,npm run lint(full) green.5to a work item, added it to a module, and confirmed the module list row shows 5 in the estimate column (previously—). Screenshot verified.Out of scope (follow-ups)
WorkspaceViewsPage) still renders placeholder cells for estimate as well as module/cycle/link/attachment. Those cells span multiple projects and would need cross-project estimate/module resolution — a separate, broader gap than this issue.npm run validate), so this is verified via typecheck/lint/build + manual E2E rather than a new UI test.AI assistance
Claude Code— and AI-assisted commits include aCo-Authored-By:trailerChecklist
Summary by CodeRabbit
New Features
Bug Fixes