feat(backend): implement controller-service architecture and query service integration #44

asithade · 2025-08-15T18:56:26Z

Summary

Refactored committee management from monolithic 987-line file to clean Controller-Service architecture
Integrated committee CRUD operations with LFX Query Service microservice
Implemented standardized logging and error handling helpers
Added ETag-based concurrency control for safe operations

Changes Made

Backend Architecture Refactoring

Controller-Service Pattern: Separated HTTP handling from business logic
Helper Classes: Added Logger and Responder for consistent operations
ETag Service: Implemented optimistic concurrency control
Microservice Integration: Full integration with Query Service API

Frontend Updates

Updated committee service for new backend endpoints
Maintained compatibility with existing UI components
Added support for ETag-based updates

Shared Package Updates

Moved API response interfaces to shared package
Updated committee interfaces for microservice compatibility
Centralized interfaces for reuse across frontend and backend

Documentation

Comprehensive backend architecture documentation
Added implementation examples and best practices
Updated development standards and commit conventions

Related JIRA Tickets

Main Ticket

LFXV2-24: Committee Management POC: Query Service Integration

Subtasks Completed

LFXV2-260: Backend: Implement Committee GET Operations with Query Service
LFXV2-261: Backend: Implement Committee CREATE Operation with Query Service
LFXV2-262: Backend: Implement Committee UPDATE Operation with Query Service
LFXV2-263: Backend: Implement Committee DELETE Operation with Query Service
LFXV2-264: Frontend: Update Committee GET Operations for Query Service
LFXV2-265: Frontend: Update Committee CREATE Operation for Query Service
LFXV2-266: Frontend: Update Committee UPDATE Operation for Query Service
LFXV2-267: Frontend: Update Committee DELETE Operation for Query Service
LFXV2-268: Implement Logger Helper for Standardized Request Logging
LFXV2-269: Implement Responder Helper for Consistent Error Handling
LFXV2-270: Refactor Committee Architecture to Controller-Service Pattern

Test Plan

All existing E2E tests pass
Committee CRUD operations work correctly
Error handling returns appropriate responses
Logging captures all operations with timing
ETag concurrency control prevents data conflicts

🤖 Generated with Claude Code

Signed-off-by: Asitha de Silva <[email protected]>

- Add ApiErrorResponse interface for standardized error responses - Add ValidationError interface for field-specific validation errors - Add PaginationInfo interface for pagination metadata - Centralize response interfaces for reuse across frontend and backend References: LFXV2-269, LFXV2-270 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>

- Add Logger helper with simplified methods (start, success, error, validation, etag, warning) - Add Responder helper for consistent error handling - Maintain microservice response format compatibility - Add sensitive data sanitization in Logger - Import interfaces from shared package in Responder References: LFXV2-268, LFXV2-269 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>

- Add CommitteeController for HTTP request handling - Add CommitteeService for business logic and query service integration - Add EtagService for concurrency control - Implement proper separation of concerns - Add comprehensive validation and error handling - Integrate Logger and Responder helpers References: LFXV2-270 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>

- Update committees route to use new controller-service architecture - Implement GET operations with query parameter support - Implement CREATE operation with validation - Implement UPDATE operation with ETag concurrency control - Implement DELETE operation with ETag validation - Add comprehensive error handling and logging - Update API client and microservice proxy services References: LFXV2-260, LFXV2-261, LFXV2-262, LFXV2-263 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>

- Update committee form component for new backend endpoints - Update table component for improved functionality - Maintain compatibility with existing UI patterns - Support ETag-based concurrency control in forms References: LFXV2-264, LFXV2-265, LFXV2-266, LFXV2-267 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>

- Update committee interfaces for microservice compatibility - Add support for new data structures from query service - Maintain backwards compatibility with existing frontend References: LFXV2-24 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>

- Add commit convention guidelines with JIRA ticket requirements - Update development memories with architecture patterns - Add branch naming conventions - Document JIRA ticket association requirements References: LFXV2-24 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>

- Document new Controller-Service architecture pattern - Add detailed implementation examples for Logger, Responder, and ETag helpers - Include directory structure and best practices - Add code quality standards and development guidelines - Document microservice integration patterns - Include comprehensive API examples and usage patterns References: LFXV2-270 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <[email protected]>

coderabbitai · 2025-08-15T18:56:34Z

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Repository-wide migration of committee/meeting identity and public fields (id → uid; public_enabled → public; public_name → display_name; committee_website → website). Introduces Controller-Service backend pattern (CommitteeController, CommitteeService), ETagService, Logger, Responder, extended microservice proxy and API client, shared API/ETag/error interfaces, and related frontend bindings and docs updates.

Changes

Cohort / File(s)	Summary
Docs & Guidelines `CLAUDE.md`, `docs/architecture/backend/README.md`	Add guideline moving shared interfaces/constants to shared package; document Controller-Service pattern, Logger/Responder/ETag helpers, architecture and implementation notes.
Shared Interfaces & Constants `packages/shared/src/interfaces/`, `packages/shared/src/constants/`	Rename committee/meeting id → uid; add ETag and richer API error interfaces (ETagResult, ETagError, ApiErrorResponse, ValidationError, PaginationInfo, ValidationApiError); add committee types (CommitteeSettings, CommitteeCreateData/UpdateData, validation types); add server constants and re-export server constants.
Frontend — Committees UI `apps/lfx-pcc/src/app/modules/project/committees/**`	Migrate templates and TS to use `uid`, `website`, `display_name`, `public`; rename form controls and payload shape; parent selection and filters use `uid`; add cleanFormData sanitizer; update routes, testids and action bindings to use `uid`.
Frontend — Meetings & Dashboard `apps/lfx-pcc/src/app/modules/project/meetings/`, `apps/lfx-pcc/src/app/modules/project/dashboard/`	Switch committee identity usage from `id` → `uid` across meeting card, MeetingCommitteeModal, meeting dashboard filtering, and project dashboard table rows/URLs. Tighten a few MIME/type assertions.
Frontend — Shared & Services `apps/lfx-pcc/src/app/shared/components/table/table.component.ts`, `apps/lfx-pcc/src/app/shared/services/committee.service.ts`	Add host ngSkipHydration to TableComponent; change getCommitteesByProject filter to tags-based `project_uid:<id>`.
Server — Controllers & Routes `apps/lfx-pcc/src/server/controllers/committee.controller.ts`, `apps/lfx-pcc/src/server/routes/committees.ts`	Add CommitteeController with CRUD handlers; routes now delegate to controller; committee-member routes keep Supabase usage with parent existence validation.
Server — Helpers `apps/lfx-pcc/src/server/helpers/logger.ts`, `apps/lfx-pcc/src/server/helpers/responder.ts`	Add Logger (start/success/error/validation/etag/warning + sanitize) and Responder (standardized error responses, validation responses, pagination util, handle(...) mapping).
Server — Services & Proxy `apps/lfx-pcc/src/server/services/**`, `apps/lfx-pcc/src/server/services/microservice-proxy.service.ts`, `apps/lfx-pcc/src/server/services/api-client.service.ts`	Add CommitteeService (validation, CRUD, settings, ETag usage); add ETagService (fetch/update/delete with ETag); extend MicroserviceProxyService and ApiClientService to support per-request customHeaders and proxyRequestWithResponse; introduce structured error extraction and transformError.
Server — Supabase adjustments `apps/lfx-pcc/src/server/services/supabase.service.ts`, `apps/lfx-pcc/src/server/routes/meetings.ts`	Use `committee.uid` for member/voting counts and tighten some type assertions; minor import/typing reorder.
Utilities & Misc `packages/shared/src/constants/committees.ts`, `apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-form/meeting-form.component.ts`	Add helper getValidCommitteeCategories(); tighten file-type type assertion.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant Client as Client
  participant Router as /committees Router
  participant Controller as CommitteeController
  participant Service as CommitteeService
  participant ETag as ETagService
  participant Proxy as MicroserviceProxyService
  participant LFX as LFX_V2 Service

  Client->>Router: HTTP request (GET/POST/PUT/DELETE)
  Router->>Controller: delegate request
  Controller->>Service: perform action (list/get/create/update/delete)
  alt get/update/delete requires ETag
    Service->>ETag: fetchWithETag(path)
    ETag->>Proxy: GET with headers
    Proxy->>LFX: proxied request
    LFX-->>Proxy: response + ETag
    Proxy-->>ETag: ApiResponse
    ETag-->>Service: { data, etag, headers }
    opt update/delete
      Service->>ETag: updateWithETag / deleteWithETag(path, etag)
      ETag->>Proxy: PUT/DELETE with If-Match/ETag
      Proxy->>LFX: proxied request
      LFX-->>Proxy: response
      Proxy-->>ETag: ApiResponse
    end
  end
  Service-->>Controller: result or error
  Controller-->>Client: JSON (200/201) or 204 / Responder.handle on errors

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

feat: meeting attachments, project service and logger improvements #28 — Overlapping identifier renames (id → uid) and shared-interface changes; likely direct code-level overlap.
feat: implement committee management for meetings #26 — Touches MeetingCommitteeModal and meeting interfaces; overlaps with uid and meeting/committee changes here.
feat(ui): add subcommittee support and enhanced filtering #43 — Modifies committee-related UI and shared Committee interface fields; likely to overlap with form/table/interface updates.

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ 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-24

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

Visit our Status Page to check the current availability of CodeRabbit.
Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

Copilot

Pull Request Overview

This PR refactors the committee management system from a monolithic architecture to a clean Controller-Service pattern while integrating with the LFX Query Service microservice. The implementation includes standardized logging, error handling, ETag-based concurrency control, and comprehensive interface updates across the codebase.

Key changes:

Implemented Controller-Service architecture with separation of concerns between HTTP handling and business logic
Integrated committee CRUD operations with LFX Query Service microservice using standardized API patterns
Added helper classes for consistent logging, response formatting, and ETag concurrency control
Updated all frontend components to use uid instead of id for committee identification

Reviewed Changes

Copilot reviewed 29 out of 29 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
packages/shared/src/interfaces/committee.interface.ts	Updated committee interfaces with new fields and validation structures for microservice compatibility
packages/shared/src/interfaces/meeting.interface.ts	Changed committee ID field from `id` to `uid` for consistency
packages/shared/src/interfaces/api.ts	Added ETag support interfaces and standardized API error response structures
apps/lfx-pcc/src/server/services/committee.service.ts	New service layer implementing business logic for committee operations with microservice integration
apps/lfx-pcc/src/server/controllers/committee.controller.ts	New controller implementing HTTP request handling with standardized logging and error responses
apps/lfx-pcc/src/server/helpers/logger.ts	Added standardized logging helper with timing, sanitization, and request correlation
apps/lfx-pcc/src/server/helpers/responder.ts	Added consistent error response formatting helper with proper HTTP status codes
apps/lfx-pcc/src/server/services/etag.service.ts	New service for ETag-based concurrency control with safe update operations
apps/lfx-pcc/src/server/routes/committees.ts	Refactored routes to use new controller pattern while maintaining member route compatibility
Multiple frontend components	Updated to use `uid` instead of `id` and adapted to new committee interface structure

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.}

packages/shared/src/interfaces/committee.interface.ts

apps/lfx-pcc/src/server/services/committee.service.ts

apps/lfx-pcc/src/server/helpers/logger.ts

apps/lfx-pcc/src/server/services/etag.service.ts

github-actions · 2025-08-15T19:05:37Z

✅ E2E Tests Passed

Browser: chromium
Status: passed

All E2E tests passed successfully.

Test Configuration

Node.js: 22
Command: e2e
Browser: chromium
Base URL: http://localhost:4200

coderabbitai

Actionable comments posted: 12

🔭 Outside diff range comments (6)

packages/shared/src/interfaces/meeting.interface.ts (1)
114-121: Duplicate DeleteMeetingRequest interface causes a compile-time error

There are two identical DeleteMeetingRequest declarations. This will fail TypeScript compilation due to a duplicate identifier.

Apply this diff to remove the duplicate:
 export interface DeleteMeetingRequest {
   deleteType?: 'single' | 'series';
 }
 
-export interface DeleteMeetingRequest {
-  deleteType?: 'single' | 'series';
-}
apps/lfx-pcc/src/app/modules/project/settings/components/user-form/user-form.component.ts (1)
154-167: Potential null/undefined project UID usage in initCommittees()

initCommittees() reads this.project()?.uid at construction time. If the project signal has not resolved yet, this passes "undefined" down to getCommitteesByProject, likely resulting in a bad request.

Apply this diff to wait for a valid project UID before querying committees:
-  private initCommittees(): Signal<{ label: string; value: string }[]> {
-    return toSignal(
-      this.committeeService.getCommitteesByProject(this.project()?.uid as string).pipe(
-        take(1),
-        map((committees) =>
-          committees.map((committee) => ({
-            label: committee.name,
-            value: committee.uid,
-          }))
-        )
-      ),
-      {
-        initialValue: [],
-      }
-    );
-  }
+  private initCommittees(): Signal<{ label: string; value: string }[]> {
+    return toSignal(
+      this.projectService.project$.pipe(
+        // wait until a project with uid is available
+        // eslint-disable-next-line rxjs/no-ignored-notifier
+        map((p) => p?.uid || null),
+        // filter out null/empty uids
+        // prefer explicit check to avoid passing "undefined" as string
+        // eslint-disable-next-line @typescript-eslint/no-unsafe-function-type
+        switchMap((uid) =>
+          uid ? this.committeeService.getCommitteesByProject(uid) : of([])
+        ),
+        take(1),
+        map((committees) =>
+          committees.map((committee) => ({
+            label: committee.name,
+            value: committee.uid,
+          }))
+        )
+      ),
+      { initialValue: [] }
+    );
+  }
CLAUDE.md (1)
126-127: Fix typo: “resuable” → “reusable”

Minor typo in the new guideline line.
-- All interfaces, resuable constants, and enums should live in the shared package.
+- All interfaces, reusable constants, and enums should live in the shared package.
apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.html (1)
69-101: Fix visibility property in CommitteeTable template

The Committee interface now uses public: boolean (packages/shared/src/interfaces/committee.interface.ts:12), so the template’s committee.public_enabled will be undefined and break at runtime. Update the visibility checks accordingly:

• File: apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.html
– Replace all occurrences of committee.public_enabled with committee.public

Suggested diff:
         
         <td class="px-4 py-3 text-center">
-          @if (committee.public_enabled) {
+          @if (committee.public) {
             <div class="inline-flex items-center gap-1 px-2 py-0.5 rounded-full bg-blue-100 text-blue-700 border border-blue-200">
               <i class="fa-light fa-eye text-[10px]"></i>
               <span class="text-[10px] font-medium">Public</span>
             </div>
-          } @else {
+          } @else {
             <div class="inline-flex items-center gap-1 px-2 py-0.5 rounded-full bg-gray-100 text-gray-700 border border-gray-200">
               <i class="fa-light fa-eye-slash text-[10px]"></i>
               <span class="text-[10px] font-medium">Private</span>
             </div>
           }
         </td>
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts (1)
191-203: Include ETag (If-Match) in your DELETE call

The current front-end deleteCommittee only takes an id and issues
http.delete<void>(`/api/committees/${id}`)
but the backend now requires an If-Match: <etag> header.

Please update:
apps/lfx-pcc/src/app/shared/services/committee.service.ts
• Change
public deleteCommittee(id: string): Observable<void> {
  return this.http.delete<void>(`/api/committees/${id}`).pipe(take(1));
}
to something like
public deleteCommittee(id: string, etag: string): Observable<void> {
  const headers = new HttpHeaders({ 'If-Match': etag });
  return this.http
    .delete<void>(`/api/committees/${id}`, { headers })
    .pipe(take(1));
}
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts
• Update the call at line 191 to pass the ETag:
this.committeeService.deleteCommittee(committee.uid, committee.etag).subscribe({ … });
apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.ts
• Likewise, adjust the call around line 116 to include committee.etag.
Once you’ve added the If-Match header and updated both the service signature and its usages, the DELETE flow will satisfy the backend’s ETag validation.
docs/architecture/backend/README.md (1)
60-62: Fix logging library inconsistency (Winston → Pino).

Docs mention Winston, but codebase and earlier sections standardize on Pino.

Apply this diff:
-Explore Winston logging configuration, structured logging, and monitoring strategies.
+Explore Pino logging configuration, structured logging, and monitoring strategies.

🧹 Nitpick comments (36)

apps/lfx-pcc/src/app/modules/project/dashboard/project-dashboard/project.component.ts (1)
241-247: Prefer display_name (fallback to name) if available in the Committee interface

If Committee.display_name exists (per shared interfaces update), consider using it for table titles to reflect the user-facing label.

Apply this diff to improve the label:
-      return committees.map((committee) => ({
-        id: committee.uid,
-        title: committee.name,
-        url: `/project/${project.slug}/committees/${committee.uid}`,
+      return committees.map((committee) => ({
+        id: committee.uid,
+        title: committee.display_name || committee.name,
+        url: `/project/${project.slug}/committees/${committee.uid}`,
         status: 'Active',
         date: committee.updated_at || committee.created_at,
       }));
packages/shared/src/interfaces/meeting.interface.ts (1)
70-112: Clarify committees identity in request DTOs (string[] should be UIDs)

If committees in CreateMeetingRequest/UpdateMeetingRequest are expected to be committee UIDs post-migration, consider documenting it in a JSDoc comment to prevent accidental use of legacy numeric IDs.

Apply this doc-comment addition to both interfaces:
 export interface CreateMeetingRequest {
   project_uid: string;
@@
-  committees?: string[];
+  /**
+   * Array of committee UIDs associated with this meeting.
+   */
+  committees?: string[];
 }
 
 export interface UpdateMeetingRequest {
   project_uid: string;
@@
-  committees?: string[];
+  /**
+   * Array of committee UIDs associated with this meeting.
+   */
+  committees?: string[];
 }
apps/lfx-pcc/src/app/shared/components/table/table.component.ts (1)

13-15: Good call adding ngSkipHydration for PrimeNG table

Skipping hydration here helps avoid SSR/CSR mismatch issues common with dynamic DOM manipulation in PrimeNG’s table.

Consider adding a short comment explaining why hydration is skipped, to prevent future “cleanup” PRs from removing it unintentionally.
packages/shared/src/interfaces/committee.interface.ts (3)
26-29: Clarify duplication of settings-derived fields on Committee vs CommitteeSettings

Committee includes business_email_required and is_audit_enabled with a note “populated from the settings endpoint,” while CommitteeSettings also defines these as canonical. Keeping them on Committee is fine if they’re denormalized for convenience, but it can drift. Consider documenting the intended source of truth and update semantics to avoid divergence.

Would you like me to propose a small JSDoc block to clarify that these fields on Committee are read-only/denormalized from CommitteeSettings?

48-63: “joinable” in create payload but not present on Committee

CreateData includes joinable?: boolean, but Committee no longer has joinable. If this field is removed platform-wide, keep CreateData consistent to avoid confusion and dead params across FE/BE.

Proposed cleanup:
 export interface CommitteeCreateData {
   name: string;
   category: string;
   description?: string;
   parent_uid?: string;
   business_email_required?: boolean;
   enable_voting?: boolean;
   is_audit_enabled?: boolean;
   public?: boolean;
   display_name?: string;
   sso_group_enabled?: boolean;
   sso_group_name?: string;
   website?: string;
   project_uid?: string;
-  joinable?: boolean;
 }
If joinable is still supported at creation (but omitted from the returned entity), consider adding a brief comment indicating it’s a write-only attribute.

78-81: Consider codifying validation error “code” as an enum

If error codes are finite/standardized, an enum (e.g., CommitteeValidationCode) improves discoverability and prevents typos in consumers.
apps/lfx-pcc/src/app/modules/project/committees/components/committee-table/committee-table.component.ts (1)
48-56: Optional: avoid O(N^2) filtering by pre-indexing children

Precompute children grouped by parent_uid once to prevent repeated scans per parent. Useful if committees list grows large.

Apply within this block:
-      const subcommittees = allCommittees.filter((c) => c.parent_uid === parent.uid);
-      subcommittees.forEach((sub) => {
-        result.push({ ...sub, level: 1 });
-      });
+      const subs = childrenByParent[parent.uid] ?? [];
+      for (const sub of subs) {
+        result.push({ ...sub, level: 1 });
+      }
And add this just above parentCommittees.forEach(...):
// Pre-index children by parent for O(N) assembly
const childrenByParent = allCommittees.reduce((acc, c) => {
  if (c.parent_uid) {
    (acc[c.parent_uid] ||= []).push(c);
  }
  return acc;
}, {} as Record<string, Committee[]>);
apps/lfx-pcc/src/app/shared/services/committee.service.ts (1)
26-28: Rename projectId to projectUid in service and confirm backend tag forwarding

Please update the service methods in apps/lfx-pcc/src/app/shared/services/committee.service.ts as follows:
--- a/apps/lfx-pcc/src/app/shared/services/committee.service.ts
+++ b/apps/lfx-pcc/src/app/shared/services/committee.service.ts
@@ -26,7 +26,7 @@ export class CommitteeService {
-  public getCommitteesByProject(projectId: string, limit?: number, orderBy?: string): Observable<Committee[]> {
-    let params = new HttpParams().set('tags', `project_uid:${projectId}`);
+  public getCommitteesByProject(projectUid: string, limit?: number, orderBy?: string): Observable<Committee[]> {
+    let params = new HttpParams().set('tags', `project_uid:${projectUid}`);

-  public getRecentCommitteesByProject(projectId: string, limit: number = 3): Observable<Committee[]> {
-    return this.getCommitteesByProject(projectId, limit, 'updated_at.desc');
+  public getRecentCommitteesByProject(projectUid: string, limit: number = 3): Observable<Committee[]> {
+    return this.getCommitteesByProject(projectUid, limit, 'updated_at.desc');
• Update all internal callers of these methods to pass projectUid instead of projectId.
• Ensure any unit/integration tests reflect the renamed parameter.

Verification required:
• Confirm the backend’s committees endpoint in apps/lfx-pcc/src/server/… accepts a tags query (formatted as project_uid:<uid>) and correctly forwards it to the Query Service.
• If no existing handler inspects req.query.tags, add or update the controller to pass tags through.
apps/lfx-pcc/src/server/services/supabase.service.ts (2)
147-175: Align getCommitteeById with UID migration (name/param/query).

The project moved to UID as canonical. This method still queries id=eq.${id} and then computes counts by passing the same id. If route params now carry a UID, this works but is confusing. Consider aligning naming and query to reflect UID to prevent future breakage.

Apply this diff to better reflect the UID-based API (adjust only if the committees table has a uid column):
-  public async getCommitteeById(id: string) {
+  public async getCommitteeById(id: string) {
     const params = {
-      id: `eq.${id}`,
+      uid: `eq.${id}`,
       limit: '1',
     };
     const queryString = new URLSearchParams(params).toString();
     const url = `${this.baseUrl}/committees?${queryString}`;
@@
-    if (committee) {
-      // Calculate total members and voting reps
-      const [totalMembers, totalVotingReps] = await Promise.all([this.getCommitteeMemberCountByCommitteeId(id), this.getCommitteeVotingRepsCount(id)]);
+    if (committee) {
+      // Calculate total members and voting reps (treat incoming id as UID)
+      const [totalMembers, totalVotingReps] = await Promise.all([
+        this.getCommitteeMemberCountByCommitteeId(id),
+        this.getCommitteeVotingRepsCount(id),
+      ]);
If the DB doesn’t expose uid on committees, skip changing the query key and instead rename local variables for clarity (e.g., committeeUid).

132-134: Confirm database schema for committee_members.committee_id before merging

I didn’t find any committee_uid column in the codebase—every Supabase REST call (and the member.interface.ts) still filters on committee_id. Since we’re now passing committee.uid (a string) into:

apps/lfx-pcc/src/server/services/supabase.service.ts:
• getCommitteeMemberCountByCommitteeId (line 380): committee_id: \eq.${committeeId}` • getCommitteeVotingRepsCount (line 410):committee_id: `eq.${committeeId}``

—but the table column remains named committee_id, it’s almost certain that it still holds the legacy internal numeric ID unless a migration repointed it to store UIDs. If that migration hasn’t run (or if there isn’t a committee_uid column), these count queries will silently return 0.

Please verify that in your DB:

committee_members.committee_id now stores the committee UID
OR

there is a committee_uid column you can filter on

Otherwise you’ll need to either update the schema/view or revert to using the internal ID until the UID migration is fully in place.
apps/lfx-pcc/src/app/modules/project/committees/components/committee-members/committee-members.component.ts (1)
210-235: Use UID for member delete; consider using finalize() to avoid duplicated state toggles.

The switch to committee.uid is consistent. Minor cleanup: use RxJS finalize to ensure isDeleting is reset in both success and error paths.

Apply:
-    this.committeeService.deleteCommitteeMember(committee.uid, member.id).subscribe({
-      next: () => {
-        this.isDeleting.set(false);
+    this.committeeService
+      .deleteCommitteeMember(committee.uid, member.id)
+      .pipe(finalize(() => this.isDeleting.set(false)))
+      .subscribe({
+        next: () => {
@@
-      },
-      error: (error) => {
-        this.isDeleting.set(false);
+        },
+        error: (error) => {
           console.error('Failed to delete member:', error);
@@
-      },
-    });
+        },
+      });
Also confirm the backend expects a committee UID in this endpoint signature (not an internal ID).
apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.html (2)
117-126: Use new Angular control flow consistently and avoid explicit === true check

You’re mixing new @if with *ngIf. For consistency and readability, consider using @if here and drop the explicit === true.
-    <div *ngIf="form().get('public')?.value === true">
+    @if (form().get('public')?.value) {
       <label for="public-name" class="block text-sm font-medium text-gray-700 mb-1"> Public Name </label>
       <lfx-input-text
         size="small"
         [form]="form()"
         control="display_name"
         id="public-name"
         placeholder="Enter public display name"
         styleClass="w-full"></lfx-input-text>
-    </div>
+    }
110-115: Add data-testid attributes to key inputs/toggles for E2E stability

While not a new component, adding stable test selectors on changed/critical controls improves E2E resilience and aligns with our testing guidelines.
   <lfx-toggle
     size="small"
     [form]="form()"
     control="public"
     label="Make Committee Public"
-    id="public-toggle"
+    id="public-toggle"
+    data-testid="committees-form-public-toggle"
     tooltip="When enabled, the committee will be visible to the public on LFX tools like the public meeting calendar"
     tooltipPosition="right"></lfx-toggle>

   <lfx-input-text
     size="small"
     [form]="form()"
     control="display_name"
-    id="public-name"
+    id="public-name"
+    data-testid="committees-form-public-name"
     placeholder="Enter public display name"
     styleClass="w-full"></lfx-input-text>

   <lfx-input-text
     size="small"
     [form]="form()"
     control="website"
-    id="committee-website"
+    id="committee-website"
+    data-testid="committees-form-website"
     type="url"
     placeholder="https://example.com"
     styleClass="w-full"></lfx-input-text>
   <p class="mt-1 text-sm text-red-600" *ngIf="form().get('website')?.errors?.['pattern'] && form().get('website')?.touched">Please enter a valid URL</p>
Also applies to: 120-126, 154-165
apps/lfx-pcc/src/app/modules/project/committees/committee-dashboard/committee-dashboard.component.ts (1)
191-203: Simplify isDeleting toggling with finalize

Minor readability improvement: use finalize to centralize the flag reset.
-    this.committeeService.deleteCommittee(committee.uid).subscribe({
-      next: () => {
-        this.isDeleting.set(false);
-        // Refresh the committees list by reloading
-        this.refreshCommittees();
-      },
-      error: (error) => {
-        this.isDeleting.set(false);
-        console.error('Failed to delete committee:', error);
-        // TODO: Show error toast/notification
-      },
-    });
+    this.committeeService
+      .deleteCommittee(committee.uid)
+      .pipe(finalize(() => this.isDeleting.set(false)))
+      .subscribe({
+        next: () => {
+          // Refresh the committees list by reloading
+          this.refreshCommittees();
+        },
+        error: (error) => {
+          console.error('Failed to delete committee:', error);
+          // TODO: Show error toast/notification
+        },
+      });
Note: add import { finalize } from 'rxjs/operators'; if not already present.
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-committee-modal/meeting-committee-modal.component.ts (1)
124-132: Avoid mutating arrays when comparing sets of IDs

Array.prototype.sort() mutates the array. Clone before sorting to avoid mutating form values or local references.
-    const selectedIds = this.form.value.committees as string[];
+    const selectedIds = this.form.value.committees as string[];

     // If no changes, just close
-    const currentIds = this.meeting.meeting_committees?.map((c) => c.uid) || [];
-    if (JSON.stringify(selectedIds.sort()) === JSON.stringify(currentIds.sort())) {
+    const currentIds = this.meeting.meeting_committees?.map((c) => c.uid) || [];
+    const sortedSelected = [...(selectedIds || [])].sort();
+    const sortedCurrent = [...currentIds].sort();
+    if (JSON.stringify(sortedSelected) === JSON.stringify(sortedCurrent)) {
       this.ref.close();
       return;
     }
apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.html (2)
131-141: Consider updating label text to match field rename

Nit: “Public Enabled” could be shortened to “Public” or “Public Visibility” to reflect the new public field name.

77-79: Optionally add a data-testid to Next Meeting component

For E2E targeting, consider adding a test id to the upcoming meeting component.
-              <lfx-upcoming-committee-meeting [committeeId]="committee()!.uid"></lfx-upcoming-committee-meeting>
+              <lfx-upcoming-committee-meeting
+                data-testid="committee-view-next-meeting"
+                [committeeId]="committee()!.uid"></lfx-upcoming-committee-meeting>
apps/lfx-pcc/src/server/routes/committees.ts (2)

25-58: Unify member routes with Logger/Responder for consistent responses and logs

Member endpoints still hand-roll JSON errors and use req.log. For consistency with the new Logger/Responder approach, consider migrating these to a small CommitteeMemberController that uses the same helpers, even if they still back Supabase for now. This will:

Standardize error schema and codes (ETag/validation/conflict formatting)

Centralize logging and timing

Ease later migration to the query microservice

If you prefer to keep routes inline, at least wrap errors via Responder and swap req.log for the Logger helper.

I can provide a drop-in controller skeleton for committee members using Responder/Logger if helpful.

Also applies to: 60-109, 111-161, 162-215, 216-268

17-23: Path param naming: consider :uid for clarity

Optional: since the canonical identity is uid, consider renaming :id to :uid in these routes for clarity. The controller would simply read req.params.uid.
packages/shared/src/interfaces/api.ts (1)
56-61: Replace union of string literals with a shared enum for error codes

To align with the guideline “Use TypeScript interfaces instead of union types” and centralize shared enums, introduce an enum for ETag error codes (in packages/shared/src/enums) and reference it here. This improves maintainability and avoids scattering string unions.

Apply this diff in the current file:
-export interface ETagError {
-  code: 'NOT_FOUND' | 'ETAG_MISSING' | 'NETWORK_ERROR' | 'PRECONDITION_FAILED';
+import { ETagErrorCode } from '../enums/etag-error-code.enum';
+
+export interface ETagError {
+  code: ETagErrorCode;
   message: string;
   statusCode: number;
   headers?: Record<string, string>;
 }
Add this new enum file (outside the selected range):
// packages/shared/src/enums/etag-error-code.enum.ts
// Copyright The Linux Foundation and each contributor to LFX.
// SPDX-License-Identifier: MIT

export enum ETagErrorCode {
  NOT_FOUND = 'NOT_FOUND',
  ETAG_MISSING = 'ETAG_MISSING',
  NETWORK_ERROR = 'NETWORK_ERROR',
  PRECONDITION_FAILED = 'PRECONDITION_FAILED',
}
apps/lfx-pcc/src/server/controllers/committee.controller.ts (5)
47-68: Surface ETag on GET by id responses to enable optimistic concurrency on the client

Given the PR’s ETag support, consider returning the ETag header with GET /committees/:id. Either have the service return an ETagResult or retrieve headers via proxyRequestWithResponse and set res.set('ETag', etag) before res.json.

If the service already returns an ETagResult, the controller could look like:
-const committee = await this.committeeService.getCommitteeById(req, id);
+const { data: committee, etag } = await this.committeeService.getCommitteeById(req, id);
+if (etag) {
+  res.set('ETag', etag);
+}
73-92: Consider adding Location header on creation

For POST /committees, set a Location header pointing to the new resource (e.g., /committees/:uid) to follow REST best practices.
- res.status(201).json(newCommittee);
+ res.location(`/committees/${newCommittee.uid}`).status(201).json(newCommittee);
25-41: Pagination metadata opportunity for list endpoint

If the downstream Query Service supports pagination, consider returning pagination info (e.g., via Responder.createPagination) along with the result to standardize list responses.

18-20: Remove unused constructor property or use it

microserviceProxy is stored as a private field but only used to instantiate CommitteeService. You can drop the class property and pass the parameter directly, or keep and reuse it if you plan to.
-  public constructor(private microserviceProxy: MicroserviceProxyService) {
-    this.committeeService = new CommitteeService(microserviceProxy);
+  public constructor(microserviceProxy: MicroserviceProxyService) {
+    this.committeeService = new CommitteeService(microserviceProxy);
   }
56-60: Use uid consistently in logs and metadata keys

Given the repo-wide shift to uid, prefer committee_uid over committee_id in log metadata for consistency.

Example:
- committee_id: id,
+ committee_uid: id,
Also applies to: 82-85, 108-111, 133-135
apps/lfx-pcc/src/server/services/microservice-proxy.service.ts (3)
48-59: Log the actual endpoint URL

endpoint in the log currently uses ${service}${path}, which can be misleading. Prefer the fully resolved endpoint URL for clarity.
-          endpoint: `${service}${path}`,
+          endpoint,
Also applies to: 95-106

32-38: Avoid recreating MICROSERVICE_URLS per call

Minor: hoist MICROSERVICE_URLS to a module-level constant or inject config. Reduces per-call overhead and centralizes config.

Also applies to: 79-85

136-204: Error transformation is comprehensive; consider ETag-specific conflicts

If you plan to surface PRECONDITION_FAILED (412) conflicts distinctly, add a 412 case here and map to a more specific code (e.g., PRECONDITION_FAILED) for downstream Responder handling.
apps/lfx-pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts (3)
25-28: Verify Angular version: styleUrl vs styleUrls

styleUrl is only supported in newer Angular versions. If your project uses an older version, switch to styleUrls: ['...'].
-  styleUrl: './committee-form.component.scss',
+  styleUrls: ['./committee-form.component.scss'],
248-249: Loosen website URL pattern to allow localhost/dev and intranet URLs

The current pattern requires a dot, which rejects localhost or internal hosts. Consider a simpler scheme-based check or rely on backend validation.
-      website: new FormControl(committee?.website || '', [Validators.pattern(/^https?:\/\/.+\..+/)]),
+      website: new FormControl(committee?.website || '', [Validators.pattern(/^https?:\/\/.+/)]),
144-159: Optional: deep-clean nested fields and arrays

cleanFormData only handles top-level strings. If nested optional strings (now or in future) appear, a recursive cleaner improves consistency.

Example recursive cleaner:
private cleanFormDataDeep<T>(value: T): T {
  if (typeof value === 'string') return (value.trim() === '' ? null : value) as unknown as T;
  if (Array.isArray(value)) return value.map((v) => this.cleanFormDataDeep(v)) as unknown as T;
  if (value && typeof value === 'object') {
    const out: any = {};
    for (const [k, v] of Object.entries(value as any)) out[k] = this.cleanFormDataDeep(v);
    return out as T;
  }
  return value;
}
apps/lfx-pcc/src/server/services/committee.service.ts (3)
23-27: Prefer injecting ETagService for testability and separation of concerns.

Constructing ETagService internally couples this class and makes mocking harder. Inject it (optionally defaulting) to improve testability.

One option (minimal change) is to accept an optional ETagService:
-  private etagService: ETagService;
-
-  public constructor(private microserviceProxy: MicroserviceProxyService) {
-    this.etagService = new ETagService(microserviceProxy);
-  }
+  public constructor(
+    private microserviceProxy: MicroserviceProxyService,
+    private etagService: ETagService = new ETagService(microserviceProxy)
+  ) {}
46-64: Align naming with UID-based identifiers.

The repo is shifting to uid; method parameters still named committeeId. Consider renaming to committeeUid for clarity and consistency.

Example:
-  public async getCommitteeById(req: Request, committeeId: string): Promise<Committee> {
+  public async getCommitteeById(req: Request, committeeUid: string): Promise<Committee> {
   ...
-      parent: `committee:${committeeId}`,
+      parent: `committee:${committeeUid}`,
Apply similarly to updateCommittee and deleteCommittee.

Also applies to: 120-175, 180-194

110-115: Returned settings may be misleading if settings update fails.

When settings update throws, the response still includes the requested settings values, which may not match persisted state. Either re-fetch settings before merging or omit these fields when the settings update fails.

Possible adjustment: track success and only merge on success, or fetch /committees/${id}/settings after update to return the authoritative state.

Also applies to: 170-175
apps/lfx-pcc/src/server/helpers/responder.ts (2)
99-110: Guard against invalid limits in pagination.

limit <= 0 yields Infinity/NaN pages. Add a floor to 1 to avoid bad metadata.

Apply this diff:
-  public static createPagination(page: number, limit: number, total: number): PaginationInfo {
-    const pages = Math.ceil(total / limit);
+  public static createPagination(page: number, limit: number, total: number): PaginationInfo {
+    const safeLimit = Math.max(1, limit || 0);
+    const pages = Math.ceil(total / safeLimit);
115-146: Propagate details and handle 409 conflicts in handler.

If transformError maps 409 to code 'CONFLICT', let Responder return 409. Also forward error.details where available for troubleshooting.

Apply this diff:
   public static handle(res: Response, error: any, operation = 'operation'): void {
@@
     if (error.code === 'ETAG_MISSING') {
       this.internalError(res, 'ETag header missing from upstream service');
       return;
     }
+    if (error.code === 'CONFLICT') {
+      this.conflict(res, error.message);
+      return;
+    }
@@
-    this.error(res, message, {
-      statusCode,
-      code: statusCode >= 500 ? 'INTERNAL_ERROR' : undefined,
-    });
+    this.error(res, message, {
+      statusCode,
+      code: statusCode >= 500 ? 'INTERNAL_ERROR' : undefined,
+      details: error.details,
+    });

apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.html

...pcc/src/app/modules/project/committees/components/committee-form/committee-form.component.ts

apps/lfx-pcc/src/server/helpers/logger.ts

apps/lfx-pcc/src/server/services/etag.service.ts

apps/lfx-pcc/src/server/services/microservice-proxy.service.ts

docs/architecture/backend/README.md

- Prevent Authorization header override by placing customHeaders first - Fix error status code handling to prefer statusCode over status - Update documentation examples to match actual service implementations - Correct ETagService to show instance methods instead of static - Fix MicroserviceProxyService example to use proxyRequest method Addresses PR #44 review feedback References: LFXV2-24 Signed-off-by: Asitha de Silva <[email protected]>

- Created comprehensive error interface types with proper type guards - Updated all error handling to use extractErrorDetails utility - Implemented ValidationApiError interface for validation-specific errors - Added proper type guards for isApiError and isValidationApiError - Fixed all type casting issues in services and helpers - Updated API client, microservice proxy, and ETag services with type safety - Replaced 'as any' with proper type assertions for file type validation - Enhanced error logging and response handling with typed error details - Used 'in' operator checks instead of unsafe type casting All backend error handling now uses proper TypeScript types without any type assertions. References: LFXV2-24 Signed-off-by: Asitha de Silva <[email protected]>

coderabbitai

Actionable comments posted: 1

🔭 Outside diff range comments (2)

apps/lfx-pcc/src/server/services/microservice-proxy.service.ts (1)
137-204: Critical: createApiError receives ‘statusCode’ but setter only assigns ‘status’ (status lost → always 500 downstream)

createApiError sets error.status when options.status is provided. Passing statusCode leaves status undefined, causing extractErrorDetails to fall back to 500. Fix by passing status and add explicit 412 mapping for ETag conflicts.

Apply this diff:
   switch (errorDetails.statusCode) {
     case 400:
@@
-    case 409:
+    case 409:
       userMessage = 'Conflict. The resource you are trying to create already exists.';
       errorCode = 'CONFLICT';
       break;
+    case 412:
+      userMessage = 'Resource has been modified. Please refresh and try again.';
+      errorCode = 'PRECONDITION_FAILED';
+      break;
@@
-    return createApiError({
-      message: userMessage,
-      statusCode: errorDetails.statusCode,
-      code: errorCode,
-      service,
-      path,
-      originalMessage: errorDetails.message,
-      originalError: error instanceof Error ? error : undefined,
-    });
+    return createApiError({
+      message: userMessage,
+      status: errorDetails.statusCode,
+      code: errorCode,
+      service,
+      path,
+      originalMessage: errorDetails.message,
+      originalError: error instanceof Error ? error : undefined,
+    });
docs/architecture/backend/README.md (1)
60-62: Documentation inconsistency: mentions Winston but project uses Pino

Earlier sections (Lines 13-15) correctly document Pino. Update this section to avoid confusion.

Apply this diff:
-Explore Winston logging configuration, structured logging, and monitoring strategies.
+Explore Pino logging configuration, structured logging, and monitoring strategies.

♻️ Duplicate comments (5)

apps/lfx-pcc/src/server/services/microservice-proxy.service.ts (2)
46-46: Sanitize and allowlist custom headers before forwarding to the HTTP client

Forwarding arbitrary custom headers can introduce security/behavioral issues and unintentionally override protected headers. Filter to an allowlist (e.g., If-Match, If-None-Match, X-Request-Id, X-Correlation-Id) before passing to the client.

Apply this diff to use a sanitized header set here:
-      const response = await this.executeRequest<T>(method, endpoint, token, data, mergedParams, customHeaders);
+      const safeHeaders = this.filterCustomHeaders(customHeaders);
+      const response = await this.executeRequest<T>(method, endpoint, token, data, mergedParams, safeHeaders);
Add this helper method in the class (outside the selected range):
// Inside MicroserviceProxyService
private filterCustomHeaders(custom?: Record<string, string>): Record<string, string> | undefined {
  if (!custom) return undefined;
  const allowlist = new Set(['if-match', 'if-none-match', 'x-request-id', 'x-correlation-id']);
  const safe: Record<string, string> = {};
  for (const [k, v] of Object.entries(custom)) {
    const key = k.toLowerCase();
    if (allowlist.has(key)) safe[key] = v;
  }
  return Object.keys(safe).length ? safe : undefined;
}
66-74: Mirror header sanitization in proxyRequestWithResponse for consistency

Ensure both proxy paths apply the same header filtering to avoid inconsistent behavior.

Apply this diff:
-      const response = await this.executeRequest<T>(method, endpoint, token, data, mergedParams, customHeaders);
+      const safeHeaders = this.filterCustomHeaders(customHeaders);
+      const response = await this.executeRequest<T>(method, endpoint, token, data, mergedParams, safeHeaders);
Also applies to: 93-95
apps/lfx-pcc/src/server/services/etag.service.ts (2)
107-108: Use the standard If-Match header for conditional updates (not ETag)

Most services enforce preconditions via If-Match. Sending ETag won’t trigger a precondition check.

Apply this diff:
-    return await this.microserviceProxy.proxyRequest<T>(req, service, path, 'PUT', {}, data, { [HTTP_HEADERS.ETAG]: etag });
+    return await this.microserviceProxy.proxyRequest<T>(req, service, path, 'PUT', {}, data, { ['If-Match']: etag });
124-125: Use the standard If-Match header for conditional deletes (not ETag)

Apply this diff:
-    await this.microserviceProxy.proxyRequest(req, service, path, 'DELETE', {}, undefined, { [HTTP_HEADERS.ETAG]: etag });
+    await this.microserviceProxy.proxyRequest(req, service, path, 'DELETE', {}, undefined, { ['If-Match']: etag });
apps/lfx-pcc/src/server/services/api-client.service.ts (1)

43-53: Authorization cannot be overridden by custom headers — good

Merging customHeaders first and setting Authorization afterwards ensures the bearer token remains authoritative while allowing ETag/correlation headers through.

🧹 Nitpick comments (15)

packages/shared/src/constants/committees.ts (1)
92-97: Return a read-only array to discourage accidental mutation

The values are used for validation; returning a read-only view communicates intent and avoids accidental mutations by consumers.

Apply this diff:
-export function getValidCommitteeCategories(): string[] {
+export function getValidCommitteeCategories(): ReadonlyArray<string> {
   return COMMITTEE_CATEGORIES.map((category) => category.value);
 }
packages/shared/src/interfaces/api.ts (4)
21-30: Tighten response typing and document status vs statusCode

Prefer ApiResponse over any to retain type-safety.

If both status and statusCode are needed for interop, add a brief JSDoc explaining precedence to reduce confusion.

Apply this diff:
 export interface ApiError extends Error {
   status?: number;
   statusCode?: number;
   code?: string;
   service?: string;
   path?: string;
   originalMessage?: string;
   cause?: { code?: string };
-  response?: ApiResponse<any>;
+  response?: ApiResponse<unknown>;
 }

 export interface ApiErrorOptions {
   message: string;
   status?: number;
   statusCode?: number;
   code?: string;
   service?: string;
   path?: string;
   originalMessage?: string;
   originalError?: Error;
-  response?: ApiResponse<any>;
+  response?: ApiResponse<unknown>;
 }
Also applies to: 32-42

54-66: Derive ETagError.code from shared ERROR_CODES to avoid drift

Leverage the shared ERROR_CODES to centralize allowable codes and prevent divergence across the codebase.

Apply this diff:
+type ETagErrorCode =
+  | typeof ERROR_CODES.NOT_FOUND
+  | typeof ERROR_CODES.ETAG_MISSING
+  | typeof ERROR_CODES.NETWORK_ERROR
+  | typeof ERROR_CODES.PRECONDITION_FAILED;
+
 export interface ETagError {
-  code: 'NOT_FOUND' | 'ETAG_MISSING' | 'NETWORK_ERROR' | 'PRECONDITION_FAILED';
+  code: ETagErrorCode;
   message: string;
   statusCode: number;
   headers?: Record<string, string>;
 }
And add this import at the top of the file:
import { ERROR_CODES } from '../constants/server';
104-112: Broaden type guard to detect HTTP client errors with response.status

Some HTTP client errors (e.g., Axios) may expose only response.status without setting status/statusCode on the error object. Including this increases reliability of the guard.

Apply this diff:
 export function isApiError(error: unknown): error is ApiError {
   if (!(error instanceof Error)) return false;

   return (
     ('status' in error && typeof error.status === 'number') ||
     ('statusCode' in error && typeof error.statusCode === 'number') ||
-    ('code' in error && typeof error.code === 'string')
+    ('code' in error && typeof error.code === 'string') ||
+    ('response' in error && typeof (error as any).response?.status === 'number')
   );
 }
126-141: Include response.status fallback when extracting status code

Improves accuracy when upstream libraries set only response.status.

Apply this diff:
     return {
       message: error.message,
-      statusCode: error.statusCode || error.status || 500,
+      statusCode: error.statusCode || error.status || error.response?.status || 500,
       code: error.code || 'UNKNOWN_ERROR',
       service: error.service,
       path: error.path,
     };
apps/lfx-pcc/src/server/routes/meetings.ts (3)
25-25: LGTM: Passing req.query directly is fine given logging/validation

Assuming SupabaseService.getMeetings validates/sanitizes its input, this is acceptable. Consider a typed DTO at the service boundary in a future pass for stronger typing.

477-495: Normalize MIME types before validation to handle params and casing

Incoming MIME types can include parameters (e.g., "image/jpeg; charset=binary") or casing variants, which would fail exact includes checks.

Apply this diff:
-    if (!ALLOWED_FILE_TYPES.includes(mimeType as (typeof ALLOWED_FILE_TYPES)[number])) {
+    const normalizedMimeType = String(mimeType).split(';')[0].trim().toLowerCase();
+    if (!ALLOWED_FILE_TYPES.includes(normalizedMimeType as (typeof ALLOWED_FILE_TYPES)[number])) {
658-665: Normalize MIME types here as well for consistency

Mirror the normalization in the general storage upload endpoint.

Apply this diff:
-    if (!ALLOWED_FILE_TYPES.includes(mimeType as (typeof ALLOWED_FILE_TYPES)[number])) {
+    const normalizedMimeType = String(mimeType).split(';')[0].trim().toLowerCase();
+    if (!ALLOWED_FILE_TYPES.includes(normalizedMimeType as (typeof ALLOWED_FILE_TYPES)[number])) {
packages/shared/src/constants/server.ts (3)
8-23: Augment sensitive fields and note over-redaction risk for generic "key"

Consider adding common header/property variants often logged in metadata (e.g., set-cookie, x-api-key, session identifiers, CSRF/XSRF tokens).

Caution: the generic "key" may over-redact benign fields; acceptable if your sanitizer is context-aware, but worth documenting.

Apply this diff:
   'access_token',
   'refresh_token',
+  'set-cookie',
+  'x-api-key',
+  'session',
+  'session_id',
+  'csrf',
+  'x-csrf-token',
+  'xsrf',
+  'x-xsrf-token',
 ] as const;
28-35: Add If-Match and If-None-Match headers to support ETag flows

These are commonly used in optimistic concurrency and align with the new ETag service.

Apply this diff:
 export const HTTP_HEADERS = {
   ETAG: 'ETag',
+  IF_MATCH: 'If-Match',
+  IF_NONE_MATCH: 'If-None-Match',
   CONTENT_TYPE: 'Content-Type',
   AUTHORIZATION: 'Authorization',
   USER_AGENT: 'User-Agent',
   ACCEPT: 'Accept',
   CACHE_CONTROL: 'Cache-Control',
 } as const;
40-51: Export a centralized ErrorCode type for reuse across interfaces

This enables ApiError.code and ETagError.code to reference a single source of truth.

Apply this diff:
 export const ERROR_CODES = {
   NOT_FOUND: 'NOT_FOUND',
   PRECONDITION_FAILED: 'PRECONDITION_FAILED',
   ETAG_MISSING: 'ETAG_MISSING',
   NETWORK_ERROR: 'NETWORK_ERROR',
   VALIDATION_ERROR: 'VALIDATION_ERROR',
   BAD_REQUEST: 'BAD_REQUEST',
   UNAUTHORIZED: 'UNAUTHORIZED',
   FORBIDDEN: 'FORBIDDEN',
   CONFLICT: 'CONFLICT',
   INTERNAL_ERROR: 'INTERNAL_ERROR',
 } as const;
+
+export type ErrorCode = typeof ERROR_CODES[keyof typeof ERROR_CODES];
apps/lfx-pcc/src/server/services/etag.service.ts (2)
20-27: Optional: avoid logging full ETag values

ETags can reflect resource versions; logging full values isn’t necessary for diagnostics and can be noisy. Log presence only.

Suggested small change (optional):
-        etag_value: etag,
+        etag_present: Boolean(etag),
Also applies to: 97-105, 114-122

32-39: Question: treating falsy data as NOT_FOUND might misclassify valid empty payloads

If upstream returns 200 with {}, [], or null for valid cases, this throws NOT_FOUND. Consider relying on HTTP status (but proxy already throws on non-2xx) or add a stricter predicate based on expected schema.

Example (optional):

For resource objects: check for required identifiers (e.g., uid).

For collections: accept empty arrays.

If helpful, we can scan committee service responses to confirm expected shapes and adjust this predicate accordingly.
apps/lfx-pcc/src/server/services/api-client.service.ts (1)
44-61: Optional: avoid sending Content-Type for GET requests

Setting Content-Type on GETs is unnecessary and can confuse some intermediaries. If desired, only set Content-Type when a body is present.

Example minimal change (optional):
// After building headers, delete Content-Type for GET/DELETE without body
if ((method === 'GET' || method === 'DELETE') && requestInit.body == null) {
  delete (requestInit.headers as Record<string, string>)['Content-Type'];
}
Also applies to: 128-174
docs/architecture/backend/README.md (1)
146-152: Doc snippet for ETagService looks good; consider noting If-Match usage explicitly

To align with implementation and best practices, add a short note that updates/deletes must send the ETag via the If-Match header.

Example addition (optional):
Note: For conditional PUT/DELETE operations, pass the ETag using the If-Match header (not ETag).

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

MCP integration is disabled by default for public repositories
Jira integration is disabled by default for public repositories
Linear 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 a3b2a8a and bd9fd13.

📒 Files selected for processing (14)

apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.html (5 hunks)
apps/lfx-pcc/src/app/modules/project/meetings/components/meeting-form/meeting-form.component.ts (1 hunks)
apps/lfx-pcc/src/server/helpers/logger.ts (1 hunks)
apps/lfx-pcc/src/server/helpers/responder.ts (1 hunks)
apps/lfx-pcc/src/server/routes/meetings.ts (4 hunks)
apps/lfx-pcc/src/server/services/api-client.service.ts (5 hunks)
apps/lfx-pcc/src/server/services/committee.service.ts (1 hunks)
apps/lfx-pcc/src/server/services/etag.service.ts (1 hunks)
apps/lfx-pcc/src/server/services/microservice-proxy.service.ts (6 hunks)
docs/architecture/backend/README.md (2 hunks)
packages/shared/src/constants/committees.ts (1 hunks)
packages/shared/src/constants/index.ts (1 hunks)
packages/shared/src/constants/server.ts (1 hunks)
packages/shared/src/interfaces/api.ts (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

apps/lfx-pcc/src/app/modules/project/committees/committee-view/committee-view.component.html
apps/lfx-pcc/src/server/services/committee.service.ts
apps/lfx-pcc/src/server/helpers/logger.ts

🧰 Additional context used

📓 Path-based instructions (6)

{apps,packages}/**/*.{ts,tsx,js,jsx,css,scss}