feat(csv): added the csv export to the analytics page#393
Conversation
… line in one of the project GET
…t present in the database.. :(
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughWalkthroughAdds server-side workspace and project analytics with authorization, partial aggregation, and CSV export endpoints. The web analytics page now consumes backend analytics and provides workspace and project exports. Issue-list loading, web package versions, and module route ordering are also updated. ChangesAnalytics feature
Ancillary application updates
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Browser
participant AnalyticsAPI
participant AnalyticsService
participant AnalyticsStore
Browser->>AnalyticsAPI: Request workspace or project analytics
AnalyticsAPI->>AnalyticsService: Pass authenticated analytics request
AnalyticsService->>AnalyticsStore: Validate access and query grouped counts
AnalyticsStore-->>AnalyticsService: Return analytics rows
AnalyticsService-->>AnalyticsAPI: Return analytics response
AnalyticsAPI-->>Browser: Render analytics data
Browser->>AnalyticsAPI: Request CSV export
AnalyticsAPI->>AnalyticsService: Pass authenticated export request
AnalyticsService->>AnalyticsStore: Query authorized issue rows
AnalyticsStore-->>AnalyticsAPI: Return export rows
AnalyticsAPI-->>Browser: Download CSV attachment
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 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 |
LADsy8
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
apps/api/internal/service/analytics.go (1)
110-115: 🎯 Functional Correctness | 🔵 Trivial | 💤 Low valueInconsistent priority-error handling.
In
GetProjectAnalytics, a failure fromGetProjectPriorityAnalyticsis silently swallowed (nos.log.Warn, nopartial_error), whereas the workspace path treats a priority failure as fatal (return nil, err). Consider logging a warning here so a failed priority aggregate is observable rather than silently returning an empty map.🤖 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/service/analytics.go` around lines 110 - 115, Update GetProjectAnalytics so errors from GetProjectPriorityAnalytics are not silently ignored: log a warning through s.log.Warn while preserving the existing partial analytics response and empty priority map behavior. Keep successful priority aggregation unchanged.apps/web/src/pages/AnalyticsWorkItemsPage.tsx (1)
185-192: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueOptional: surface
partial_error/warningsto the user.The response type declares
partial_error/warnings, and the backend sets them when assignee/label/trend aggregates fail, but they are never read here. When only secondary aggregates fail the page renders silently with missing/empty sections (e.g. an empty trend chart) and no indication anything is degraded. Consider showing thewarningswhenpartial_erroris true.🤖 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/AnalyticsWorkItemsPage.tsx` around lines 185 - 192, Update the fulfilled analytics handling in the page’s analytics-loading flow to inspect the response’s partial_error and warnings fields. When partial_error is true, surface the available warnings to the user while still setting the analytics result; preserve the existing full-request failure handling for rejected responses.
🤖 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/api/internal/handler/analytics.go`:
- Around line 57-77: Replace the raw *gorm.DB dependency and direct queries in
AnalyticsHandler.GetWorkspaceAnalytics and the other analytics/export handlers
with an AnalyticsService dependency; pass middleware.GetUser(c).ID to service
methods so their existing workspace/project authorization guards run before
aggregation. In apps/api/internal/handler/analytics.go lines 57-77, route the
workspace analytics request through AnalyticsService; in
apps/api/internal/router/router.go lines 248-251, construct
service.NewAnalyticsService(...) and inject it into AnalyticsHandler instead of
cfg.DB.
In `@apps/api/internal/service/analytics.go`:
- Around line 1-6: Add the required package declaration and import block before
analyticsService, importing the packages needed by the file, including fmt,
log/slog, and gorm.io/gorm. Remove the trailing whitespace from the db field and
run gofmt so analytics.go parses and builds cleanly.
- Around line 17-45: Update checkWorkspaceAccess and checkProjectAccess so their
workspace_members joins or conditions require workspace_members.deleted_at IS
NULL. Preserve the existing authorization and project/workspace deletion checks
while ensuring soft-deleted memberships cannot satisfy either analytics access
check.
In `@apps/web/package.json`:
- Line 47: Remove the "node" dependency from apps/web/package.json and declare
the supported Node version through the package's engines field or the
repository's existing toolchain/CI configuration, preserving the intended
version range without installing a local Node binary.
---
Nitpick comments:
In `@apps/api/internal/service/analytics.go`:
- Around line 110-115: Update GetProjectAnalytics so errors from
GetProjectPriorityAnalytics are not silently ignored: log a warning through
s.log.Warn while preserving the existing partial analytics response and empty
priority map behavior. Keep successful priority aggregation unchanged.
In `@apps/web/src/pages/AnalyticsWorkItemsPage.tsx`:
- Around line 185-192: Update the fulfilled analytics handling in the page’s
analytics-loading flow to inspect the response’s partial_error and warnings
fields. When partial_error is true, surface the available warnings to the user
while still setting the analytics result; preserve the existing full-request
failure handling for rejected responses.
🪄 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: 39ffc658-20e4-4a45-9662-d1b164012b20
⛔ Files ignored due to path filters (1)
apps/web/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (6)
apps/api/internal/handler/analytics.goapps/api/internal/router/router.goapps/api/internal/service/analytics.goapps/web/package.jsonapps/web/src/pages/AnalyticsWorkItemsPage.tsxapps/web/src/pages/IssueListPage.tsx
| "i18next": "^26.3.6", | ||
| "i18next-browser-languagedetector": "^8.2.1", | ||
| "lucide-react": "^0.563.0", | ||
| "node": "^26.5.0", |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify whether the repository intentionally relies on a locally installed Node binary.
fd -H -t f . | xargs rg -n '"node"|engines|nvm|fnm|volta|asdf|node_modules/\.bin|setup-node'Repository: Devlaner/devlane
Length of output: 46356
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo '--- apps/web/package.json ---'
cat -n apps/web/package.json | sed -n '1,140p'
echo
echo '--- apps/web/package-lock.json (around node dependency) ---'
python3 - <<'PY'
from pathlib import Path
p = Path('apps/web/package-lock.json')
text = p.read_text()
for needle in ['"node": "^26.5.0"', '"node_modules/node"', '"node": "bin/node"', '"bin/node"', '"engines"']:
idx = text.find(needle)
print(f'\nNEEDLE: {needle} -> {idx}')
if idx != -1:
start = max(0, idx-300)
end = min(len(text), idx+700)
snippet = text[start:end]
print(snippet)
PY
echo
echo '--- repository root package.json (if present) ---'
if [ -f package.json ]; then cat -n package.json | sed -n '1,220p'; else echo 'no root package.json'; fi
echo
echo '--- web package scripts/engines/private ---'
python3 - <<'PY'
import json
from pathlib import Path
data = json.loads(Path('apps/web/package.json').read_text())
for key in ['scripts','engines','dependencies','devDependencies','optionalDependencies','peerDependencies']:
if key in data:
print(f'\n[{key}]')
if isinstance(data[key], dict):
for k,v in data[key].items():
if k == 'node' or k.startswith('`@tiptap/`') or k in ('vite','typescript','react','react-dom'):
print(f'{k}: {v}')
else:
print(data[key])
PYRepository: Devlaner/devlane
Length of output: 10010
Do not add node as a web-app dependency. This pulls in the node package and its install script, which downloads a local Node binary and can override the system runtime during installs. Declare the supported Node range via engines or the repo’s toolchain/CI instead.
🤖 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/package.json` at line 47, Remove the "node" dependency from
apps/web/package.json and declare the supported Node version through the
package's engines field or the repository's existing toolchain/CI configuration,
preserving the intended version range without installing a local Node binary.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/api/internal/service/analytics.go (2)
114-147: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
GetProjectAnalyticssilently drops priority/assignee/label failures — no error, no log, unlike the workspace twin.In
GetWorkspaceAnalytics, a priority-query failure is fatal (returned as an error) and assignee/label failures are at least logged vias.log.Warn(...). Here, priority/assignee/label errors are swallowed with neither a returned error nor any log call — callers get a200 OKwith silently-empty maps and there is zero observability into why. This is an inconsistent criticality classification between two nearly identical functions.🔧 Align logging/error semantics with GetWorkspaceAnalytics
byPriority := make(map[string]int64) if priorityResults, err := s.store.GetProjectPriorityAnalytics(ctx, projectID); err == nil { for _, r := range priorityResults { byPriority[r.Priority] = r.Count } + } else if s.log != nil { + s.log.Warn("failed to fetch project priority analytics", "error", err, "project_id", projectID) } byAssignee := make(map[string]int64) if assigneeResults, err := s.store.GetProjectAssigneeAnalytics(ctx, projectID); err == nil { for _, r := range assigneeResults { byAssignee[r.Email] = r.Count } + } else if s.log != nil { + s.log.Warn("failed to fetch project assignee analytics", "error", err, "project_id", projectID) } byLabel := make(map[string]int64) if labelResults, err := s.store.GetProjectLabelAnalytics(ctx, projectID); err == nil { for _, r := range labelResults { byLabel[r.Label] = r.Count } + } else if s.log != nil { + s.log.Warn("failed to fetch project label analytics", "error", err, "project_id", projectID) }🤖 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/service/analytics.go` around lines 114 - 147, Update GetProjectAnalytics to match GetWorkspaceAnalytics error semantics: return a wrapped error when GetProjectPriorityAnalytics fails, and log warnings through s.log.Warn for GetProjectAssigneeAnalytics and GetProjectLabelAnalytics failures while preserving successful result population.
106-111: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winDefine
AnalyticsResponsewith snake_case JSON tags.The frontend expects
by_state,by_priority,trend,partial_error, andwarnings, butapps/api/internal/service/analytics.goreturnsAnalyticsResponse{ByState, ByPriority, ByAssignee, ByLabel}without the struct definition. Add the Go response type withjson:"by_state"/json:"by_priority"tags so/analyticsdoes not emitByState/ByPriorityto the frontend.Also applies to the project analytics return at lines 149-154.
🤖 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/service/analytics.go` around lines 106 - 111, Define the AnalyticsResponse type used by the analytics service with snake_case JSON tags for by_state, by_priority, trend, partial_error, and warnings, and include the existing assignee and label fields as appropriate. Ensure both the main analytics return and the project analytics return serialize through this type so the frontend receives the expected property names instead of Go field names.
🧹 Nitpick comments (3)
apps/api/internal/handler/analytics.go (2)
136-148: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winCSV write/flush errors are discarded in both export handlers.
_ = writer.Write(...)throughout, andwriter.Flush()'s subsequent error state is never checked viawriter.Error(). A mid-stream write failure would go completely unnoticed.🔧 Check writer.Error() after Flush
writer.Flush() + if err := writer.Error(); err != nil { + h.Log.Error("csv write error during export", "error", err) + } }Also applies to: 186-197
🤖 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/analytics.go` around lines 136 - 148, Update both CSV export handlers, including the shown loop and the corresponding block around the second export, to stop discarding csv.Writer errors: check each Write result and check writer.Error() after Flush, propagating or returning the error through the handler’s existing error path.
34-46: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnused
parseParamUUIDhelper; both project handlers duplicate its logic inline.
parseParamUUIDis defined but never called.GetProjectAnalyticsandExportProjectCSVboth re-implement the sameprojectId/projectIDfallback +uuid.Parsepattern instead of reusing it.♻️ Reuse the helper
func (h *AnalyticsHandler) GetProjectAnalytics(c *gin.Context) { - projectParam := c.Param("projectId") - if projectParam == "" { - projectParam = c.Param("projectID") - } - projectID, err := uuid.Parse(projectParam) - if err != nil { - c.JSON(http.StatusBadRequest, gin.H{"error": "invalid project ID"}) + projectID, ok := parseProjectID(c) + if !ok { return }Also applies to: 74-83, 150-159
🤖 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/analytics.go` around lines 34 - 46, Use the existing parseParamUUID helper in GetProjectAnalytics and ExportProjectCSV instead of duplicating projectId/projectID lookup and UUID parsing inline. Ensure both handlers pass the appropriate parameter key or preserve the fallback behavior through the helper, remove the redundant parsing logic, and keep the existing bad-request responses and early returns.apps/api/internal/store/analytics.go (1)
125-146: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftExport queries load entire result set into memory with no bound.
GetWorkspaceIssuesForExport/GetProjectIssuesForExportScanthe full issue set for a workspace/project with noLimit/pagination/streaming. For large workspaces this directly undercuts the PR's stated scalability goal and risks high memory usage / long-held DB connections on export.♻️ Consider streaming rows instead of buffering the full slice
-func (s *AnalyticsStore) GetWorkspaceIssuesForExport(ctx context.Context, slug string) ([]model.WorkspaceIssueExport, error) { - var exports []model.WorkspaceIssueExport - err := s.db.WithContext(ctx). - Table("issues"). - Select("issues.id, issues.name, issues.state, issues.priority"). - Joins("JOIN workspaces ON issues.workspace_id = workspaces.id"). - Where("workspaces.slug = ? AND issues.deleted_at IS NULL AND workspaces.deleted_at IS NULL", slug). - Scan(&exports).Error - - return exports, err -} +// Consider exposing a Rows()-based iterator (or FindInBatches) so the +// handler can stream CSV rows to the client instead of loading the +// entire workspace's issues into memory at once.🤖 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/store/analytics.go` around lines 125 - 146, Update GetWorkspaceIssuesForExport and GetProjectIssuesForExport so exports do not unboundedly Scan all matching issues into memory; implement bounded pagination or streaming row processing while preserving the existing filters and selected export fields, and ensure resources are properly released.
🤖 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/api/internal/handler/analytics.go`:
- Around line 109-148: Sanitize or escape the user-controlled slug before
interpolating it into the Content-Disposition filename in ExportWorkspaceCSV.
Preserve the existing filename format while ensuring embedded quotes or other
header-sensitive characters cannot break the quoted header value.
In `@apps/api/internal/service/analytics.go`:
- Around line 41-63: Update ensureWorkspaceAccess and ensureProjectAccess to
return underlying errors from GetBySlug, GetByID, and IsMember instead of
classifying them as ErrAnalyticsNotFound or ErrAnalyticsForbidden. Preserve
ErrAnalyticsNotFound only for nil records and ErrAnalyticsForbidden only when
IsMember succeeds with false, so genuine infrastructure failures propagate to
the handler’s 500 path.
In `@apps/api/internal/store/analytics.go`:
- Around line 19-72: Update the workspace-scoped analytics methods
GetWorkspaceStateAnalytics, GetWorkspacePriorityAnalytics,
GetWorkspaceAssigneeAnalytics, and GetWorkspaceLabelAnalytics to join the
relevant projects relation and filter projects.deleted_at IS NULL alongside the
existing issue and workspace soft-delete conditions, ensuring retained issues
from soft-deleted projects are excluded.
---
Outside diff comments:
In `@apps/api/internal/service/analytics.go`:
- Around line 114-147: Update GetProjectAnalytics to match GetWorkspaceAnalytics
error semantics: return a wrapped error when GetProjectPriorityAnalytics fails,
and log warnings through s.log.Warn for GetProjectAssigneeAnalytics and
GetProjectLabelAnalytics failures while preserving successful result population.
- Around line 106-111: Define the AnalyticsResponse type used by the analytics
service with snake_case JSON tags for by_state, by_priority, trend,
partial_error, and warnings, and include the existing assignee and label fields
as appropriate. Ensure both the main analytics return and the project analytics
return serialize through this type so the frontend receives the expected
property names instead of Go field names.
---
Nitpick comments:
In `@apps/api/internal/handler/analytics.go`:
- Around line 136-148: Update both CSV export handlers, including the shown loop
and the corresponding block around the second export, to stop discarding
csv.Writer errors: check each Write result and check writer.Error() after Flush,
propagating or returning the error through the handler’s existing error path.
- Around line 34-46: Use the existing parseParamUUID helper in
GetProjectAnalytics and ExportProjectCSV instead of duplicating
projectId/projectID lookup and UUID parsing inline. Ensure both handlers pass
the appropriate parameter key or preserve the fallback behavior through the
helper, remove the redundant parsing logic, and keep the existing bad-request
responses and early returns.
In `@apps/api/internal/store/analytics.go`:
- Around line 125-146: Update GetWorkspaceIssuesForExport and
GetProjectIssuesForExport so exports do not unboundedly Scan all matching issues
into memory; implement bounded pagination or streaming row processing while
preserving the existing filters and selected export fields, and ensure resources
are properly released.
🪄 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: fb81f93e-939d-4fa0-9e9c-8b05a2f5a379
📒 Files selected for processing (5)
apps/api/internal/handler/analytics.goapps/api/internal/model/analytics.goapps/api/internal/router/router.goapps/api/internal/service/analytics.goapps/api/internal/store/analytics.go
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/api/internal/router/router.go
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Summary
This PR implements the server-side analytics endpoints and the CSV export capability for workspaces and projects, resolving the client-side scalability issues and wiring up the "Export as CSV" button in the UI.
Linked issues
Closes #204
Type of change
feat:) — user-visible new capabilitySurface
apps/api/)apps/web/)What changed
API Backend (
apps/api/)Frontend UI (
apps/web/)AnalyticsOverviewPageandAnalyticsWorkItemsPageto fetch metrics from the new backend endpoints instead of doing expensive client-side aggregation.Why this approach
Processing aggregations on the server ensures the platform scales efficiently as workspaces grow to thousands of issues. Using optimized database joins prevents the "N+1 query" performance pitfall.
Database / migrations
No schema migrations required.
Breaking changes
Test plan
npm run validatefrom the repository root passes cleanly (TypeScript check + golang test suites).go run ./cmd/api seed.Screenshots / recordings (UI changes)
AI assistance
Gemini— and AI-assisted commits include aCo-Authored-By:trailerChecklist
feat(api/ui): implement server-side analytics and working CSV export #204)--no-verifybypass).envvalues committedReview Notes & Future Improvements
While testing locally, I noticed a few behaviors and structural details to highlight:
Percentage Calculations & Charts:
doneandbacklogare visible in the data, they do not render properly inside the main chart. This might be an existing issue with how state types/cycles are mapped, which we can refine later.Database Schema Details (Assignees & Labels):
assigneeorlabelcolumns on theissuestable, and because setting up the many-to-many joins (issue_assigneesandissue_labels) was causing SQL errors, I skipped adding assignees and labels to the CSV columns for now to keep the code stable. We can add these relations in a follow-up PR once we iron out the database join logicSummary by CodeRabbit