feat(newsletters): add recipient-facing my newsletters archive - #1154
feat(newsletters): add recipient-facing my newsletters archive#1154niravpatel27 wants to merge 10 commits into
Conversation
Implement the My Newsletters feature under the Me lens, allowing users to view sent newsletters targeted at committees they belong to. Includes BFF service layer with project enrichment, Angular component with foundation filtering, and sidebar navigation integration. - Shared: MyNewsletterListItem and MyNewsletterArchiveResponse interfaces - Backend: my-newsletters service, controller, and route with project enrichment - Frontend: my-newsletters-list component with foundation filtering and detail drawer - Newsletter service: new listMyNewsletters() and getMyNewsletterDetail() methods - Sidebar: "My Newsletters" entry in My Engagement section Signed-off-by: Nirav Patel <npatel@linuxfoundation.org>
- Removed dead-code unused `newsletters` signal (was redundant with allNewsletters) - Converted clickable div rows to keyboard-accessible button elements with Enter/Space handlers and aria-label - Fixed logger operation name dedup violation in getArchiveDetail service method (moved from startOperation to debug/warning logs per pattern) - Typed projects array properly as (Project | null)[] instead of (any | null)[] - Applied prettier formatting - Verified: silent error handling matches Me-lens component pattern - Verified: signal<boolean> with [(visible)] binding matches newsletter-list.component pattern Signed-off-by: Nirav Patel <npatel@linuxfoundation.org>
Add comprehensive vitest suite covering: - Empty committee set short-circuit without upstream call - Happy path newsletter fetching and enrichment - Project/foundation resolution (parent_uid and self-foundation cases) - Project lookup failures with graceful degradation - Batch fetching in groups of 25 - Error propagation (403/404 from archive endpoints) Also enhance enrichNewslettersWithProjectData to fetch parent_uid foundations in addition to project UIDs, ensuring complete enrichment. Signed-off-by: Nirav Patel <npatel@linuxfoundation.org>
Replace raw query parameter cast `(req.query['page_token'] as string)` with getStringQueryParam(req, 'page_token') helper to safely handle query string casting and prevent type confusion from repeated keys. Fixes KB pattern violation: raw-query-string-cast Signed-off-by: Nirav Patel <npatel@linuxfoundation.org>
Run prettier to fix formatting issues in spec file. Signed-off-by: Nirav Patel <npatel@linuxfoundation.org>
PR SummaryMedium Risk Overview The BFF exposes The Angular list uses signals for loading, empty state, foundation filter pills, load-more, and lazy detail fetch into the existing Reviewed by Cursor Bugbot for commit 410acaa. Bugbot is set up for automated code reviews on this repo. Configure here. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughAdds an authenticated “My Newsletters” archive with paginated retrieval, project and foundation enrichment, filtering, preview details, navigation, server endpoints, and service tests. ChangesMy Newsletters Archive
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant MyNewslettersListComponent
participant NewsletterService
participant MyNewslettersController
participant MyNewslettersService
User->>MyNewslettersListComponent: Open My Newsletters
MyNewslettersListComponent->>NewsletterService: listMyNewsletters(pageToken)
NewsletterService->>MyNewslettersController: GET archive endpoint
MyNewslettersController->>MyNewslettersService: listArchive(request, pageToken)
MyNewslettersService-->>MyNewslettersController: Enriched archive response
MyNewslettersController-->>NewsletterService: JSON response
NewsletterService-->>MyNewslettersListComponent: Newsletter list
MyNewslettersListComponent-->>User: Render filtered newsletter cards
User->>MyNewslettersListComponent: Select newsletter
MyNewslettersListComponent->>NewsletterService: getMyNewsletterDetail(newsletterUid)
NewsletterService-->>MyNewslettersListComponent: Newsletter body
MyNewslettersListComponent-->>User: Show preview drawer
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed. For unrecoverable errors, disable the tool in CodeRabbit configuration. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
1. HIGH: Move /my-newsletters to top-level Me-lens route with authGuard only, removing it from foundation/newsletters children. This bypasses the parent newsletterAccessGuard + projectQueryParamGuard that was blocking access. Update sidebar link from /newsletters/my-newsletters to /my-newsletters. 2. loadMore: Check guards before bumping generation counter, and ensure loadingMore flag is cleared even when stale responses are discarded. Prevents loading state from getting stuck on concurrent requests. 3. Controller param validation: Move UUID validation inside try/catch block so Express routes validation errors to next() rather than hanging the request. Validation throw happens before the handler's request/response boundary. 4. Foundation filter: Make filter pills reversible by toggling on reclick. Clicking an already-selected pill now clears selection (show "all"), matching FilterPillsComponent idiom. 5. Native button: Remove redundant (keydown.enter)/(keydown.space) handlers. Native <button> type="button" handles Enter and Space automatically. Handlers cause double-firing of preview requests. 6. Date pipe: Pin locale to 'en-US' to prevent SSR hydration mismatch. Change from `| date: 'short'` to `| date: 'MMM d, y': 'en-US'` to match repo's SSR-safe date formatting pattern. Signed-off-by: Nirav Patel <npatel@linuxfoundation.org>
Signed-off-by: Nirav Patel <npatel@linuxfoundation.org>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
apps/lfx-one/src/server/services/my-newsletters.service.ts (2)
78-98: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUnexpected upstream failures logged only at
warning, noterror.
getArchiveDetail's catch block always logs atwarninglevel, even for genuinely unexpected failures (5xx/network errors), per the docstring "logs warnings for expected and unexpected failures" and confirmed by testlogs unexpected errors at warning level. This is intentional, but it means real outages in this path won't be distinguishable from expected 403/404s by log level, which could reduce alerting visibility if alerts are keyed offerror-level logs.Consider branching on
errorstatus/type to log unexpected failures (non-403/404) aterrorlevel while keeping 403/404 atwarning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/server/services/my-newsletters.service.ts` around lines 78 - 98, Update the catch block in getArchiveDetail to distinguish expected 403/404 upstream responses from unexpected failures such as 5xx or network errors; retain warning logging for 403/404 and use error-level logging for all other failures, preserving the existing context and rethrow behavior.
137-160: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDuplicate foundation lookups when a foundation project is also directly referenced.
The second-pass
parentUidscollection isn't filtered against UIDs already fetched in the first pass (projectUids), so any foundation that is both aparent_uidand a directly-referencedproject_uidgets fetched twice viaprojectService.getProjectByIdon every archive request.♻️ Proposed fix to skip already-fetched UIDs
const parentUids = Array.from( new Set( projects .filter((p): p is Project => p !== null) .map((p) => p.parent_uid) - .filter((uid): uid is string => uid !== undefined && uid !== null) + .filter((uid): uid is string => uid !== undefined && uid !== null && !projectUids.includes(uid)) ) );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/server/services/my-newsletters.service.ts` around lines 137 - 160, Update the parent UID collection in the second-pass flow to exclude any UIDs already present in the first-pass projectUids set before batching and calling projectService.getProjectById. Preserve collection of unique non-null parent_uid values and leave fetching for genuinely new parent projects unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.html`:
- Around line 36-43: Remove the explicit keydown.enter and keydown.space
bindings from the newsletter preview button while retaining its click binding to
openPreview(newsletter). Keep the button’s existing accessibility attributes and
styling unchanged.
In
`@apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.ts`:
- Around line 101-116: Update the catchError fallback in the listMyNewsletters
load-more flow to preserve the current token by returning it as next_page_token
instead of undefined. Keep the existing newsletter fallback and response
handling unchanged so transient failures leave the Load more control available
for retry.
In `@apps/lfx-one/src/server/controllers/my-newsletters.controller.ts`:
- Around line 47-56: Move the newsletterUid presence and UUID validation in
getArchiveDetail inside its existing try block so ServiceValidationError is
handled by the method’s local catch and forwarded through Express error
handling. Preserve the current validation message and operation metadata.
---
Nitpick comments:
In `@apps/lfx-one/src/server/services/my-newsletters.service.ts`:
- Around line 78-98: Update the catch block in getArchiveDetail to distinguish
expected 403/404 upstream responses from unexpected failures such as 5xx or
network errors; retain warning logging for 403/404 and use error-level logging
for all other failures, preserving the existing context and rethrow behavior.
- Around line 137-160: Update the parent UID collection in the second-pass flow
to exclude any UIDs already present in the first-pass projectUids set before
batching and calling projectService.getProjectById. Preserve collection of
unique non-null parent_uid values and leave fetching for genuinely new parent
projects unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c7b022fa-d2c5-4a4a-bad9-fac961531d2a
📒 Files selected for processing (13)
apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.htmlapps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.scssapps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.tsapps/lfx-one/src/app/modules/newsletters/newsletters.routes.tsapps/lfx-one/src/app/shared/services/newsletter.service.tsapps/lfx-one/src/app/shared/services/sidebar-nav.service.tsapps/lfx-one/src/server/controllers/my-newsletters.controller.tsapps/lfx-one/src/server/routes/my-newsletters.route.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/services/my-newsletters.service.spec.tsapps/lfx-one/src/server/services/my-newsletters.service.tsapps/lfx-one/src/server/services/newsletter-service.client.tspackages/shared/src/interfaces/newsletter.interface.ts
MRashad26
left a comment
MRashad26
left a comment
There was a problem hiding this comment.
LGTM — full-stack newsletter archive is clean. takeUntilDestroyed(this.destroyRef) pattern is correct for subscriptions outside injection context; computed signals for filteredNewsletters/foundationOptions/canLoadMore/isEmpty are well-structured; UUID validation in the controller gates the upstream path interpolation; route ordering is safe. No template method calls, no effect(), no local interfaces.
| this.previewVisible.set(true); | ||
| } | ||
| this.previewLoading.set(false); | ||
| }); |
There was a problem hiding this comment.
Preview fetches lack ordering guard
Medium Severity
openPreview fires detail requests without correlating responses to the clicked row. A slower older response can overwrite a newer one, and a failed fetch leaves the drawer open with the previous newsletter’s content.
Reviewed by Cursor Bugbot for commit 99918e4. Configure here.
There was a problem hiding this comment.
Pull request overview
Adds a recipient-facing newsletter archive to the Me lens, backed by authenticated archive endpoints and project/foundation enrichment.
Changes:
- Adds shared archive contracts and Express list/detail endpoints.
- Adds the Angular archive list, filtering, pagination, and preview drawer.
- Adds navigation and server route registration.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/interfaces/newsletter.interface.ts |
Defines archive response types. |
apps/lfx-one/src/server/services/newsletter-service.client.ts |
Calls upstream archive endpoints. |
apps/lfx-one/src/server/services/my-newsletters.service.ts |
Resolves memberships and enriches projects. |
apps/lfx-one/src/server/services/my-newsletters.service.spec.ts |
Tests archive service behavior. |
apps/lfx-one/src/server/server.ts |
Mounts the archive API router. |
apps/lfx-one/src/server/routes/my-newsletters.route.ts |
Defines list and detail routes. |
apps/lfx-one/src/server/controllers/my-newsletters.controller.ts |
Validates and handles archive requests. |
apps/lfx-one/src/app/shared/services/sidebar-nav.service.ts |
Adds the Me-lens navigation item. |
apps/lfx-one/src/app/shared/services/newsletter.service.ts |
Adds frontend archive requests. |
apps/lfx-one/src/app/modules/newsletters/newsletters.routes.ts |
Registers the archive page. |
apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.ts |
Implements archive state and interactions. |
apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.scss |
Adds the component stylesheet placeholder. |
apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.html |
Renders filtering, archive rows, and preview. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| public async archiveList(req: Request, committeeUids: string[], pageToken?: string): Promise<MyNewsletterArchiveResponse> { | ||
| const query: Record<string, string> = { | ||
| committee_uids: committeeUids.join(','), |
| logger.debug(req, 'enrich_newsletters_project_data', 'Project enrichment complete', { | ||
| resolved: projectMap.size, | ||
| unresolved: projectUids.length - projectMap.size, | ||
| }); |
| await service.listArchive(mockRequest); | ||
|
|
||
| // Verify that getProjectById was called 51 times total (batching is internal) | ||
| expect(projectService.getProjectById).toHaveBeenCalledTimes(51); |
| for (let i = 0; i < projectUids.length; i += batchSize) { | ||
| const batch = projectUids.slice(i, i + batchSize); | ||
| const results = await Promise.all( | ||
| batch.map(async (uid) => { | ||
| try { | ||
| return await this.projectService.getProjectById(req, uid, false); |
| <!-- Newsletter preview drawer --> | ||
| @if (previewNewsletter()) { | ||
| <lfx-newsletter-preview-drawer |
| public async archiveDetail(req: Request, newsletterUid: string): Promise<Newsletter> { | ||
| return this.microserviceProxy.proxyRequest<Newsletter>(req, 'LFX_V2_SERVICE', `/newsletters/archive/${encodeURIComponent(newsletterUid)}`, 'GET'); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (2)
apps/lfx-one/src/server/services/my-newsletters.service.ts:171
projectMapincludes both requested projects and their parent foundations, so subtracting its total size can report a negative unresolved count whenever parents were fetched. Count unresolved requested UIDs directly to keep operational telemetry accurate.
unresolved: projectUids.length - projectMap.size,
apps/lfx-one/src/server/services/my-newsletters.service.spec.ts:223
- This assertion only proves that every UID was looked up; it still passes if batching is removed or if all 51 calls run concurrently. It also contradicts the comment above: a batch size of 25 produces 25 + 25 + 1, not 25 + 26. Add a concurrency-aware assertion so the test actually covers the 25-request cap.
// Verify that getProjectById was called 51 times total (batching is internal)
expect(projectService.getProjectById).toHaveBeenCalledTimes(51);
| page_token: pageToken ? 'present' : 'absent', | ||
| }); | ||
|
|
||
| const archiveResponse = await this.newsletterClient.archiveList(req, committeeUidList, pageToken); |
| this.previewSubject.set(newsletter.subject); | ||
| this.previewBodyHtml.set(newsletter.body_html); | ||
| // Use created_by as the display name (creator email/identifier) | ||
| this.previewDisplayName.set(newsletter.created_by); |
| this.newsletterService | ||
| .getMyNewsletterDetail(item.id) | ||
| .pipe( | ||
| catchError(() => of(null)), | ||
| takeUntilDestroyed(this.destroyRef) |
| event.stopPropagation(); | ||
| } | ||
|
|
||
| this.previewLoading.set(true); |
| return all.filter((nl) => nl.foundation_slug === selectedFoundation); | ||
| }); | ||
|
|
||
| protected readonly canLoadMore: Signal<boolean> = computed(() => !!this.nextPageToken() && !this.loading() && !this.loadingMore()); |
| <lfx-newsletter-preview-drawer | ||
| [(visible)]="previewVisible" | ||
| [subject]="previewSubject()" | ||
| [bodyHtml]="previewBodyHtml()" | ||
| [displayName]="previewDisplayName()" |
| <span class="text-gray-500"> • {{ newsletter.project_name }}</span> | ||
| } | ||
| </div> | ||
| <div class="text-xs text-gray-500">Sent {{ newsletter.sent_at | date: 'MMM d, y' : 'en-US' }}</div> |
1. Use foundation_name (fallback to project_name) as the newsletter preview displayName instead of created_by, which doesn't exist in the upstream DTO. Matches sender-side preview pattern (project/foundation name). 2. Add defensive cap for committee UIDs at 500 to prevent exceeding the upstream archive endpoint limit. Truncate with warning log if exceeded. Signed-off-by: Nirav Patel <npatel@linuxfoundation.org>
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 410acaa. Configure here.
| this.previewVisible.set(true); | ||
| } | ||
| this.previewLoading.set(false); | ||
| }); |
There was a problem hiding this comment.
Preview fetch shows no loading UI
Medium Severity
Row click starts a lazy detail fetch and toggles previewLoading, but the template never reads that signal. The preview drawer is only rendered after previewNewsletter is set, so users see no loading feedback until the request finishes or fails silently.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 410acaa. Configure here.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (7)
apps/lfx-one/src/server/services/my-newsletters.service.ts:49
- The linked archive endpoint rejects more than 50
committee_uids, whilegetMyCommitteeUids()intentionally fetches every membership and this call sends the entire set. A user in 51+ committees will receive a 400, which the UI converts into an empty archive. The BFF/upstream contract needs to support the full set (simply truncating would violate the “every sent newsletter” requirement; chunking also requires globally correct cursor merging).
original_count: committeeUidList.length,
apps/lfx-one/src/server/services/newsletter-service.client.ts:192
- This return type does not match the linked upstream contract.
/newsletters/archive/{uid}returns onlyid,project_uid,subject,body_html,sent_at,committee_uids, andstatus;Newsletteradditionally promises fields such ascreated_by,ed_reply_email, and timestamps. The false type already masks the undefinedcreated_byconsumed by the new component. Add a dedicated archive-detail interface and use it through the BFF and Angular client.
public async archiveDetail(req: Request, newsletterUid: string): Promise<Newsletter> {
return this.microserviceProxy.proxyRequest<Newsletter>(req, 'LFX_V2_SERVICE', `/newsletters/archive/${encodeURIComponent(newsletterUid)}`, 'GET');
apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.ts:142
- The upstream archive detail DTO does not include
created_by, so this sets an undefined display name and the drawer falls back to the generic “Foundation” branding. The clicked list item already has the enriched foundation name; use that instead.
this.previewDisplayName.set(item.foundation_name || item.project_name);
apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.ts:134
- Multiple row clicks can leave concurrent detail requests in flight because rows remain enabled and each request subscribes independently. If A is clicked and then B, a slower A response can arrive last and replace B in the drawer. Cancel the prior request (for example with
switchMap) or add a preview request generation check before applying the response.
this.newsletterService
.getMyNewsletterDetail(item.id)
.pipe(
catchError(() => of(null)),
takeUntilDestroyed(this.destroyRef)
apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.html:69
- The drawer is only instantiated after
previewNewsletteris populated, andpreviewLoadingis not rendered anywhere. Consequently a row click shows no drawer or skeleton while detail loads, contrary to the described/manual flow, and failures provide no visible response at all. Open a loading-state drawer immediately and render a skeleton (or remove the stated behavior and provide another accessible progress indication).
@if (previewNewsletter()) {
apps/lfx-one/src/server/services/my-newsletters.service.ts:128
- This is not the batch NATS enrichment described by the PR:
getProjectById()performs one HTTP proxy request to/projects/{uid}. Grouping promises by 25 only limits concurrency, so one archive page can fan out to one request per project plus one per parent foundation. Use the intended batch lookup, or update the implementation description and explicitly accept the downstream request fan-out.
apps/lfx-one/src/server/services/my-newsletters.service.ts:171 projectMap.sizeincludes fetched parent foundations, so this telemetry can report a negative unresolved count whenever the map contains more parents than missing child projects. Count unresolved requested project UIDs directly.
}
|
|
||
| try { | ||
| // Resolve user's committee UIDs via query service | ||
| const committeeUids = await this.committeeService.getMyCommitteeUids(req); |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (6)
apps/lfx-one/src/server/services/my-newsletters.service.ts:46
- The companion service currently rejects requests with more than 50
committee_uids, not 500. This sends 51–500 IDs and receives a 400, so users in that range cannot load the archive. Align both contracts; merely truncating to 50 would still omit newsletters despite the “every sent newsletter” requirement, so the upstream API or pagination design must support the complete membership set.
// Cap committee UIDs at 500 (upstream service limit)
const COMMITTEE_UID_CAP = 500;
apps/lfx-one/src/server/services/newsletter-service.client.ts:192
- The archive detail contract is not a full
Newsletter: the companion API returns onlyid,project_uid,subject,body_html,sent_at,committee_uids, andstatus. Typing this response asNewsletterfalsely promises fields such ased_reply_email,total_recipients, and timestamps, which will beundefinedat runtime. Add a dedicated archive-detail DTO and use it through the server and frontend clients.
public async archiveDetail(req: Request, newsletterUid: string): Promise<Newsletter> {
return this.microserviceProxy.proxyRequest<Newsletter>(req, 'LFX_V2_SERVICE', `/newsletters/archive/${encodeURIComponent(newsletterUid)}`, 'GET');
apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.ts:134
- Each row click starts an independent detail subscription without canceling or ordering earlier requests. If the user clicks A and then B but A resolves last, A overwrites the preview for the newer B selection. Use
switchMapor a preview request generation/selected-ID check before applying the response.
this.newsletterService
.getMyNewsletterDetail(item.id)
.pipe(
catchError(() => of(null)),
takeUntilDestroyed(this.destroyRef)
apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.ts:128
previewLoadingis never consumed by this component's template or the drawer, so clicking a newsletter shows no skeleton or other loading feedback despite the documented flow. On a slow request the UI appears inert. Render a loading state (or open a loading-capable drawer immediately) while the detail request is pending.
this.previewLoading.set(true);
apps/lfx-one/src/app/modules/newsletters/my-newsletters/my-newsletters-list.component.html:51
- Angular's
DatePipearguments are(format, timezone, locale), so this passesen-USas a timezone and leaves the locale unchanged. Put it in the locale position (or omit it if the application locale is intended).
<div class="text-xs text-gray-500">Sent {{ newsletter.sent_at | date: 'MMM d, y' : 'en-US' }}</div>
apps/lfx-one/src/server/services/my-newsletters.service.ts:139
- This still makes one downstream request per distinct project and another per parent; limiting concurrency to 25 is not a batch lookup. A page can therefore fan out to dozens of project requests.
ProjectService.getProjectsByIds(project.service.ts:365-403) already performs batched query-service lookups; use it for the initial and parent UID sets.
return await this.projectService.getProjectById(req, uid, false);
| this.newsletterService | ||
| .listMyNewsletters(token) | ||
| .pipe( | ||
| catchError(() => of({ newsletters: [], next_page_token: undefined as string | undefined })), |
Summary
Adds a Me-lens
/my-newslettersarchive for recipients: logged-in users see every sent newsletter that targeted a committee (group) they belong to. Newsletters are filterable by foundation, showing subject and sent date. Clicking a row lazily loads the full newsletter content and opens the existing NewsletterPreviewDrawerComponent.Access model: User sees a sent newsletter iff they are a member of at least one committee in that newsletter's
committee_uids. Membership is verified server-side by the archive endpoints (no per-recipient send records, no schema changes).JIRA
LFXV2-2803
Companion Work
Upstream archive endpoints implemented in parallel:
GET /newsletters/archive?committee_uids=<csv>&page_token=...(list) andGET /newsletters/archive/{uid}(detail)sent_at DESC, id DESCImplementation Details
Shared Package
MyNewsletterListItemandMyNewsletterArchiveResponseinterfaces (newsletters, next_page_token)BFF (Express)
POST /api/newsletters/my-newsletterscontroller + service{ newsletters: [] }without upstream callFrontend (Angular 20)
/my-newslettersroute (Me-lens, authGuard only — no newsletterAccessGuard)Sidebar
Security
Deferred: Component Spec Coverage
Status: Component spec for
my-newsletters-list.component.tsis deferred.Reason: Test infrastructure mismatch. Component tests in this repo assume Jasmine (vi.fn mocks, expect().toHaveBeenCalledWith, SpyObj types); the server test runner is vitest with hoisted mocks. Adding component specs requires:
Covered: Server service has full vitest coverage (12 tests: empty committees, enrichment, project failures, error propagation, batching, parent_uid foundation lookup).
Follow-up: Align component test harness (either jasmine types or vitest pattern) and add component specs.
Protected Files
apps/lfx-one/src/server/server.ts— 4 lines changed (route mount for/api/newsletters/my-newslettersbefore project-scoped routes). Flags for code-owner review per CODEOWNERS.Test Plan
yarn format:check— All files use Prettier code styleyarn lint:check— 0 new linting errors (pre-existing campaigns.component warning unrelated)yarn check-types— TypeScript strict mode passesyarn build— Full monorepo build successfulyarn test— 33 tests pass (12 new service spec + 21 existing)Manual flow:
/my-newsletters(Me lens)