Skip to content

Commit af6fa01

Browse files
manishdixitlfxclaudeCopilotCopilotasithade
authored
feat(committees): add file upload, folder hierarchy, drill-down nav (#625)
* feat(committees): add file upload, folder hierarchy, drill-down nav End-to-end document management on the committee Documents tab: File upload - Upload File button (gated by canEdit) reuses the shared lfx-file-upload component for drag-drop + picker - POST /api/committees/:id/documents/upload streams raw binary, BFF re-emits as multipart/form-data to the upstream committee service using the form-data package (now declared explicitly) - GET /api/committees/:id/documents/:documentId/download streams the file binary back to the browser with Content-Disposition Folder hierarchy + drill-down - Switched the committee Documents tab from documentService.getMyDocuments (which silently filtered out folders and never fetched files) to committeeService.getCommitteeDocuments, which now also queries the query-service for committee_document resources alongside folders + links - Root view: folders alphabetically with (N) child counts, then orphans - Click a folder name to drill in; breadcrumb returns to root - Add Link from inside a folder pre-selects that folder as the parent Toast wiring fix - Removed local MessageService provider in committee-documents.component. A local provider creates a fresh instance disconnected from the global <p-toast /> in app.component.html, which is why every Add Link / Add Folder / Upload File toast was silently dropped Action button polish - Links now render an Open button (was Download) — opens in a new tab - Files render Download wired to the new BFF download endpoint via the new MyDocumentItem.downloadUrl field (the existing /api/documents/ download proxy is for external URLs only) Better error toasts - Read err.error.error (the actual BFF response field) before falling back to err.error.message and the generic string - Translate 409 conflicts to "A document with this name already exists in this committee. Please pick a different name." Two upstream gaps remain (filed separately against lfx-v2-committee-service, assigned to Prabodh Chaudhari): 1. GET /committees/{uid}/documents to list uploaded files — replaces the BFF indexer fallback used here 2. folder_uid on POST /committees/{uid}/documents — uploaded files currently always land at the root regardless of the active folder Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(review): address PR #625 review feedback Address review comments from @github-advanced-security, @copilot-pull-request-reviewer, and @coderabbitai: - committee.controller.ts: extracted contentDispositionAttachment() helper emitting RFC 5987 (ASCII fallback + filename*=UTF-8'') so backslashes, control chars, and Unicode filenames are handled safely (per @github-advanced-security CodeQL alert and @copilot/@CodeRabbit nits) - committee.controller.ts: validate file_size query param matches the actual request body length in uploadCommitteeDocument — refuse mismatches rather than forwarding inaccurate metadata upstream (per @copilot) - committee.controller.ts + committee.service.ts: switched download from buffering the entire file to streaming. Added api-client.streamRequest() and microservice-proxy.proxyStreamRequest() that return the raw fetch Response so the controller pipes upstream.body directly to res via pipeline(). Avoids holding 100MB payloads in RAM under concurrent downloads (per @copilot) - committee.service.ts: split downloadCommitteeDocument into getCommitteeDocumentMetadata (uses committee_uid:{id} tag matching the documented indexer contract and finds the doc by uid in the response, instead of the unverified committee_document_uid tag) and getCommitteeDocumentStream (per @CodeRabbit) - committee.interface.ts: reverted CreateCommitteeDocumentType to 'link' | 'folder' (matches the JSON create endpoint validation) and introduced DocumentFormMode = CreateCommitteeDocumentType | 'file' for the dialog-mode discriminator (per @copilot) - documents-table.component.ts: omit the anchor's download attribute when navigating to a BFF downloadUrl so the server's Content-Disposition filename (the original file_name) is honored instead of being overridden by the display name (per @copilot) - committee-documents.component.ts: reset currentFolderUid to null after a successful file upload so the just-uploaded file is visible — uploads always land at the committee root (folder_uid is not yet supported on the upload endpoint, filed separately) (per @CodeRabbit) - document-form.component.html: added aria-label and title to the icon-only clear-file button for keyboard/screen-reader users (per @CodeRabbit) Resolves 9 review threads. Pushing back on 2 remaining threads (no code change): - @copilot suggested isChild=true for folder-view children — intentionally false because drilled-in children are siblings and the breadcrumb already conveys context; isChild indentation is for the root view's grouped layout - @copilot suggested adding file_size to the multipart upload payload — upstream UploadCommitteeDocumentRequestBody has no file_size field; the upstream calculates size from the binary 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(security): replace req.query cast with getStringQueryParam in uploadCommitteeDocument Use getStringQueryParam() from validation.helper for all upload query params instead of `req.query as Record<string, string>`. Express req.query values are typed string | string[] | ParsedQs | ParsedQs[]; the cast was TypeScript- only and provided no runtime protection against array injection via repeated query-string keys (e.g. ?file_size[]=100). getStringQueryParam enforces `typeof value === 'string'` at runtime and returns undefined for arrays/objects, which the existing fieldErrors guard then rejects with a ServiceValidationError. Fixes CodeQL alerts js/type-confusion-through-parameter-tampering on committee.controller.ts:694 and :697. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> Agent-Logs-Url: https://github.com/linuxfoundation/lfx-v2-ui/sessions/fa5314b8-b06b-4e9c-898b-b11c9dcf6b9e Co-authored-by: manishdixitlfx <142783321+manishdixitlfx@users.noreply.github.com> * fix(review): address PR #625 review iteration 2 Address follow-up review comments from @coderabbitai on commit 24b0405. The CodeQL alert fix landed separately as 8954620 (Copilot autofix bot). - api-client.service.ts: wrapped streamRequest's fetch() in try/catch and classify transport errors as MicroserviceError (timeout → 408, network → 500 with cause code) — mirrors the executeRequest pattern so the download path no longer degrades a timeout into a generic 500 (per @CodeRabbit) - committee.service.ts: paginate the committee_document listing via fetchAllQueryResources (matches the pattern used for committees, members, etc.). A single-page fetch silently dropped older files once a committee accumulated more than the upstream page size (per @CodeRabbit) - committee.service.ts: switched single-document metadata lookup to query by `committee_document_uid:{documentId}` tag — every committee_document is indexed with this tag per upstream CommitteeDocument.Tags(), so the query returns at most one resource. Avoids both the pagination concern (no need to walk pages) and the previous wide-then-filter pattern. Expanded the CommitteeDocumentQueryResult docstring to document all available indexer tags so future callers pick the right one (per @CodeRabbit) Resolves 3 review threads. Deferred to follow-up (response posted, thread left unresolved): - Multipart double-buffering on upload — concern is valid but the fix requires a cross-cutting api-client refactor (stream form-data via pipe() instead of getBuffer()) that affects every FormData caller. Belongs in its own PR after a sweep of upload sites. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(review): address PR #625 review feedback Substantive fixes (per @copilot-pull-request-reviewer and @manishdixitlfx): - committee.controller.ts: server-side allowlist check on upload using shared ALLOWED_FILE_TYPES + isFileTypeAllowed (frontend-side allowlist was bypassable via direct API calls); added path-traversal guard on file_name; replaced `as any` on Readable.fromWeb with typed NodeReadableStream<Uint8Array> assertion - committee.service.ts: switched two logger.warning catches to use the `err` field instead of `error: err.message` so the Pino error serializer captures the full stack trace per logging-patterns.md; updated the get_committee_documents debug log to mention files; dropped file_size from the upload FormData (upstream UploadCommitteeDocumentRequestBody schema does not declare file_size, so it was dead data — verified against committee-service openapi3.yaml) - committee-documents.component.ts: dropped redundant inner catchError in initCommitteeDocuments — the frontend committeeService already wraps with catchError(() => of([])); fixed Source filter so folder rows are excluded from source matching (folders previously fell through to source='link' which polluted the Link filter) Comment-style cleanup (per @MRashad26): - Shortened or removed verbose explanatory comments across committee.controller.ts, committee.service.ts, api-client.service.ts, committee-documents.component.ts, document-form.component.ts, and documents-table.component.ts to align with the project's "default to no comments" guidance — kept comments only where the WHY is non-obvious Net: -20 lines. Resolves the substantive Copilot threads on folder source mapping, server-side MIME validation, and the stale debug log; resolves the two err-vs-error logger threads from my own review pass; resolves all 16 of @MRashad26's verbosity comments; corroborates the existing file_size FormData pushback by removing the dead field. Pushbacks left unresolved (no code change, prior explanations stand): - isChild=false for folder-view children — drilled-in rows are siblings, indentation conveys nothing the breadcrumb doesn't already - Copilot's request to ADD file_size to FormData — upstream schema does not declare it; this commit removes the existing append for consistency 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * docs(committees): clarify file_size is BFF-only and add folder_uid TODOs - Expand JSDoc on UploadCommitteeDocumentRequest.file_size to make it explicit that the field is BFF-only (used for request body validation only; not forwarded to upstream which has no file_size in its schema) - Add optional folder_uid field to UploadCommitteeDocumentRequest with a TODO comment for when upstream adds support (LFXV2-1632) - Add matching TODO comment in committee.service.ts where the FormData is assembled to append folder_uid once upstream accepts it Signed-off-by: copilot <copilot@github.com> Agent-Logs-Url: https://github.com/linuxfoundation/lfx-v2-ui/sessions/fb2bbf75-72bf-4ae7-99ce-81ae9471ea70 Co-authored-by: manishdixitlfx <142783321+manishdixitlfx@users.noreply.github.com> * fix(review): trim upload names; classify all stream errors Address review comments from @coderabbitai: - committee.controller.ts: trim `name` and `file_name` query params before required-field validation so whitespace-only strings (e.g. `" "`) fail. Use the trimmed values for the MIME allowlist check, the path-traversal guard, the uploadData payload, and the success log so upstream receives the actual stored value. - api-client.service.ts: add a final-fallback `MicroserviceError(500, NETWORK_ERROR)` in `streamRequest`'s catch block. Previously, an Error instance without `cause.code` was rethrown raw, bypassing typed server-side error handling. Every transport failure on the streaming path is now a typed MicroserviceError, matching `executeRequest`. Resolves 2 review threads. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Signed-off-by: Manish Dixit <142783321+manishdixitlfx@users.noreply.github.com> * perf(ssr): route SSR API calls over loopback (#632) * perf(ssr): route SSR API calls over loopback (LFXV2-1634) Add ssrBaseUrlInterceptor that rewrites relative /api/* and /public/api/* URLs to http://127.0.0.1:$PORT when running on the server platform, so SSR talks to the in-process Express server directly instead of routing through Cloudflare/Traefik via the public hostname. Browser behavior is unchanged. Datadog trace b40ef136d08d7f7740c34d90b0ddf006 showed an SSR-emitted POST /public/api/meetings/:id/join-url taking 160 seconds; the actual upstream meeting service responded in <200ms. The remaining time was Cloudflare/LB/connection-queueing on the SSR's call to itself via app.lfx.dev. Loopback removes that round trip. Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(review): address PR #632 review feedback Address review comments from copilot-pull-request-reviewer, coderabbitai: - app.config.ts: reorder interceptors so authenticationInterceptor runs before ssrBaseUrlInterceptor. authentication only attaches cookies when req.url starts with /api/ or /public/api/, so it must see the original relative URL before the SSR rewrite to loopback (per copilot-pull-request-reviewer, coderabbitai) - ssr-base-url.interceptor.ts: add JSDoc above the exported interceptor to match the convention in authentication.interceptor.ts (per coderabbitai) Resolves 3 review threads. Signed-off-by: Asitha de Silva <asithade@gmail.com> --------- Signed-off-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * perf(ssr): defer meeting join URL fetch to client (LFXV2-1634) (#634) Skip the POST /public/api/meetings/:id/join-url call during SSR. The Zoom join URL is only consumed by window.open in the browser, so fetching it server-side adds upstream-call latency to TTFB without producing any user-visible content. Datadog trace b40ef136d08d7f7740c34d90b0ddf006 showed this fetch blocking SSR until the upstream resolved. PR #632 fixed the routing of the call (loopback); this change removes it from the SSR critical path entirely. Signed-off-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(dashboard): add info-icon tooltip to ED metric cards (#633) * fix(dashboard): add info-icon tooltip to ED metric cards Methodology tooltips were only visible on full-card hover with no visual indicator. Add an optional infoTooltip input to metric-card that renders a small (i) icon next to the title. Wire it up for all ED marketing overview cards so users can discover calculation details. Addresses Mazin's feedback that Flywheel Conversion is "opaque, no calc shown" (Confluence item #13). LFXV2-1623 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> * fix(dashboard): make info tooltip keyboard-accessible Use a button element instead of bare <i> for the info icon so it is focusable and has an aria-label. Stop click propagation to prevent triggering the card click handler. LFXV2-1623 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> --------- Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(dashboard): label health scores as "projects scored" (#631) * fix(dashboard): label health scores as "projects scored" Health score buckets (4) can differ from Total Projects (3) because not all projects have a Crowd.dev health score. Add "N projects scored" subtitle on the card and update the drawer badge to say "projects scored" so users don't confuse it with total project count. LFXV2-1618 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> * fix(dashboard): update badge comment to match new copy LFXV2-1618 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> --------- Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(dashboard): add past/upcoming tag to event growth drawer rows (#630) * fix(dashboard): add past/upcoming tag to event growth drawer rows Event rows in the Event Growth drawer mixed realized and forecast revenue without visual distinction. Add a "Past" or "Upcoming" tag next to each event name, computed from the event date. Helps users understand whether revenue is realized or committed. LFXV2-1622 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> * fix(dashboard): address review feedback on event past/upcoming tags Guard isPast for empty event dates, update JSDoc to reflect isPast field, and add min-w-0/truncate to prevent long event names from breaking column alignment. LFXV2-1622 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> --------- Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(dashboard): clarify events change indicator with past/upcoming split (#629) The change indicator showed "+10 vs prev period" without context, misleading users when most events are upcoming. Now appends a breakdown "(2 past · 8 upcoming)" when upcoming events exist so the total is transparent. LFXV2-1626 Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(dashboard): cap sponsorship percentage label at 999% (#628) The progress bar already caps at 100% visually, but the text label displayed the raw uncapped value (e.g. "61365% of goal") when the Snowflake goal target is misconfigured. Cap the label at ">999%" to prevent misleading numbers while the upstream data is corrected. LFXV2-1619 Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(dashboard): align engaged community channel names with chart labels (#627) The card description and tooltip listed "Slack, Discord, GitHub, mailing lists" but the Community Breakdown chart shows "Community, Working Groups, Newsletter, Certified". Update both strings to match the actual data channels. LFXV2-1623 Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(meetings): match registrant by username for restricted join (#636) * fix(meetings): match registrant by username for join (LFXV2-1634) Public meeting join and "my RSVP" lookups now identify the user by their registrant rather than just their auth email. Resolves the case where an account has multiple emails (auth-provider primary differs from invited registrant email), which previously caused a false "not registered" denial on join and a missing RSVP on the meeting page. - Restricted-meeting access check matches by username first, falling back to email; the matched registrant's stored email is used for the upstream join_link call so Zoom resolves to the correct invitee. - Current-user RSVP lookup resolves the registrant via email-or- username, then filters RSVPs by registrant_id — RSVP records carry registrant_id reliably, while their username field is often null. Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(review): address PR #636 review feedback Address review comments from copilot[bot], coderabbitai: - meeting.service.ts: switched RSVP lookup from getUsernameFromAuth to getEffectiveUsername — the former returns the OIDC `sub` (e.g. `auth0|<id>`) for non-Authelia sessions, the latter returns the LFID nickname that matches `registrant.username` (per copilot[bot]) - meeting.service.ts: filter RSVPs by a Set of all matching registrant UIDs rather than only registrants[0], so users with occurrence- specific invites or multiple registrant rows for the same meeting see all their RSVPs (per copilot[bot], coderabbitai) - public-meeting.controller.ts: identity-neutral validation and authorization error messages — failures via the username path no longer claim the problem was a missing or unmatched email address (per copilot[bot], coderabbitai) Resolves 6 review threads. Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(meetings): match restricted-join denial by error code, not message The previous commit changed the restricted-meeting denial message to identity-neutral wording, which broke the meeting-join UI's alternate- email affordance — meeting-join.component.ts only showed the "Click here to join using a different email address" link when the error message contained the substring "email address is not registered for this restricted meeting". Replace the brittle substring match with a stable error code: - AuthorizationError now accepts an optional `code` override so call sites can emit a more specific code than the generic AUTHORIZATION_REQUIRED. - The restricted-meeting denial path now throws with code: 'NOT_REGISTERED_FOR_MEETING'. - meeting-join.component.ts captures `error.error.code` from the API response into a new joinUrlErrorCode signal and matches the email- fallback affordance on that code instead of the message text. Signed-off-by: Asitha de Silva <asithade@gmail.com> --------- Signed-off-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * perf(observability): tag APM spans with matched Express route (#635) * perf(observability): tag APM spans with Express route (LFXV2-1637) Add an applyCustomAttributesOnSpan hook on HttpInstrumentation that sets http.route to req.baseUrl + req.route.path and updates the span name to METHOD ROUTE. Datadog uses these to compute resource_name, so spans now bucket by endpoint (e.g. GET /api/meetings/:id) instead of bare HTTP method. For SSR catch-all routes where req.route isn't set, fall back to bucketing by first URL segment (e.g. /meetings/*) so SSR latency is no longer lumped into a single GET bucket. Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(observability): bucket root SSR requests under route '/' Address PR #635 review feedback from coderabbitai, copilot[bot]: - otel.mjs: handle root path in SSR fallback so http.route is set to '/' when URL has no segments. Previously, requests to '/' left http.route unset and landed in the bare GET resource bucket — the exact problem this PR set out to fix. The root dashboard route (app.routes.ts path: '') is real SSR traffic, so this matters. Resolves 2 review threads. Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(review): address PR #635 review feedback Address review comments from copilot-pull-request-reviewer: - otel.mjs: skip pure wildcard route paths ('**', '*', '/*', '/**') so static-asset fallthrough does not collapse SSR spans into GET ** (per copilot[bot]) - otel.mjs: strip trailing slash from concatenated routes so router.get('/') mounts (e.g. /api/meetings/) bucket as their canonical path (per copilot[bot]) - otel.mjs: bucket single-segment fallback URLs as exact paths (e.g. /login) instead of /login/*, since they are concrete endpoints not prefixes (per copilot[bot]) Resolves 4 review threads. Signed-off-by: Asitha de Silva <asithade@gmail.com> --------- Signed-off-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(dashboards): rename marketing metrics heading to marketing overview (#637) * fix(dashboards): rename Marketing Metrics heading to Marketing Overview Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> * fix(dashboards): clarify interface name vs UI label in JSDoc comments Address review feedback: add note that MarketingMetrics* interface names are retained for backwards compatibility while the UI heading has been renamed to Marketing Overview. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> --------- Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * feat(sidebar): use best_match sort for project search (#638) * feat(sidebar): use best_match sort for project search LFXV2-1639 Switch sidebar lens-items query to upstream best_match sort whenever the user supplies a name term. Default alphabetical ordering is preserved for empty-search browsing. Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(review): address PR #638 review feedback Address review comments from copilot[bot], @dealako: - apps/lfx-one/src/server/services/navigation.service.ts: trim `name` once in buildQuery — whitespace-only values no longer flip sort to best_match or forward a meaningless query upstream (per copilot[bot], confirmed by @dealako) Resolves 1 review thread. Signed-off-by: Asitha de Silva <asithade@gmail.com> --------- Signed-off-by: Asitha de Silva <asithade@gmail.com> Co-authored-by: David Deal <ddeal@linuxfoundation.org> Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * docs: add permission persona navigation preread Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * feat(observability): add UX readiness health check endpoint (#639) Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * docs: add markdown permission persona preread Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> * fix(review): address PR #625 review feedback Address review comments from @copilot-pull-request-reviewer and self-flagged nits: - committee.service.ts:741: rewrite misleading pagination comment to describe fetchAllQueryResources behavior accurately (per @copilot) - committee.service.ts:960 (getCommitteeDocumentMetadata): scope query with tags_all=[committee_document_uid, committee_uid] to prevent cross-committee metadata leakage on a guessed/leaked document UID (per @copilot) - microservice-proxy.service.ts: remove dead error-wrapping branch in proxyStreamRequest — apiClient.streamRequest throws MicroserviceError with statusCode (not status), so the wrapper was unreachable; bare rethrow is correct (per @copilot) - committee-documents.component.ts: drop unused projectUid arg from toDisplayItem and leave foundationUid undefined for committee documents (project_uid may be a sub-project, not a foundation) (per @copilot) - documents-table.component.html: add aria-hidden=true to decorative folder/file icons (per @copilot) - committee.interface.ts:549: drop the @see LFXV2-1632 reference on the folder_uid TODO — that ticket is the streaming-uploads refactor, not the folder_uid upstream gap (self-flagged) - docs/architecture/frontend/permission-persona-navigation-model-preread.md: prettier formatting fix to unblock pre-commit format:check Resolves 7 review threads. Two threads remain unresolved pending human review (isChild=false in folder view, file_size removed from upstream multipart) — pushback responses already posted. Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> --------- Signed-off-by: Manish Dixit <mdixit@linuxfoundation.org> Signed-off-by: Manish Dixit <142783321+manishdixitlfx@users.noreply.github.com> Signed-off-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Misha Rautela <mrautela@linuxfoundation.org> Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Asitha de Silva <asithade@gmail.com> Co-authored-by: Misha Rautela <mrautela@linuxfoundation.org> Co-authored-by: David Deal <ddeal@linuxfoundation.org>
1 parent 36d7493 commit af6fa01

18 files changed

Lines changed: 1174 additions & 56 deletions

apps/lfx-one/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
"express": "^4.18.2",
5858
"express-openid-connect": "^2.19.2",
5959
"express-rate-limit": "^8.3.2",
60+
"form-data": "^4.0.4",
6061
"html-to-image": "^1.11.13",
6162
"launchdarkly-js-client-sdk": "^3.9.0",
6263
"marked": "^17.0.4",

apps/lfx-one/src/app/modules/committees/components/committee-documents/committee-documents.component.html

Lines changed: 26 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,14 @@
44
<div class="flex flex-col gap-4 pt-2" data-testid="committee-documents-tab">
55
@if (canEdit()) {
66
<div class="flex items-center justify-end gap-2">
7+
<lfx-button
8+
label="Upload File"
9+
icon="fa-light fa-arrow-up-from-line"
10+
size="small"
11+
severity="secondary"
12+
[outlined]="true"
13+
(onClick)="openUploadFileDialog()"
14+
data-testid="committee-documents-upload-file-button"></lfx-button>
715
<lfx-button
816
label="New Folder"
917
icon="fa-light fa-folder-plus"
@@ -23,6 +31,22 @@
2331
}
2432

2533
<lfx-card data-testid="committee-documents-card">
34+
<!-- Breadcrumb (only shown when drilled into a folder) -->
35+
@if (currentFolder(); as folder) {
36+
<nav class="flex items-center gap-2 pb-3 mb-3 border-b border-gray-100 text-sm" data-testid="committee-documents-breadcrumb" aria-label="Breadcrumb">
37+
<button
38+
type="button"
39+
class="text-blue-600 hover:text-blue-700 hover:underline font-medium bg-transparent border-none cursor-pointer p-0"
40+
(click)="onBreadcrumbHome()"
41+
data-testid="committee-documents-breadcrumb-home">
42+
<i class="fa-light fa-folder-open text-amber-500 mr-1"></i>
43+
Documents
44+
</button>
45+
<i class="fa-light fa-chevron-right text-gray-300 text-xs"></i>
46+
<span class="text-gray-900 font-medium" data-testid="committee-documents-breadcrumb-current">{{ folder.name }}</span>
47+
</nav>
48+
}
49+
2650
<!-- Filters -->
2751
<div class="pb-3 mb-3 border-b border-gray-100">
2852
<div class="flex flex-wrap items-center gap-2">
@@ -60,8 +84,9 @@
6084
[documents]="filteredDocuments()"
6185
[loading]="loading()"
6286
[showFoundation]="false"
87+
(folderOpen)="onFolderOpen($event)"
6388
testIdPrefix="committee-documents"
64-
emptyMessage="No documents found for this group">
89+
[emptyMessage]="currentFolder() ? 'This folder is empty' : 'No documents found for this group'">
6590
</lfx-documents-table>
6691
</lfx-card>
6792
</div>

apps/lfx-one/src/app/modules/committees/components/committee-documents/committee-documents.component.ts

Lines changed: 150 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -12,26 +12,25 @@ import { SelectComponent } from '@components/select/select.component';
1212
import { MEETING_GROUP_SOURCES } from '@lfx-one/shared/constants';
1313
import { Committee, CommitteeDocument, MyDocumentItem, MyDocumentSource } from '@lfx-one/shared/interfaces';
1414
import { CommitteeService } from '@services/committee.service';
15-
import { DocumentService } from '@services/document.service';
16-
import { MessageService } from 'primeng/api';
1715
import { DialogService, DynamicDialogRef } from 'primeng/dynamicdialog';
18-
import { catchError, combineLatest, debounceTime, distinctUntilChanged, filter, finalize, map, of, startWith, switchMap, take } from 'rxjs';
16+
import { combineLatest, debounceTime, distinctUntilChanged, filter, finalize, map, startWith, switchMap, take } from 'rxjs';
1917

2018
import { DocumentFormComponent } from '../document-form/document-form.component';
2119

2220
@Component({
2321
selector: 'lfx-committee-documents',
2422
imports: [ButtonComponent, CardComponent, InputTextComponent, SelectComponent, DocumentsTableComponent, ReactiveFormsModule],
25-
providers: [DialogService, MessageService],
23+
// NOTE: Do NOT provide MessageService here. It's already provided at root and a single
24+
// instance must back the global <p-toast /> in app.component.html. A local provider
25+
// creates a fresh instance whose messages never reach the root toast outlet.
26+
providers: [DialogService],
2627
templateUrl: './committee-documents.component.html',
2728
changeDetection: ChangeDetectionStrategy.OnPush,
2829
})
2930
export class CommitteeDocumentsComponent {
3031
// === Services ===
31-
private readonly documentService = inject(DocumentService);
3232
private readonly committeeService = inject(CommitteeService);
3333
private readonly dialogService = inject(DialogService);
34-
private readonly messageService = inject(MessageService);
3534

3635
// === Inputs ===
3736
public readonly committee = input.required<Committee>();
@@ -46,22 +45,25 @@ export class CommitteeDocumentsComponent {
4645
// === Writable Signals ===
4746
protected readonly loading = signal<boolean>(true);
4847
protected readonly refreshTrigger = signal<number>(0);
48+
/** UID of the folder the user has drilled into; null means the root view. */
49+
protected readonly currentFolderUid = signal<string | null>(null);
4950

5051
// === Static Options ===
5152
protected readonly sourceOptions: { label: string; value: MyDocumentSource | null }[] = [
5253
{ label: 'All Sources', value: null },
5354
{ label: 'Link', value: 'link' },
54-
{ label: 'Meeting', value: 'meeting' },
55-
{ label: 'Mailing List', value: 'mailing_list' },
55+
{ label: 'File', value: 'file' },
5656
];
5757

5858
// === Computed Signals ===
5959
protected readonly searchQuery: Signal<string> = this.initSearchQuery();
6060
protected readonly sourceFilter: Signal<MyDocumentSource | null> = this.initSourceFilter();
61+
protected readonly committeeDocuments: Signal<CommitteeDocument[]> = this.initCommitteeDocuments();
6162
protected readonly documents: Signal<MyDocumentItem[]> = this.initDocuments();
6263
protected readonly filteredDocuments: Signal<MyDocumentItem[]> = this.initFilteredDocuments();
63-
protected readonly committeeDocuments: Signal<CommitteeDocument[]> = this.initCommitteeDocuments();
6464
protected readonly folderOptions: Signal<{ label: string; value: string }[]> = this.initFolderOptions();
65+
/** The folder the user has drilled into, used by the breadcrumb. Null when at root. */
66+
protected readonly currentFolder: Signal<CommitteeDocument | null> = this.initCurrentFolder();
6567

6668
// === Public Methods ===
6769
public openAddLinkDialog(): void {
@@ -74,6 +76,8 @@ export class CommitteeDocumentsComponent {
7476
mode: 'link',
7577
committeeId: this.committee().uid,
7678
folders: this.folderOptions(),
79+
// Pre-select the folder the user is currently inside so the new link lands here
80+
defaultParentUid: this.currentFolderUid(),
7781
},
7882
});
7983

@@ -86,6 +90,19 @@ export class CommitteeDocumentsComponent {
8690
});
8791
}
8892

93+
/** Drill into a folder shown in the table — switches the view to that folder's contents. */
94+
public onFolderOpen(doc: MyDocumentItem): void {
95+
const folderUid = doc.id.startsWith('committee_folder:') ? doc.id.slice('committee_folder:'.length) : null;
96+
if (folderUid) {
97+
this.currentFolderUid.set(folderUid);
98+
}
99+
}
100+
101+
/** Reset the breadcrumb to the root view. */
102+
public onBreadcrumbHome(): void {
103+
this.currentFolderUid.set(null);
104+
}
105+
89106
public openNewFolderDialog(): void {
90107
const dialogRef: DynamicDialogRef | null = this.dialogService.open(DocumentFormComponent, {
91108
header: 'New Folder',
@@ -107,6 +124,29 @@ export class CommitteeDocumentsComponent {
107124
});
108125
}
109126

127+
public openUploadFileDialog(): void {
128+
const dialogRef: DynamicDialogRef | null = this.dialogService.open(DocumentFormComponent, {
129+
header: 'Upload File',
130+
width: '560px',
131+
modal: true,
132+
closable: true,
133+
data: {
134+
mode: 'file',
135+
committeeId: this.committee().uid,
136+
},
137+
});
138+
139+
dialogRef?.onClose.pipe(take(1)).subscribe({
140+
next: (result: boolean | undefined) => {
141+
if (result) {
142+
// Upload API doesn't accept folder_uid yet — pop to root so the new file is visible.
143+
this.currentFolderUid.set(null);
144+
this.refreshTrigger.update((v) => v + 1);
145+
}
146+
},
147+
});
148+
}
149+
110150
// === Private Initializers ===
111151
private initSearchQuery(): Signal<string> {
112152
return toSignal(
@@ -124,19 +164,64 @@ export class CommitteeDocumentsComponent {
124164
return toSignal(this.filterForm.controls.source.valueChanges.pipe(startWith<MyDocumentSource | null>(null)), { initialValue: null });
125165
}
126166

167+
/**
168+
* Derives the displayed `MyDocumentItem[]` from `committeeDocuments()` (the canonical
169+
* committee-scoped fetch backed by upstream `/folders`, `/links`, and indexed
170+
* `committee_document` resources). Previously this used `documentService.getMyDocuments()`
171+
* which queries the `committee_link` indexer type and explicitly filters out items without
172+
* a URL — that silently dropped folders and never included uploaded files.
173+
*
174+
* The list is scoped by `currentFolderUid()` for drill-down navigation:
175+
* - **Root view (currentFolderUid === null):** show folders (alphabetically) and any orphan
176+
* items (no parent or parent deleted). Folders display a child count and are clickable.
177+
* - **Inside a folder (currentFolderUid set):** show only items whose `parent_uid` matches.
178+
*/
127179
private initDocuments(): Signal<MyDocumentItem[]> {
128-
return toSignal(
129-
combineLatest([toObservable(this.committee), toObservable(this.refreshTrigger)]).pipe(
130-
switchMap(([committee]) => {
131-
this.loading.set(true);
132-
return this.documentService.getMyDocuments(committee.project_uid, committee.uid).pipe(
133-
catchError(() => of([] as MyDocumentItem[])),
134-
finalize(() => this.loading.set(false))
135-
);
136-
})
137-
),
138-
{ initialValue: [] as MyDocumentItem[] }
139-
);
180+
return computed(() => {
181+
const committee = this.committee();
182+
const docs = this.committeeDocuments();
183+
const committeeUid = committee?.uid;
184+
const groupName = committee?.name ?? '';
185+
const currentFolderUid = this.currentFolderUid();
186+
187+
const folders = docs.filter((d) => d.type === 'folder');
188+
const nonFolders = docs.filter((d) => d.type !== 'folder');
189+
const folderUids = new Set(folders.map((f) => f.uid));
190+
191+
// Folder view — show only direct children of the selected folder.
192+
if (currentFolderUid) {
193+
return nonFolders
194+
.filter((d) => d.parent_uid === currentFolderUid)
195+
.sort((a, b) => a.name.localeCompare(b.name))
196+
.map((d) => this.toDisplayItem(d, committeeUid, groupName, false));
197+
}
198+
199+
// Root view — folders (with child counts) followed by orphan items.
200+
const childCountByFolder = new Map<string, number>();
201+
for (const item of nonFolders) {
202+
if (item.parent_uid && folderUids.has(item.parent_uid)) {
203+
childCountByFolder.set(item.parent_uid, (childCountByFolder.get(item.parent_uid) ?? 0) + 1);
204+
}
205+
}
206+
207+
const ordered: MyDocumentItem[] = [];
208+
const sortedFolders = [...folders].sort((a, b) => a.name.localeCompare(b.name));
209+
for (const folder of sortedFolders) {
210+
ordered.push({
211+
...this.toDisplayItem(folder, committeeUid, groupName, false),
212+
isFolder: true,
213+
childCount: childCountByFolder.get(folder.uid) ?? 0,
214+
});
215+
}
216+
217+
// Orphans appear at the bottom of the root view so newly uploaded files don't disappear.
218+
const orphans = nonFolders.filter((d) => !d.parent_uid || !folderUids.has(d.parent_uid)).sort((a, b) => a.name.localeCompare(b.name));
219+
for (const orphan of orphans) {
220+
ordered.push(this.toDisplayItem(orphan, committeeUid, groupName, false));
221+
}
222+
223+
return ordered;
224+
});
140225
}
141226

142227
private initFilteredDocuments(): Signal<MyDocumentItem[]> {
@@ -148,7 +233,8 @@ export class CommitteeDocumentsComponent {
148233
if (query && !doc.name.toLowerCase().includes(query) && !(doc.groupOrMeetingName ?? '').toLowerCase().includes(query)) {
149234
return false;
150235
}
151-
if (source && doc.source !== source && !(source === 'meeting' && MEETING_GROUP_SOURCES.includes(doc.source))) {
236+
// Folder rows are structural navigation, not content — never filter them out by source.
237+
if (source && !doc.isFolder && doc.source !== source && !(source === 'meeting' && MEETING_GROUP_SOURCES.includes(doc.source))) {
152238
return false;
153239
}
154240
return true;
@@ -160,17 +246,58 @@ export class CommitteeDocumentsComponent {
160246
return toSignal(
161247
combineLatest([toObservable(this.committee), toObservable(this.refreshTrigger)]).pipe(
162248
filter(([committee]) => !!committee?.uid),
163-
switchMap(([committee]) => this.committeeService.getCommitteeDocuments(committee.uid))
249+
switchMap(([committee]) => {
250+
this.loading.set(true);
251+
return this.committeeService.getCommitteeDocuments(committee.uid).pipe(finalize(() => this.loading.set(false)));
252+
})
164253
),
165254
{ initialValue: [] as CommitteeDocument[] }
166255
);
167256
}
168257

258+
/**
259+
* Maps an upstream-shaped CommitteeDocument to the unified MyDocumentItem the table renders.
260+
* Folders use source `'link'` as a placeholder (the row is rendered via `isFolder` and
261+
* skipped by the Source filter — see `initFilteredDocuments`). Files get a `downloadUrl`
262+
* pointing at the BFF streaming endpoint.
263+
*/
264+
private toDisplayItem(doc: CommitteeDocument, committeeUid: string | undefined, groupName: string, isChild: boolean): MyDocumentItem {
265+
const isFile = doc.type === 'file';
266+
const ownerCommitteeUid = committeeUid ?? doc.committee_uid ?? '';
267+
return {
268+
id: `committee_${doc.type}:${doc.uid}`,
269+
name: doc.name,
270+
source: (isFile ? 'file' : 'link') as MyDocumentSource,
271+
foundationName: '',
272+
// Intentionally undefined for committee documents — committee.project_uid may be a
273+
// sub-project, not a foundation, so populating this would mislabel any downstream
274+
// consumer that uses it for foundation-scoped routing. The column is hidden here.
275+
foundationUid: undefined,
276+
groupOrMeetingName: groupName,
277+
groupOrMeetingUid: ownerCommitteeUid,
278+
date: doc.created_at ?? doc.updated_at ?? '',
279+
url: doc.url,
280+
attachmentUid: isFile ? doc.uid : undefined,
281+
fileType: doc.mime_type,
282+
parentUid: doc.parent_uid,
283+
isChild,
284+
downloadUrl: isFile && ownerCommitteeUid ? `/api/committees/${ownerCommitteeUid}/documents/${doc.uid}/download` : undefined,
285+
};
286+
}
287+
169288
private initFolderOptions(): Signal<{ label: string; value: string }[]> {
170289
return computed(() =>
171290
this.committeeDocuments()
172291
.filter((doc) => doc.type === 'folder')
173292
.map((folder) => ({ label: folder.name, value: folder.uid }))
174293
);
175294
}
295+
296+
private initCurrentFolder(): Signal<CommitteeDocument | null> {
297+
return computed(() => {
298+
const uid = this.currentFolderUid();
299+
if (!uid) return null;
300+
return this.committeeDocuments().find((doc) => doc.type === 'folder' && doc.uid === uid) ?? null;
301+
});
302+
}
176303
}

0 commit comments

Comments
 (0)