feat(cycle): compute real cycle and module completion progress - #273
Conversation
Cycle and module list progress was faked from status (0% or 100%). Adds bulk per-project state-group distribution queries and cycles-progress / modules-progress endpoints (mirroring epics-progress), and renders both list pages' progress from completed/total. The modules progress sort now uses the real value too. Closes Devlaner#185 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
📝 WalkthroughWalkthroughAdds real cycle and module progress endpoints backed by state-group counts, then updates the web pages to fetch and render completion from ChangesCycle/Module Progress Feature
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CyclesPage
participant cycleService
participant CycleHandler
participant CycleService
participant CycleStore
CyclesPage->>cycleService: listProgress(workspaceSlug, projectId)
cycleService->>CycleHandler: GET /cycles-progress/
CycleHandler->>CycleService: ProgressBulk(ctx, slug, projectID, userID)
CycleService->>CycleStore: StateDistributionByProject(ctx, projectID)
CycleStore-->>CycleService: per-cycle grouped counts
CycleService-->>CycleService: compute total per cycle
CycleService-->>CycleHandler: progress map
CycleHandler-->>cycleService: 200 JSON progress
cycleService-->>CyclesPage: Record<string, CycleProgress>
sequenceDiagram
participant ModulesPage
participant moduleService
participant ModuleHandler
participant ModuleService
participant ModuleStore
ModulesPage->>moduleService: listProgress(workspaceSlug, projectId)
moduleService->>ModuleHandler: GET /modules-progress/
ModuleHandler->>ModuleService: ProgressBulk(ctx, slug, projectID, userID)
ModuleService->>ModuleStore: StateDistributionByProject(ctx, projectID)
ModuleStore-->>ModuleService: per-module grouped counts
ModuleService-->>ModuleService: compute total per module
ModuleService-->>ModuleHandler: progress map
ModuleHandler-->>moduleService: 200 JSON progress
moduleService-->>ModulesPage: Record<string, ModuleProgress>
Possibly related PRs
Suggested labels: Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/api/internal/handler/cycle.go (1)
66-92: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
prog == nilbranch is unreachable.
ProgressBulkalways returns a non-nil map fromStateDistributionByProject(initialized viamake(...)), even with zero rows. Soif prog == nilat Line 87 never triggers; an empty progress dict is instead returned via the finalc.JSON(http.StatusOK, prog)at Line 91, which already serializes identically togin.H{}. Harmless but dead code.🤖 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/api/internal/handler/cycle.go` around lines 66 - 92, The nil check in CycleHandler.CyclesProgress is dead code because Cycle.ProgressBulk always returns a non-nil map from StateDistributionByProject, even when empty. Remove the unreachable prog == nil branch and rely on the existing c.JSON(http.StatusOK, prog) path, since an empty map already serializes to an empty JSON object; keep the error handling around middleware.GetUser, uuid.Parse, and ProgressBulk unchanged.apps/web/src/pages/ModulesPage.tsx (1)
272-275: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate progress computation between sort comparator and render helper.
getProgressis defined twice with identical logic (inline in the sort comparator and again as a module-scope function). Consider extracting one shared helper to prevent the two from drifting apart.♻️ Proposed consolidation
+ const getProgress = (mod: ModuleApiResponse) => { + const pr = moduleProgress[mod.id]; + const total = pr?.total ?? mod.issue_count ?? 0; + if (!total) return 0; + return Math.round(((pr?.completed ?? 0) / total) * 100); + }; + const sortedModules = [...filteredModules].sort((a, b) => { - const getProgress = (mod: ModuleApiResponse) => { - const pr = moduleProgress[mod.id]; - const total = pr?.total ?? mod.issue_count ?? 0; - if (!total) return 0; - return Math.round(((pr?.completed ?? 0) / total) * 100); - }; let cmp = 0; switch (sortBy) { ... } return order === 'desc' ? -cmp : cmp; }); - - const getProgress = (mod: ModuleApiResponse) => { - const pr = moduleProgress[mod.id]; - const total = pr?.total ?? mod.issue_count ?? 0; - if (!total) return 0; - return Math.round(((pr?.completed ?? 0) / total) * 100); - };Also applies to: 383-386
🤖 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/ModulesPage.tsx` around lines 272 - 275, The progress calculation for modules is duplicated between the sort comparator and the render-time helper, so consolidate it into a single shared function. Extract the repeated logic from the inline `getProgress` in `ModulesPage.tsx` and the module-scope `getProgress` helper into one reusable function, then use that shared helper in both the sorting code and the UI rendering path. Keep the existing behavior with `moduleProgress`, `mod.issue_count`, and the zero-total guard unchanged while routing both call sites through the same symbol.
🤖 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 `@apps/web/src/pages/CyclesPage.tsx`:
- Around line 398-414: The PROJECT_CYCLES_REFRESH_EVENT refresh path in
CyclesPage currently reloads cycles and issues but leaves cycleProgress stale,
so update the event handler to also call
cycleService.listProgress(workspaceSlug, projectId) and setCycleProgress with
the result (or clear it on failure) using the same workspaceSlug/projectId flow
already used in the useEffect that loads progress.
---
Nitpick comments:
In `@apps/api/internal/handler/cycle.go`:
- Around line 66-92: The nil check in CycleHandler.CyclesProgress is dead code
because Cycle.ProgressBulk always returns a non-nil map from
StateDistributionByProject, even when empty. Remove the unreachable prog == nil
branch and rely on the existing c.JSON(http.StatusOK, prog) path, since an empty
map already serializes to an empty JSON object; keep the error handling around
middleware.GetUser, uuid.Parse, and ProgressBulk unchanged.
In `@apps/web/src/pages/ModulesPage.tsx`:
- Around line 272-275: The progress calculation for modules is duplicated
between the sort comparator and the render-time helper, so consolidate it into a
single shared function. Extract the repeated logic from the inline `getProgress`
in `ModulesPage.tsx` and the module-scope `getProgress` helper into one reusable
function, then use that shared helper in both the sorting code and the UI
rendering path. Keep the existing behavior with `moduleProgress`,
`mod.issue_count`, and the zero-total guard unchanged while routing both call
sites through the same symbol.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: dffd8b89-0ce5-45e2-8891-33aae645c003
📒 Files selected for processing (12)
apps/api/internal/handler/cycle.goapps/api/internal/handler/cycle_test.goapps/api/internal/handler/module.goapps/api/internal/router/router.goapps/api/internal/service/cycle.goapps/api/internal/service/module.goapps/api/internal/store/cycle.goapps/api/internal/store/module.goapps/web/src/pages/CyclesPage.tsxapps/web/src/pages/ModulesPage.tsxapps/web/src/services/cycleService.tsapps/web/src/services/moduleService.ts
|
fix the comments tho |
The PROJECT_CYCLES_REFRESH_EVENT handler reloaded cycles and issues but left cycleProgress stale, so progress could lag after add/remove. Reload it too. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@cavidelizade Can you resolve the merge conflicts please? |
|
Great work on this PR! If the merge conflicts are still pending, I'd be happy to resolve them. Feel free to assign it to me. |
|
@Musa-Qureshi-01 Thanks! |
|
@Musa-Qureshi-01 are you still considering to work on this PR? |
|
Yes I'm working on it, will push next day because I have exams.
…On Thu, 9 Jul, 2026, 1:46 am Martian, ***@***.***> wrote:
*martian56* left a comment (Devlaner/devlane#273)
<#273 (comment)>
@Musa-Qureshi-01 <https://github.com/Musa-Qureshi-01> are you still
considering to work on this PR?
—
Reply to this email directly, view it on GitHub
<#273?email_source=notifications&email_token=BTRRO4NWKQDWVJA6LN6U7ND5D2T3TA5CNFSNUABFM5UWIORPF5TWS5BNNB2WEL2JONZXKZKDN5WW2ZLOOQXTIOJRHA4DGNZQGI2KM4TFMFZW63VHNVSW45DJN5XKKZLWMVXHJLDGN5XXIZLSL5RWY2LDNM#issuecomment-4918837024>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BTRRO4LGXXAEERTOCZP2MDT5D2T3TAVCNFSNUABGKJSXA33TNF2G64TZHMYTCNJYGU2DCNJVGI5US43TOVSTWNBYGEZTOOJTGY2TNILWAI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
@Musa-Qureshi-01 Alright, good luck on your exams 🙏 |
…eal-progress # Conflicts: # apps/api/internal/handler/cycle_test.go
Feature summary
Cycle and module list pages now show real completion progress computed from work-item states, instead of a faked 0%/100% from the cycle/module status.
Linked issues / discussion
Closes #185
User-facing behavior
On the Cycles and Modules list pages, each item's progress circle/bar reflects completed / total work items (by state group). The modules "sort by progress" now sorts on the real value.
What changed
API (
apps/api/)store/cycle.go,store/module.go:StateDistributionByProjectreturns per-cycle / per-module issue counts grouped by state group in one query.service/cycle.go,service/module.go:ProgressBulkadds atotaland returns the map keyed by id.handler/cycle.go,handler/module.go+ routes:GET .../cycles-progress/andGET .../modules-progress/, mirroring the existingepics-progressendpoint.UI (
apps/web/)cycleService.listProgress/moduleService.listProgress.CyclesPage/ModulesPage: fetch the bulk progress and computegetProgressfromcompleted / total.Database
cycle_issues/module_issues+states).Why this design
Mirrors the proven
epics-progressbulk pattern so the list pages make one request instead of one per item, and reuses the same state-group distribution shape the epic progress bar already consumes.Test plan
go vet ./...and fullgo test ./...green (testcontainers).TestCycle_ProgressBulk: a cycle with two backlog issues reportstotal: 2,completed: 0via the endpoint.npm run typecheck,npm run lint,npm run buildgreen.Out of scope (follow-ups)
Rollout notes
None.
AI assistance
Claude Code— and AI-assisted commits include aCo-Authored-By:trailerChecklist
--no-verifybypassSummary by CodeRabbit
{completed, total}values with fallbacks, rather than status-derived estimates.