Skip to content

feat(org-lens): org lens project detail page UI (LFXV2-1885)#1028

Open
danoqualls wants to merge 58 commits into
mainfrom
feat/LFXV2-1885
Open

feat(org-lens): org lens project detail page UI (LFXV2-1885)#1028
danoqualls wants to merge 58 commits into
mainfrom
feat/LFXV2-1885

Conversation

@danoqualls

@danoqualls danoqualls commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Business case: company can view their involvement in a project in a detailed way that adds up to influence in the project and compares influence to other companies. UI only with demo data. Includes both influence detail tab and leaderboards tab.

This pull request introduces the Org Lens Project Detail sub-page (LFXV2-1885) to the application, enabling users to view detailed analytics and leaderboard data for individual projects within an organization. It includes the new route and navigation entry, a complete UI with tabbed navigation, metric and time range toggles, demo data integration, and comprehensive E2E tests. Additionally, it restores interactivity for project rows in the Org Overview, allowing users to drill down into project details.

Org Lens Project Detail Feature Implementation:

  • Added the /org/projects/:projectSlug route, navigation entry, and associated metadata, enabling direct navigation to project detail pages. [1] [2]
  • Implemented the OrgProjectDetailService to provide demo data for the project detail page, designed for easy replacement with a real backend API in the future.
  • Created the OrgProjectDetailTabBarComponent for tabbed navigation, including keyboard accessibility, metric toggles, and time range selection. [1] [2]
  • Added comprehensive Playwright E2E tests covering rendering, tab navigation, leaderboard interactions, URL parameter persistence, and 404 handling for unknown slugs.

Org Overview Interactivity Restoration:

  • Restored click and keyboard handlers to project rows in the Org Overview, allowing users to navigate to the new project detail page and emitting telemetry events. [1] [2] [3]

Supporting and Miscellaneous Updates:

  • Exported new interfaces for the Org Lens Project Detail feature in the shared package.
  • Minor improvement to the Valkey cache service for more robust JSON retrieval.

Summary

  • Adds the Org Lens Project Detail sub-page at /org/projects/:projectSlug, opened from the Overview's Foundations & Projects table
  • Backed by a TypeScript demo-data service (org-lens-project-detail.demo-data.ts) via a new service + controller + route — demo company data only; real Snowflake integration is a separate story. Note: there is no static JSON fixture; all demo payloads are generated server-side in TypeScript.
  • Two tabs — Our Influence (default) and Leaderboards — with tab/metric/range state persisted as query params (?tab=, ?metric=, ?range=)

Our Influence tab:

  • Hero block (logo, name, description, LFX Insights link, First commit, Software value, Health badge, Foundation pill)
  • Technical Influence section (Maintainers, Contributors, Commit Activities, Pull Requests, Avg Merge Time) — horizontal carousel with scrolling chevrons; each card opens a detail drawer
  • Ecosystem Influence section (Collaboration Activity, Meeting Attendance, Board Members, Committee Members, Event Attendance, Event Speakers, Training Completions, Certified Individuals, Mailing List Posts)
  • Per-card influence chart (area / bar / line variants with custom external tooltip)
  • Card detail drawer: definition table + card-specific data table

Leaderboards tab:

  • Side-by-side Technical / Ecosystem leaderboard tables with search, PrimeNG paginator, viewing-org row highlighted, band tags (Leading / Contributing / Participating / Non-LF)
  • Metric toggle (Calculated Influence / Activity Count) + time range pill group (1y / 2y / All time) in the shared tab bar
  • Stacked 100% area trend chart showing top-10 companies' share over time with deterministic demo trajectories

Shell:

  • Page-state machine (loading / error / notFound / ready) with <p-skeleton> loading states and lfx-empty-state error/not-found panels
  • Breadcrumb: Projects → <project name>
  • All sections use data-testid attributes; E2E coverage added

PR size rationale

This PR is ~2,500 lines. The branch builds the complete Project Detail page end-to-end (scaffold → hero → influence cards → trend chart → leaderboards → e2e → polish). The intermediate commits are not independently shippable — routing and the data layer land without visible UI until the component is complete. Splitting at a natural seam (e.g., scaffold-only or data-layer-only) would merge dead routes that don't render. The full page is the atomic unit of work here, consistent with how the sibling Membership Detail page (PR #921) was delivered.

Test plan

  • Start dev server with org lens flag enabled; open /org/overview, expand a foundation, click an LF project row → lands on /org/projects/<slug>
  • Hero renders all tiles (name, description, LFX Insights link, First commit, Software value, Health badge, Foundation)
  • Tab switch updates ?tab= and survives page reload
  • Technical + Ecosystem cards render counts, trendlines; clicking a card opens the detail drawer with definition + data table
  • Influence Trend stacked area chart renders with correct ordering (most influential at bottom); time range pills update the dataset window
  • Leaderboard tables rank orgs, pin the viewing-org row (highlighted), paginator works
  • Metric toggle switches between Calculated Influence (band tag) and Activity Count (numeric)
  • Time range pills update ?range= query param and persist on reload
  • Unknown slug shows the not-found panel; Back to Projects navigates to /org/projects
  • Playwright E2E suite passes: yarn e2e

🤖 Generated with Claude Code

Add the Org Lens Project Detail sub-page (LFXV2-1885) shell, opened from
the Overview project rows via /org/projects/:projectSlug. Includes the
self-contained payload contracts, a demo-data seam (OrgLensProjectDetailService
returns fixtures; an unknown slug yields the not-found state), the page-state
machine, and the URL-persisted (?tab=) two-tab strip. Hero, metric cards,
trend chart and leaderboard land in follow-up commits.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Render the breadcrumb (Projects / <Foundation> / <Project>) and the two-column
hero: logo (initials fallback), name, description, Source + View on LFX Insights
links, and the stat tiles (first commit, software value with the project-vs-org
distinction note, health badge, foundation pill, last updated).

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Render the Our Influence tab card groups: four Technical cards (Maintainers,
Contributors, Commit Activities, Pull Requests) via lfx-metric-card with the
org count, '% of all' subtitle, 12-month sparkline and freshness label; and four
Ecosystem count cards (Collaboration, Meeting Attendance, Board Members,
Committee Members) with per-metric empty guidance copy.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Render the combined Influence Trend line chart in the Our Influence tab with
Combined / Technical / Ecosystem overlays (legend toggles each line) and an
index-mode hover tooltip. Falls back to 'Not enough history yet' when fewer
than three months of data are available.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Render the project leaderboard ranking all contributing orgs by Rank / Org /
Score / Trend (1y sparkline + delta) / Band. Score-type (Combined/Technical/
Ecosystem) and metric (Calculated Influence/Activity Count) toggles persist in
?score=/?metric=; Activity Count mode ranks by raw count and hides the Trend +
Band columns. The viewing-org row is highlighted and pinned when it falls
outside the visible top-N, and 'Show more' pages 10 at a time up to 100.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Add Playwright coverage for the Project Detail page: testid resolution
(breadcrumb, hero, card groups, trend), tab switching via click/keyboard with
?tab= persistence, leaderboard ranking with the viewing-org pin + Show more,
score/metric URL persistence, Activity Count hiding Trend + Band, and the
not-found panel for an unknown slug. Also normalizes formatting.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Apply the wireframe's visual language through existing LFX components rather than
hand-rolled markup: render the health badge and leaderboard band chips with the
shared lfx-tag wrapper (severity-mapped), rename the card-group headings to 'Our
Technical/Ecosystem Influence', and add the Project/Foundation scope label to each
Ecosystem card. Ticket structure (trend chart, single leaderboard with toggles,
viewing-org pin, Show more) is unchanged.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Restructure the Leaderboards tab to match the prototype: side-by-side Technical
and Ecosystem Influence boards (the established training/events pattern), each
with a Last-12-months header, per-board search, top-10 rows (# / Organization /
Influence Score band chip), and the viewing-org row pinned below with a blue
rank badge when outside the top 10. Replaces the single combined table; drops the
Combined/Technical/Ecosystem segmented control (the two boards cover technical +
ecosystem), the trend column, and Show more. The Calculated Influence / Activity
Count toggle persists in ?metric= and swaps the score column. E2e updated.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Make the Ecosystem cards match the Technical cards: one shared card style with a
title, dual-line trendline (metric line + faint baseline), and a descriptive
sentence with the stat bolded. Ecosystem cards gain a project/foundation scope
label header (project name for Collaboration + Meetings; foundation name for
Board + Committee members) and a 'No data' state when there's no series. Add the
org's influence-level band badge beside each section heading. Drop the per-card
freshness label and the hero 'Last updated' tile.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Match the My Org project hero: fold the source link into the description as an
inline 'Source: LFX Insights' link, label the tiles First commit / Software value
/ Health score, and use a two-segment 'Projects / <Project>' breadcrumb. Add the
Foundation tile that the My Org design omits. Update the Kubernetes demo values to
the design (first commit July 2013, software value $6B) and full-month formatting.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Show the bare project name (e.g. 'Kubernetes') on project-level ecosystem cards
instead of 'Kubernetes Project'; foundation-level cards keep the foundation name.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Match the My Org design's complete Our Influence set. Add Avg Time to Merge PRs
(technical) and Event Attendance, Event Speakers, Event Sponsorships, Meetup
Attendance, and Certified Individuals (ecosystem). Unify the two card types into a
single OrgLensProjectInfluenceCard whose descriptive caption + scope label are
built in the demo data (where the foundation name is known), so the component just
renders prefix/emphasis/suffix + trendline. Project-level cards (collaboration,
meetings) label with the project name; the rest with the foundation name.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
- Add orgName computed signal (falls back to 'Acme Corp' for demo)
- Show org-name chip above project title in hero
- Remove orgUid filter so page loads without an org in context;
  use 'demo-org' / 'Acme Corp' fallbacks for the demo-data phase

LFXV2-1885

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
- Horizontal-scroll single row for Technical and Ecosystem metric cards
- Section-header band chip includes signal-strength SVG bars icon
- Remove "Get involved with this project" link from card section
- Move Influence Trend chart to Leaderboards tab beneath the boards
- Dual-line sparklines: blue = company data, grey = project average
- Hover tooltip shows both "Your company" and "Project average" labels
- Increase sparkline height h-14→h-20 so tooltip rows aren't clipped
- Fix valkey TS2345 type cast and null-check on cache reads
- Uncomment Projects nav entry and wire Overview project-row click

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Replace the built-in Chart.js canvas tooltip with an external HTML
overlay (data-lfx-tip) so it is not constrained by the 80px canvas
height. The overlay renders at full card width with readable 13px text,
colored dot indicators, and bold values. Pointer-events-none lets the
canvas receive hover events through the overlay.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
- Remove org-context chip from project hero (project page, not org page)
- Show real project logo (Kubernetes/Argo CNCF artwork SVGs)
- Fix hero stats layout: flex-wrap replaces compressed lg:grid-cols-4
- Fix band chip SVG: 15px viewBox centers bars flush with label text
- Add arrow-button carousels (<  >) to Technical and Ecosystem card rows

Signed-off-by: Daniel Qualls <dqualls@linuxfoundation.org>
Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
- Tooltip: switch from absolute card-overlay to fixed viewport-positioned
  popup using getBoundingClientRect + caretX/caretY; tooltip now floats
  above the cursor rather than replacing card content
- Hero: place logo and project name on the same row; description spans
  the full container width below instead of being constrained to the
  right of the name

Signed-off-by: Daniel Qualls <dqualls@linuxfoundation.org>
Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Arrows were flanking the card track at mid-height; move them to the
far-right of the section title row to match the app-wide pattern used
in other card carousels (e.g. Recent Progress on the overview page).

Signed-off-by: Daniel Qualls <dqualls@linuxfoundation.org>
Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
…ibility

- Cards: w-56 → w-72, rounded-xl, small muted title, h-16 sparkline,
  border-separated stat block with large metric number + short label
- Arrows: hide individually based on scroll position; both hidden when
  all cards fit without scrolling (scrollWidth ≤ clientWidth)
- Scroll step updated to 304px (w-72 + gap-4) to advance one full slot
- statLabel derived from caption suffix (trimmed) for concise display

Signed-off-by: Daniel Qualls <dqualls@linuxfoundation.org>
Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
… propagation

Use area/bar/line chart variants per metric key (pull-requests and
meeting-attendance as bars; avg-time-to-merge and board-members as plain
lines; all others filled area) to match production. Fix sparkline options
not applying by moving lineCardOptions/barCardOptions to stable class-level
properties instead of per-card context, sidestepping an Angular 20 zoneless
signal-input propagation issue with object properties via ng-template context.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Move @ViewChild decorated fields before inject() fields to satisfy
member-ordering (decorated-field before field). Move scrollCards and
onTrackScroll protected methods before the first private method block.
Replace nested ternary for logoUrl in demo-data with a lookup map.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Replace conditional rounded-full h-7 w-7 arrows with always-visible
h-8 w-8 rounded square buttons matching the exact production class set:
border-gray-300 bg-white text-gray-600 hover:bg-gray-100 hover:border-gray-400.
Both < and > arrows now always render regardless of scroll position.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Closes a missing closing quote in the inline-style string generated by
buildExternalTooltipFn that caused the entire subsequent HTML to be
swallowed as a CSS attribute value, producing garbled tooltip content.
Also sets pointHoverRadius to 0 on both sparkline datasets so Chart.js
does not draw hollow ring markers on hover.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
…t detail

Replace underline-style tab buttons with rounded pill container matching
app.lfx.dev convention. Add a flex-1 h-px divider line in all three
section headers (Technical, Ecosystem, Influence Trend) so the title
extends to the right edge per the app-wide section heading pattern.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
…down

Clicking any influence card opens a right-side p-drawer showing the full
12-month trend chart (axes + legend), a primary stat summary, and a
monthly breakdown table with Your org vs Project average and month-over-
month delta indicators. Introduces InfluenceCardVm (module-local) and
OrgLensInfluenceCardDetailRow (shared interface), per-card detailChartOptions
with visible axes, and fixes the avg-merge-time chart variant key mismatch.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
…FXV2-1885

Remove the repeated 12-month chart and generic monthly breakdown table from the
influence card detail drawer. Replace with a definition table (Definition / Total
count for this project / Data source) and a card-specific data table whose columns
match app.lfx.dev per-card conventions (Maintainers, Contributors, Commit Activities,
Pull Requests, Avg Merge Time, Collaboration Activity, Meeting Attendance, Board
Members, Committee Members, plus the remaining ecosystem cards). Add
OrgLensCardDetailSection / OrgLensCardDetailCell / OrgLensCardDetailRow / OrgLensCardDefinition
to shared interfaces; generate demo rows for all 14 card keys in the demo-data
helper; wire via cardDetail computed in the component.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
PrimeNG content projection requires ng-template#header to be a static
direct child of p-drawer. When nested inside @if, the template is not
found during drawer initialization, preventing the panel from opening.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Replace title attribute with pTooltip to match the reference page
tooltip text: "Based on Constructive Cost Model (COCOMO)".

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Replace client-side top-N slice + pinned-row pattern with lfx-table
paginator (10/25/50 rows, "Showing X to Y of Z" report). Inline both
Technical and Ecosystem boards directly in the tab panel to avoid a
ContentChild resolution failure that occurs when lfx-table is placed
inside an ng-template rendered via ngTemplateOutlet.

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Dano Qualls <mynameisdano@gmail.com>
@danoqualls

Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jun 25, 2026

Copy link
Copy Markdown
Contributor
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.

Comment thread packages/shared/src/interfaces/org-lens-project-detail.interface.ts Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
apps/lfx-one/src/server/controllers/org-lens-project-detail.controller.ts (1)

34-51: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Optional: also set Cache-Control: no-store on the 404 path.

The success path sets no-store, but the 404 response omits it. For consistent cache behavior on unknown slugs, consider setting the header before responding in both branches.

🤖 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/lfx-one/src/server/controllers/org-lens-project-detail.controller.ts`
around lines 34 - 51, The 404 branch in org-lens-project-detail.controller.ts is
missing the same Cache-Control behavior as the success path. Update the
get_org_lens_project_detail handler so the no-store header is set before both
the 404 response and the normal response, using the existing res.setHeader call
in the same controller to keep cache behavior consistent for unknown project
slugs.
🤖 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/lfx-one/src/app/modules/dashboards/org/org-project-detail/org-project-detail.component.ts`:
- Line 1: The file has formatting-only issues flagged by yarn format:check, so
update the org-project-detail.component.ts content to match Prettier’s style.
Run yarn format (prettier --write) on the affected file and ensure the
formatting is consistent with the rest of the codebase.

In `@apps/lfx-one/src/app/shared/services/org-lens-project-detail.service.ts`:
- Around line 22-27: The getProjectDetail method in OrgLensProjectDetailService
needs formatting cleanup to satisfy the yarn format:check pipeline. Run Prettier
on this file and keep the existing logic intact, making sure the Observable
pipe/catchError block is formatted according to project style.

---

Nitpick comments:
In `@apps/lfx-one/src/server/controllers/org-lens-project-detail.controller.ts`:
- Around line 34-51: The 404 branch in org-lens-project-detail.controller.ts is
missing the same Cache-Control behavior as the success path. Update the
get_org_lens_project_detail handler so the no-store header is set before both
the 404 response and the normal response, using the existing res.setHeader call
in the same controller to keep cache behavior consistent for unknown project
slugs.
🪄 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

Run ID: 97699976-3f87-4001-a83a-0855ebdbcc45

📥 Commits

Reviewing files that changed from the base of the PR and between f64638c and f5952d2.

📒 Files selected for processing (16)
  • apps/lfx-one/e2e/org-project-detail.spec.ts
  • apps/lfx-one/src/app/app.routes.ts
  • apps/lfx-one/src/app/modules/dashboards/org/components/org-overview-foundations-and-projects/org-overview-foundations-and-projects.component.html
  • apps/lfx-one/src/app/modules/dashboards/org/components/org-overview-foundations-and-projects/org-overview-foundations-and-projects.component.ts
  • apps/lfx-one/src/app/modules/dashboards/org/org-project-detail/org-project-detail-tab-bar.component.html
  • apps/lfx-one/src/app/modules/dashboards/org/org-project-detail/org-project-detail-tab-bar.component.ts
  • apps/lfx-one/src/app/modules/dashboards/org/org-project-detail/org-project-detail.component.html
  • apps/lfx-one/src/app/modules/dashboards/org/org-project-detail/org-project-detail.component.ts
  • apps/lfx-one/src/app/shared/services/org-lens-project-detail.service.ts
  • apps/lfx-one/src/server/controllers/org-lens-project-detail.controller.ts
  • apps/lfx-one/src/server/routes/orgs.route.ts
  • apps/lfx-one/src/server/services/org-lens-project-detail.demo-data.ts
  • apps/lfx-one/src/server/services/org-lens-project-detail.service.ts
  • apps/lfx-one/src/server/services/valkey.service.ts
  • packages/shared/src/interfaces/index.ts
  • packages/shared/src/interfaces/org-lens-project-detail.interface.ts

Address review comments from copilot-pull-request-reviewer, coderabbitai[bot]:

- org-lens-project-detail.interface.ts: remove stale ?score= reference from
  OrgLensScoreType JSDoc (type reserved for a future toggle, not yet wired as
  a URL param); change OrgLensLeaderboardMetric comment from "Trend + Band
  columns" to "Band column" — the leaderboard has no separate Trend column
  (per copilot-pull-request-reviewer)
- org-project-detail.component.ts: filter orgUid$ in initDetailStream() so
  combineLatest only emits when a real uid is available, preventing a
  hard-coded 'demo-org' fallback from hitting assertOrgUid on the BFF and
  causing a 4xx + fetchError during initial hydration (per
  copilot-pull-request-reviewer); Prettier formatting applied
- org-lens-project-detail.service.ts: Prettier formatting applied (per
  coderabbitai[bot])

Resolves 4 review threads; 1 thread responded to as false positive
(component.ts format:check — file is already clean).

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
@danoqualls

Copy link
Copy Markdown
Contributor Author

Review Feedback Addressed

Commit: 8029a58

Changes Made

  • org-lens-project-detail.interface.ts: updated OrgLensScoreType JSDoc — removed the stale ?score= URL-param reference; clarified it is reserved for a future score-type toggle not yet wired. Also corrected OrgLensLeaderboardMetric comment from "Trend + Band columns" to "Band column" since the leaderboard table has no separate Trend column (per copilot-pull-request-reviewer)
  • org-project-detail.component.ts: removed 'demo-org' hardcoded fallback in initDetailStream()orgUid$ now filters to emit only when a real uid is available, preventing a 4xx on assertOrgUid during initial hydration; Prettier applied (per copilot-pull-request-reviewer)
  • org-lens-project-detail.service.ts: Prettier formatting applied — pipe/catchError reformatted to multi-line style (per coderabbitai[bot])

No Change Needed

  • org-project-detail.component.ts:1: format:check passes for this file locally — the coderabbitai[bot] flag appears to reference a stale CI run; Prettier was applied and the file is clean

Threads Resolved

5 of 5 unresolved threads addressed in this iteration.

@danoqualls danoqualls marked this pull request as ready for review June 25, 2026 15:04
Copilot AI review requested due to automatic review settings June 25, 2026 15:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Comment thread packages/shared/src/interfaces/org-lens-project-detail.interface.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Dano Qualls <mynameisdano@gmail.com>
Copilot AI review requested due to automatic review settings June 25, 2026 15:12
danoqualls and others added 2 commits June 25, 2026 09:12
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Dano Qualls <mynameisdano@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Dano Qualls <mynameisdano@gmail.com>

@MRashad26 MRashad26 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — PR #1028 feat/LFXV2-1885

Verdict: FAIL — Critical violations in template and interface/constant placement

Overview

Adds the Org Lens Project Detail page at /org/projects/:projectSlug with demo fixture data, Our Influence tab (hero, influence cards, trend chart), Leaderboards tab (leaderboard tables, band tags, stacked area chart), and Playwright E2E tests. Well-structured overall, but has violations across three categories.

Secrets check

org-lens-project-detail.demo-data.ts contains only demo company names, seed metrics, and placeholder values. No real API tokens, credentials, or endpoints.

Code standards

🟡 ## Test plan in PR body — lfx-self-serve PRs must not include a ## Test plan section. Move the 10 checklist items to the JIRA ticket (LFXV2-1885).


🔴 CRITICAL — Method calls in templates (org-project-detail.component.html)

Three component methods are called directly in the template — all forbidden in this codebase:

  1. [class]="bandChipClass(band)" (lines ~496, ~542) — method call in property binding
  2. @for (bar of bandSignalBars(band); track bar.x) (lines ~498, ~544) — method call in @for expression
  3. {{ bandLabel(band) }} (lines ~501, ~547) — method call in interpolation
  4. {{ personInitials(person.name) }} (line ~883) — method call in interpolation

Suggested fix — band methods: Pre-compute bandChipClass, bandSignalBars, and bandLabel into computed() signals using the existing technicalBand() / ecosystemBand() signals:

protected readonly technicalBandMeta = computed(() => {
  const band = this.technicalBand();
  if (!band) return null;
  return { label: BAND_TAG[band].label, chipClass: BAND_CHIP_CLASS[band], bars: buildSignalBars(band) };
});
// same for ecosystemBandMeta

Then in template: {{ technicalBandMeta()?.label }} / [class]="technicalBandMeta()?.chipClass".

Suggested fix — personInitials: Add initials: string to OrgLensCardDetailCell (the person shape) in the shared interface, and compute it in the service mapper instead of the template. Or promote personInitials() to a pure pipe.


🔴 CRITICAL — Interfaces/types defined in component file (org-project-detail.component.ts)

Two types are declared locally in the component file instead of in @lfx-one/shared/interfaces:

  • InfluenceCardVm (view model for influence cards)
  • LeaderboardDimension = 'technical' | 'ecosystem' (union type)

Move both to packages/shared/src/interfaces/org-lens-project-detail.interface.ts (the new shared interfaces file added in this PR).


🔴 CRITICAL — Module-level constants in component file (org-project-detail.component.ts)

14 module-level const declarations live in the component file instead of @lfx-one/shared/constants:

DEFAULT_TAB, VALID_TABS, DEFAULT_METRIC, VALID_METRICS, DEFAULT_TIME_RANGE, VALID_TIME_RANGES, HEALTH_TAG, BAND_TAG, BAND_SIGNAL_RANK, BAND_SIGNAL_FILL, BAND_SIGNAL_FILL_LIGHT, BAND_CHIP_CLASS, METRIC_OPTIONS, TIME_RANGE_OPTIONS, TIME_RANGE_MONTHS, STACKED_PALETTE

Move to packages/shared/src/constants/org-lens.constants.ts.


Code correctness — no issues

  • Signal patterns correct: all mutable state via signal(), derived state via computed(), URL param reads via toSignal(route.queryParamMap)
  • No effect() usage ✅
  • Valkey cache service change (JSON parse guard) is safe ✅
  • Route structure and query-param persistence correct ✅
  • E2E tests present and structured correctly ✅

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Comment thread apps/lfx-one/e2e/org-project-detail.spec.ts Outdated
Comment thread apps/lfx-one/e2e/org-project-detail.spec.ts Outdated
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Dano Qualls <mynameisdano@gmail.com>
Copilot AI review requested due to automatic review settings June 25, 2026 15:27
danoqualls and others added 2 commits June 25, 2026 09:27
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Dano Qualls <mynameisdano@gmail.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Dano Qualls <mynameisdano@gmail.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 3 comments.

Comment thread apps/lfx-one/src/server/services/org-lens-project-detail.demo-data.ts Outdated
Move local constants and interfaces to @lfx-one/shared per reviewer:

- packages/shared/src/constants/org-lens-project-detail.constants.ts (new):
  extracted all 16 constants (DEFAULT_TAB→PD_DEFAULT_TAB, VALID_TABS→PD_VALID_TABS,
  BAND_TAG, BAND_SIGNAL_RANK/FILL/FILL_LIGHT, BAND_CHIP_CLASS, METRIC_OPTIONS,
  TIME_RANGE_OPTIONS, TIME_RANGE_MONTHS, STACKED_PALETTE, HEALTH_TAG, etc.)
- packages/shared/src/interfaces/org-lens-project-detail.interface.ts:
  add InfluenceCardVm and LeaderboardDimension types; add initials field to
  OrgLensCardDetailCell.person
- packages/shared/src/constants/index.ts: barrel-export new constants file
- org-project-detail.component.ts: remove local constants/interfaces; replace
  bandChipClass(), bandSignalBars(), bandLabel(), personInitials() methods with
  precomputed technicalBandMeta/ecosystemBandMeta computed signals and
  private buildBandBars() helper
- org-project-detail.component.html: bind to technicalBandMeta()/ecosystemBandMeta()
  and person.initials instead of method calls
- org-lens-project-detail.demo-data.ts: fix ramp() zero case, add personInitials()
  helper and initials field on person cells, fix stale comment

LFXV2-1885

Signed-off-by: daniel qualls <dqualls@linuxfoundation.org>
@danoqualls

Copy link
Copy Markdown
Contributor Author

Review Feedback Addressed

Commit: 4bae381

Changes Made

  • org-project-detail.component.html: Replaced bandChipClass(band), bandSignalBars(band), and bandLabel(band) method calls with technicalBandMeta() / ecosystemBandMeta() precomputed computed signals; replaced personInitials(person.name) with person.initials (per @MRashad26)
  • org-project-detail.component.ts: Removed local InfluenceCardVm and LeaderboardDimension types; removed all 16 local constants; added technicalBandMeta / ecosystemBandMeta computed signals and buildBandBars() private helper; imports all types and constants from @lfx-one/shared (per @MRashad26)
  • packages/shared/src/constants/org-lens-project-detail.constants.ts (new): Extracted all 16 constants; DEFAULT_TABPD_DEFAULT_TAB and VALID_TABSPD_VALID_TABS to avoid collision with existing membership DEFAULT_TAB (per @MRashad26)
  • packages/shared/src/interfaces/org-lens-project-detail.interface.ts: Added InfluenceCardVm and LeaderboardDimension; added initials: string to OrgLensCardDetailCell.person; adopted discriminated union type shape (per @MRashad26, copilot-pull-request-reviewer)
  • org-lens-project-detail.demo-data.ts: Fixed ramp() zero case to return [] instead of 12 zeros; added personInitials() helper and initials field on person cells; fixed stale comment (per copilot-pull-request-reviewer)
  • PR description: Corrected "SSR JSON fixture" → "TypeScript demo-data service" to accurately describe how demo data is generated (per copilot-pull-request-reviewer)

Threads Resolved

7 of 7 unresolved threads addressed in this iteration.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (2)
packages/shared/src/interfaces/org-lens-project-detail.interface.ts (2)

15-31: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use shared contracts as the single source of truth

Lines 15-31 define types that are still duplicated in apps/lfx-one/src/app/modules/dashboards/org/org-project-detail/org-project-detail.component.ts (see its Lines 16-31). Please remove the local duplicates and import these shared exports to prevent contract drift.

🤖 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 `@packages/shared/src/interfaces/org-lens-project-detail.interface.ts` around
lines 15 - 31, The org project detail component still defines local copies of
LeaderboardDimension and InfluenceCardVm instead of using the shared contracts,
which risks drift between app and shared types. Remove the duplicate type
declarations from org-project-detail.component.ts and import the shared exports
from org-lens-project-detail.interface.ts, then update any usages in the
component to reference those shared symbols directly.

24-27: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Tighten InfluenceCardVm chart typing with a generic chart type

At Line 24-27, chartType is not type-coupled to chartData/chartOptions because all three use the broad ChartType union. Consider InfluenceCardVm<TType extends ChartType> so chartData/chartOptions are constrained to the selected chartType.

Proposed type-safe shape
-export interface InfluenceCardVm {
+export interface InfluenceCardVm<TType extends ChartType = ChartType> {
   key: string;
   title: string;
   scopeLabel: string | null;
   hasData: boolean;
-  chartType: ChartType;
-  chartData: ChartData<ChartType>;
-  chartOptions: ChartOptions<ChartType>;
+  chartType: TType;
+  chartData: ChartData<TType>;
+  chartOptions: ChartOptions<TType>;
   valueSuffix: string;
   caption: { prefix: string; emphasis: string; suffix: string };
   statLabel: string;
   testId: string;
 }
🤖 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 `@packages/shared/src/interfaces/org-lens-project-detail.interface.ts` around
lines 24 - 27, Tighten the `InfluenceCardVm` typing so `chartType`, `chartData`,
and `chartOptions` are linked by a generic chart type instead of the broad
`ChartType` union. Update the `InfluenceCardVm` interface to be generic (for
example, `InfluenceCardVm<TType extends ChartType>`) and use that type parameter
in the `chartData` and `chartOptions` fields so they match the selected
`chartType`. Then update any uses of `InfluenceCardVm` that construct or consume
these values to supply the concrete chart type and preserve type safety.
🤖 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 `@packages/shared/src/interfaces/org-lens-project-detail.interface.ts`:
- Around line 15-31: The org project detail component still defines local copies
of LeaderboardDimension and InfluenceCardVm instead of using the shared
contracts, which risks drift between app and shared types. Remove the duplicate
type declarations from org-project-detail.component.ts and import the shared
exports from org-lens-project-detail.interface.ts, then update any usages in the
component to reference those shared symbols directly.
- Around line 24-27: Tighten the `InfluenceCardVm` typing so `chartType`,
`chartData`, and `chartOptions` are linked by a generic chart type instead of
the broad `ChartType` union. Update the `InfluenceCardVm` interface to be
generic (for example, `InfluenceCardVm<TType extends ChartType>`) and use that
type parameter in the `chartData` and `chartOptions` fields so they match the
selected `chartType`. Then update any uses of `InfluenceCardVm` that construct
or consume these values to supply the concrete chart type and preserve type
safety.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ecce05be-781a-42df-b52f-f75db599e6bd

📥 Commits

Reviewing files that changed from the base of the PR and between 276e989 and 4bae381.

📒 Files selected for processing (6)
  • apps/lfx-one/src/app/modules/dashboards/org/org-project-detail/org-project-detail.component.html
  • apps/lfx-one/src/app/modules/dashboards/org/org-project-detail/org-project-detail.component.ts
  • apps/lfx-one/src/server/services/org-lens-project-detail.demo-data.ts
  • packages/shared/src/constants/index.ts
  • packages/shared/src/constants/org-lens-project-detail.constants.ts
  • packages/shared/src/interfaces/org-lens-project-detail.interface.ts
✅ Files skipped from review due to trivial changes (2)
  • packages/shared/src/constants/index.ts
  • packages/shared/src/constants/org-lens-project-detail.constants.ts
🚧 Files skipped from review as they are similar to previous changes (3)
  • apps/lfx-one/src/app/modules/dashboards/org/org-project-detail/org-project-detail.component.html
  • apps/lfx-one/src/app/modules/dashboards/org/org-project-detail/org-project-detail.component.ts
  • apps/lfx-one/src/server/services/org-lens-project-detail.demo-data.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants