Skip to content

Conversation

@asithade
Copy link
Contributor

@asithade asithade commented Sep 17, 2025

Summary

Remove mailing lists functionality from the UI and improve project card responsive layout.

JIRA Ticket

LFXV2-515

Changes Made

  • Removed mailing lists from project dashboard: Eliminated the mailing lists card section and adjusted grid layout from 3 columns to 2 columns on large screens
  • Removed mailing lists from home page project metrics: Eliminated mailing list count from project card metrics display
  • Removed mailing lists from project layout navigation: Removed mailing lists menu item from project navigation sidebar
  • Improved responsive grid layout: Added sm:grid-cols-2 breakpoint for better mobile/tablet experience, updated from 3 to 4 columns on large screens
  • Enhanced project card component: Implemented proper flex layout for consistent card heights across the grid
  • Added server-side metrics fetching: Implemented real-time metrics fetching for meetings and committees count in project controller
  • Temporarily commented out settings menu: Settings functionality not yet implemented, so menu item is temporarily hidden

Technical Details

Files Modified:

  • apps/lfx-one/src/app/layouts/project-layout/project-layout.component.html - Commented out settings menu
  • apps/lfx-one/src/app/layouts/project-layout/project-layout.component.ts - Removed mailing lists navigation item
  • apps/lfx-one/src/app/modules/pages/home/home.component.html - Improved grid responsive layout and card heights
  • apps/lfx-one/src/app/modules/pages/home/home.component.ts - Removed mailing list metrics
  • apps/lfx-one/src/app/modules/project/dashboard/project-dashboard/project.component.html - Removed mailing lists card, adjusted grid
  • apps/lfx-one/src/app/shared/components/project-card/project-card.component.html - Enhanced layout with proper flex containers
  • apps/lfx-one/src/server/controllers/project.controller.ts - Added metrics fetching for meetings and committees

Testing

  • Build process completed successfully with no errors
  • All linting and formatting checks passed
  • Metrics for meetings and committees are dynamically fetched from the API
  • Project cards now display with consistent heights across responsive breakpoints

Acceptance Criteria

  • Mailing lists no longer visible in project dashboard
  • Mailing lists removed from project navigation
  • Project card layout improved with consistent heights
  • Responsive grid works properly on mobile, tablet, and desktop
  • Metrics are dynamically fetched for projects
  • All existing functionality remains intact
  • Build and linting checks pass

Generated with Claude Code

- Remove mailing lists from project dashboard, navigation, and home page
- Improve responsive grid layout with better breakpoints (sm:grid-cols-2, lg:grid-cols-4)
- Enhance project card component with flex layout for consistent heights
- Add server-side metrics fetching for meetings and committees count
- Temporarily comment out settings menu item until implementation

LFXV2-515

Signed-off-by: Asitha de Silva <asithade@gmail.com>
@asithade asithade requested a review from jordane as a code owner September 17, 2025 17:51
@asithade asithade requested review from Copilot and removed request for jordane September 17, 2025 17:51
@coderabbitai
Copy link

coderabbitai bot commented Sep 17, 2025

Walkthrough

Removed Mailing Lists references from UI and tests; hid desktop Settings menu item. Updated home grid density and project card layout. Removed Mailing Lists card from project dashboard. Server now enriches projects with meetings_count and committees_count via MeetingService and CommitteeService in getProjects and searchProjects.

Changes

Cohort / File(s) Summary
E2E tests: remove Mailing Lists assertions
apps/lfx-one/e2e/homepage.spec.ts, apps/lfx-one/e2e/project-dashboard.spec.ts
Dropped/checks for “Mailing Lists” metric and menu items; commented out assertions for Mailing Lists and Settings in multiple dashboard scenarios; retained checks for Dashboard, Meetings, Committees.
Project layout/menu adjustments
apps/lfx-one/src/app/layouts/project-layout/project-layout.component.ts,
apps/lfx-one/src/app/layouts/project-layout/project-layout.component.html
Removed “Mailing Lists” from menuItems; commented out desktop Settings pill (md:flex) while keeping mobile entry.
Home page layout and metrics
apps/lfx-one/src/app/modules/pages/home/home.component.html,
apps/lfx-one/src/app/modules/pages/home/home.component.ts
Increased grid density (adds sm:2-cols, md:3, lg:4); added h-full to project card; removed Mailing Lists metric from transformed project metrics.
Project dashboard UI
apps/lfx-one/src/app/modules/project/dashboard/project-dashboard/project.component.html
Changed row from lg:3-cols to lg:2-cols; removed the Mailing Lists card and associated table/templates.
Project card component layout
apps/lfx-one/src/app/shared/components/project-card/project-card.component.html
Reworked structure to h-full flex column; resized logo; moved title/description; switched metric badge classes to ngClass with explicit severities.
Server-side project enrichment
apps/lfx-one/src/server/controllers/project.controller.ts
Added MeetingService and CommitteeService; in getProjects and searchProjects, augmented each project with meetings_count and committees_count using tag project_uid:, with error fallback to empty arrays and parallelization via Promise.all.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant C as Client
  participant PC as ProjectController
  participant PS as ProjectService
  participant MS as MeetingService
  participant CS as CommitteeService

  C->>PC: GET /projects
  PC->>PS: fetch projects
  PS-->>PC: projects[]

  loop for each project
    par Parallel counts
      PC->>MS: getMeetings(tag=project_uid:<uid>)
      PC->>CS: getCommittees(tag=project_uid:<uid>)
    and
    end
    MS-->>PC: meetings[]
    CS-->>PC: committees[]
    Note right of PC: Set meetings_count and committees_count (fallback to [])
  end

  PC-->>C: projects[] with counts

  %% Similar flow for search
  C->>PC: GET /projects/search?q=...
  PC->>PS: search projects
  PS-->>PC: projects[]
  loop enrich each
    par counts
      PC->>MS: getMeetings(tag=project_uid:<uid>)
      PC->>CS: getCommittees(tag=project_uid:<uid>)
    end
    MS-->>PC: meetings[]
    CS-->>PC: committees[]
  end
  PC-->>C: results[] with counts
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)
Check name Status Explanation
Title Check ✅ Passed The PR title succinctly captures the primary work: removing mailing lists from the UI and improving the project card layout. It directly reflects the main changes in the diff (removing mailing-lists navigation and card, removing the mailing list metric on the home page, and layout/card updates) and is concise for history scanning. The phrasing is specific and free of noisy or ambiguous terms.
Linked Issues Check ✅ Passed The changes implement the linked issue's primary coding objectives: mailing lists were removed from the home project cards, project dashboard, and navigation; responsive grid and project-card layout improvements were applied; server-side fetching for meetings and committees counts was added; and the settings menu was temporarily hidden. E2E tests were adjusted to match the UI changes. One remaining item from the file summaries is a retained "mailing lists" metric in project-layout.component.ts that could still surface a mailing-lists count in some header and may merit clarification. Apart from that residual, the PR satisfies the linked issue requirements.
Out of Scope Changes Check ✅ Passed No significant out-of-scope changes were introduced; the layout tweaks, server-side metric additions, and test updates are aligned with the linked issue. Minor cosmetic adjustments (logo height) and the commented-out desktop settings markup appear to be part of the intended visual polish or temporary hiding. The notable anomaly is the leftover mailing-lists metric in project-layout.component.ts, which looks like an incomplete removal rather than an unrelated change.
Description Check ✅ Passed The PR description is clearly related to the changeset, lists the modified files, links the JIRA ticket, and documents testing and acceptance criteria. It provides sufficient context for reviewers to understand scope and intent without being off-topic. This satisfies the lenient description check.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/LFXV2-515

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR removes mailing lists functionality from the UI and improves project card layout responsiveness. The changes streamline the project dashboard by removing the mailing lists card section and navigation item while enhancing the grid layout and card presentation.

  • Removes mailing lists functionality from project dashboard, navigation, and home page metrics
  • Improves responsive grid layout with better breakpoints and consistent card heights
  • Adds server-side metrics fetching for meetings and committees count

Reviewed Changes

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

Show a summary per file
File Description
project.controller.ts Adds server-side metrics fetching for meetings and committees
project-card.component.html Improves flex layout and card height consistency
project.component.html Removes mailing lists card and adjusts grid from 3 to 2 columns
home.component.ts Removes mailing list metrics from project cards
home.component.html Updates grid responsive breakpoints and adds card height class
project-layout.component.ts Removes mailing lists navigation menu item
project-layout.component.html Comments out settings menu temporarily
project-dashboard.spec.ts Updates test expectations for removed menu items
homepage.spec.ts Removes mailing lists metric test assertion

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@github-actions
Copy link

github-actions bot commented Sep 17, 2025

✅ E2E Tests Passed

Browser: chromium
Status: passed

All E2E tests passed successfully.

Test Configuration

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (6)
apps/lfx-one/src/app/layouts/project-layout/project-layout.component.html (1)

71-83: Hide Settings everywhere or gate it; also fix routerLink binding.

  • Inconsistent UX: desktop Settings is hidden but mobile link (Lines 59–69) remains. Hide it too until implemented, or guard both behind a feature flag.
  • Prefer property binding for routerLink over string interpolation.

Apply:

-        <div class="flex md:hidden">
-          <a
-            routerLink="/project/{{ projectSlug() }}/settings"
+        <!-- TODO: Restore Settings when implemented -->
+        <!-- <div class="flex md:hidden">
+          <a
+            [routerLink]="['/project', projectSlug(), 'settings']"
             class="pill"
             routerLinkActive="bg-blue-50 text-blue-600 border-0 block md:hidden"
             data-testid="mobile-menu-item"
             [routerLinkActiveOptions]="{ exact: true }">
             <i class="fa-light fa-gear"></i>
             <span>Settings</span>
-          </a>
-        </div>
+          </a>
+        </div> -->

And pre‑fix the commented desktop block for when it’s re‑enabled:

-          routerLink="/project/{{ projectSlug() }}/settings"
+          [routerLink]="['/project', projectSlug(), 'settings']"

Also applies to: 59-69, 61-61, 74-74

apps/lfx-one/e2e/project-dashboard.spec.ts (2)

84-86: Replace commented assertions with explicit “not visible” checks.

Keep intent executable and prevent regressions by asserting absence.

-        // await expect(page.getByTestId('menu-item').filter({ hasText: 'Mailing Lists' })).toBeVisible();
-        // await expect(page.getByTestId('menu-item').filter({ hasText: 'Settings' })).toBeVisible();
+        await expect(page.getByTestId('menu-item').filter({ hasText: 'Mailing Lists' })).not.toBeVisible();
+        await expect(page.getByTestId('menu-item').filter({ hasText: 'Settings' })).not.toBeVisible();

426-430: Likewise here: assert they’re not present instead of commenting.

-      // await expect(page.getByTestId('menu-item').filter({ hasText: 'Mailing Lists' })).toBeVisible();
+      await expect(page.getByTestId('menu-item').filter({ hasText: 'Mailing Lists' })).not.toBeVisible();

-      // await expect(page.getByTestId('menu-item').filter({ hasText: 'Settings' })).toBeVisible();
+      await expect(page.getByTestId('menu-item').filter({ hasText: 'Settings' })).not.toBeVisible();
apps/lfx-one/src/server/controllers/project.controller.ts (2)

84-92: Apply the same parallelization/logging for search results.

-      await Promise.all(
-        results.map(async (project) => {
-          project.meetings_count = (await this.meetingService.getMeetings(req, { tags: `project_uid:${project.uid}` }).catch(() => [])).length;
-          project.committees_count = (await this.committeeService.getCommittees(req, { tags: `project_uid:${project.uid}` }).catch(() => [])).length;
-        })
-      );
+      await Promise.all(
+        results.map(async (project) => {
+          const [meetingsRes, committeesRes] = await Promise.allSettled([
+            this.meetingService.getMeetings(req, { tags: `project_uid:${project.uid}` }),
+            this.committeeService.getCommittees(req, { tags: `project_uid:${project.uid}` }),
+          ]);
+          project.meetings_count = meetingsRes.status === 'fulfilled' ? meetingsRes.value.length : 0;
+          project.committees_count = committeesRes.status === 'fulfilled' ? committeesRes.value.length : 0;
+          if (meetingsRes.status === 'rejected' || committeesRes.status === 'rejected') {
+            Logger.warn(req, 'augment_project_metrics_failed', { project_uid: project.uid });
+          }
+        })
+      );

140-151: Enrich GET /projects/:slug response with meetings_count and committees_count (or document the difference).

getProjects and searchProjects already populate these metrics; add the same augmentation to getProjectBySlug for both the UUID and slug branches in apps/lfx-one/src/server/controllers/project.controller.ts (after fetching the project) — e.g. use Promise.allSettled to call meetingService.getMeetings and committeeService.getCommittees and set project.meetings_count / project.committees_count. packages/shared/src/interfaces/project.interface.ts already declares these fields, so no typing changes are required.

apps/lfx-one/src/app/shared/components/project-card/project-card.component.html (1)

8-11: Solid restructure; add a safe default for unknown badge severities.

Layout/testids look good. Provide a neutral style when severity is missing/mistyped.

-                    <span
+                    <span
                       class="inline-flex items-center px-2.5 py-0.5 rounded-full text-sm font-semibold"
                       [ngClass]="{
                         'bg-emerald-500 text-white': metric.badge.severity === 'success',
                         'bg-blue-500 text-white': metric.badge.severity === 'info',
                         'bg-amber-500 text-white': metric.badge.severity === 'warning',
                         'bg-red-500 text-white': metric.badge.severity === 'danger',
                       }"
+                      [class.bg-gray-100]="!['success','info','warning','danger'].includes(metric.badge.severity)"
+                      [class.text-gray-900]="!['success','info','warning','danger'].includes(metric.badge.severity)"
                       data-testid="metric-badge"
                       [attr.data-badge-severity]="metric.badge.severity">
                       {{ metric.badge.label }}
                     </span>

Also applies to: 19-27, 29-57

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between ce7de19 and c08f422.

📒 Files selected for processing (9)
  • apps/lfx-one/e2e/homepage.spec.ts (0 hunks)
  • apps/lfx-one/e2e/project-dashboard.spec.ts (2 hunks)
  • apps/lfx-one/src/app/layouts/project-layout/project-layout.component.html (2 hunks)
  • apps/lfx-one/src/app/layouts/project-layout/project-layout.component.ts (0 hunks)
  • apps/lfx-one/src/app/modules/pages/home/home.component.html (2 hunks)
  • apps/lfx-one/src/app/modules/pages/home/home.component.ts (0 hunks)
  • apps/lfx-one/src/app/modules/project/dashboard/project-dashboard/project.component.html (1 hunks)
  • apps/lfx-one/src/app/shared/components/project-card/project-card.component.html (1 hunks)
  • apps/lfx-one/src/server/controllers/project.controller.ts (3 hunks)
💤 Files with no reviewable changes (3)
  • apps/lfx-one/src/app/modules/pages/home/home.component.ts
  • apps/lfx-one/src/app/layouts/project-layout/project-layout.component.ts
  • apps/lfx-one/e2e/homepage.spec.ts
🧰 Additional context used
📓 Path-based instructions (5)
apps/lfx-one/src/**/*.html

📄 CodeRabbit inference engine (CLAUDE.md)

apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]

Files:

  • apps/lfx-one/src/app/layouts/project-layout/project-layout.component.html
  • apps/lfx-one/src/app/modules/pages/home/home.component.html
  • apps/lfx-one/src/app/modules/project/dashboard/project-dashboard/project.component.html
  • apps/lfx-one/src/app/shared/components/project-card/project-card.component.html
**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}

📄 CodeRabbit inference engine (CLAUDE.md)

Include required license headers on all source files

Files:

  • apps/lfx-one/src/app/layouts/project-layout/project-layout.component.html
  • apps/lfx-one/src/app/modules/pages/home/home.component.html
  • apps/lfx-one/src/app/modules/project/dashboard/project-dashboard/project.component.html
  • apps/lfx-one/e2e/project-dashboard.spec.ts
  • apps/lfx-one/src/server/controllers/project.controller.ts
  • apps/lfx-one/src/app/shared/components/project-card/project-card.component.html
**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.{ts,tsx}: Use TypeScript interfaces instead of union types for better maintainability
When defining PrimeNG-related types, reference the official PrimeNG component interfaces

Files:

  • apps/lfx-one/e2e/project-dashboard.spec.ts
  • apps/lfx-one/src/server/controllers/project.controller.ts
**/*.spec.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Use .spec.ts for content-based tests

Files:

  • apps/lfx-one/e2e/project-dashboard.spec.ts
**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not nest ternary expressions

Files:

  • apps/lfx-one/e2e/project-dashboard.spec.ts
  • apps/lfx-one/src/server/controllers/project.controller.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: E2E Tests / Playwright E2E Tests
🔇 Additional comments (3)
apps/lfx-one/src/app/modules/project/dashboard/project-dashboard/project.component.html (1)

159-159: LGTM: grid change aligns with 2-card row.

Reducing lg breakpoint to 2 columns matches the removal of the third (Mailing Lists) card and keeps heights consistent.

apps/lfx-one/src/app/modules/pages/home/home.component.html (2)

43-43: LGTM: responsive grid density bump.

sm:2 and lg:4 improve utilization without affecting tests (data-testid preserved).


58-59: LGTM: card height consistency via h-full.

Aligns with the project-card layout changes for uniform rows.

@github-actions
Copy link

🚀 Deployment Status

Your branch has been deployed to: https://ui-pr-90.dev.v2.cluster.linuxfound.info

Deployment Details:

  • Environment: Development
  • Namespace: ui-pr-90
  • ArgoCD App: ui-pr-90

The deployment will be automatically removed when this PR is closed.

@asithade asithade merged commit 738e43d into main Sep 17, 2025
12 of 14 checks passed
@asithade asithade deleted the feat/LFXV2-515 branch September 17, 2025 19:17
@github-actions
Copy link

🧹 Deployment Removed

The deployment for PR #90 has been removed.

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