feat(dashboards): add unified pending actions api and ui improvements#184
feat(dashboards): add unified pending actions api and ui improvements#184
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit 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. WalkthroughAdds a pending-actions API and UI integration for board members, introduces a cookie registry service and clear-cache UI (feature-flagged), refactors meeting selection and manage flows for persona/mode awareness, and minor UI/style/text updates across dashboards and sidebar. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Frontend Component
participant API as /api/user/pending-actions
participant UserCtrl as UserController
participant UserSvc as UserService
participant ProjectSvc as ProjectService
participant MeetingSvc as MeetingService
Client->>API: GET /pending-actions?persona=...&projectUid=...&projectSlug=...
API->>UserCtrl: handle request
UserCtrl->>UserSvc: getPendingActions(req, persona, projectUid, email, projectSlug)
rect rgb(240, 245, 255)
Note over UserSvc,ProjectSvc: Board-member flow — parallel fetch
par Surveys
UserSvc->>ProjectSvc: getPendingSurveys(projectSlug/projectUid)
ProjectSvc-->>UserSvc: surveys[]
and Meetings
UserSvc->>MeetingSvc: getMeetingsByProject(projectUid)
MeetingSvc-->>UserSvc: meetings[]
end
end
rect rgb(235, 250, 235)
Note over UserSvc: Transform & Filter
UserSvc->>UserSvc: filter meetings to 2-week window
UserSvc->>UserSvc: transform meetings -> PendingActionItem[]
UserSvc-->>UserCtrl: PendingActionItem[]
UserCtrl-->>Client: JSON response
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (6)
apps/lfx-one/src/server/controllers/user.controller.ts (1)
43-55: Validate persona against allowed values.The
personaparameter is cast toPersonaTypewithout validation. An invalid persona value will pass validation and reach the service layer.// Extract and validate persona const persona = req.query['persona'] as PersonaType | undefined; if (!persona) { Logger.error(req, 'get_pending_actions', startTime, new Error('Missing persona parameter')); const validationError = ServiceValidationError.forField('persona', 'persona query parameter is required', { operation: 'get_pending_actions', service: 'user_controller', path: req.path, }); next(validationError); return; } + + const validPersonas: PersonaType[] = ['core-developer', 'maintainer', 'projects', 'board-member']; + if (!validPersonas.includes(persona)) { + Logger.error(req, 'get_pending_actions', startTime, new Error(`Invalid persona: ${persona}`)); + + const validationError = ServiceValidationError.forField('persona', `Invalid persona. Must be one of: ${validPersonas.join(', ')}`, { + operation: 'get_pending_actions', + service: 'user_controller', + path: req.path, + }); + + next(validationError); + return; + }apps/lfx-one/src/app/shared/services/project-context.service.ts (2)
21-21: Consider using a setter method or signal for better encapsulation.Making
availableProjectspublicly mutable allows direct external modification, which bypasses encapsulation and prevents tracking changes or adding validation logic. Consider using a setter method or converting to a signal for better state management.Example using a setter:
- public availableProjects: ProjectContext[] = []; + private _availableProjects: ProjectContext[] = []; + + public get availableProjects(): ProjectContext[] { + return this._availableProjects; + } + + public setAvailableProjects(projects: ProjectContext[]): void { + this._availableProjects = projects; + }Or using a signal (Angular 19 pattern):
- public availableProjects: ProjectContext[] = []; + public availableProjects = signal<ProjectContext[]>([]);
87-88: Consider adding error handling for cookie registration.The cookie registration happens after persistence, but there's no error handling if registration fails. While the cookie will still function, it won't be tracked for the "Clear Cache" feature.
- // Register cookie for tracking - this.cookieRegistry.registerCookie(key); + // Register cookie for tracking + try { + this.cookieRegistry.registerCookie(key); + } catch (error) { + console.warn('Failed to register cookie in registry:', error); + }apps/lfx-one/src/app/shared/services/cookie-registry.service.ts (2)
23-33: Consider logging JSON parse errors for debugging.The method silently returns an empty array when JSON parsing fails. While this prevents crashes, it makes debugging registry corruption difficult. Consider adding a console warning.
} catch { - // Invalid data in registry cookie, return empty array + // Invalid data in registry cookie, return empty array + console.warn('Failed to parse cookie registry, returning empty array'); }
57-64: Consider size limits when persisting cookie registry.Cookies are typically limited to 4KB. If many cookies are registered, the JSON string could exceed this limit, causing silent failure. Consider adding a size check or limiting the number of tracked cookies.
private persistRegistry(keys: string[]): void { + const registryData = JSON.stringify(keys); + // Cookie size limit is typically 4KB + if (registryData.length > 4000) { + console.warn('Cookie registry exceeds size limit, some cookies may not be tracked'); + } this.cookieService.set(COOKIE_REGISTRY_KEY, JSON.stringify(keys), { expires: 365, // 1 year path: '/', sameSite: 'Lax', secure: process.env['NODE_ENV'] === 'production', }); }apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts (1)
166-176: Consider adding user confirmation before clearing cache.The
clearCachemethod immediately clears all cookies and reloads the page without confirmation. Users might accidentally trigger this and lose unsaved work. Consider adding a confirmation dialog (PrimeNG already provides ConfirmDialog service used elsewhere in the app).protected clearCache(): void { - this.cookieRegistry.clearAllCookies(); - - if (typeof window !== 'undefined') { - window.location.reload(); - } + // Note: Requires ConfirmationService to be provided in component + this.confirmationService.confirm({ + message: 'This will clear all cached data and reload the page. Any unsaved changes will be lost. Continue?', + header: 'Clear Cache', + icon: 'fa-solid fa-triangle-exclamation', + accept: () => { + this.cookieRegistry.clearAllCookies(); + if (typeof window !== 'undefined') { + window.location.reload(); + } + } + }); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (23)
.vscode/settings.json(1 hunks)apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html(4 hunks)apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.html(0 hunks)apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts(3 hunks)apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts(5 hunks)apps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.ts(1 hunks)apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html(1 hunks)apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts(5 hunks)apps/lfx-one/src/app/shared/services/account-context.service.ts(2 hunks)apps/lfx-one/src/app/shared/services/cookie-registry.service.ts(1 hunks)apps/lfx-one/src/app/shared/services/hidden-actions.service.ts(3 hunks)apps/lfx-one/src/app/shared/services/project-context.service.ts(2 hunks)apps/lfx-one/src/app/shared/services/project.service.ts(1 hunks)apps/lfx-one/src/server/controllers/user.controller.ts(1 hunks)apps/lfx-one/src/server/routes/user.route.ts(1 hunks)apps/lfx-one/src/server/server.ts(2 hunks)apps/lfx-one/src/server/services/user.service.ts(4 hunks)apps/lfx-one/src/styles.scss(1 hunks)packages/shared/src/constants/cookie.constants.ts(1 hunks)packages/shared/src/constants/index.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.html
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Always use direct imports for standalone components - no barrel exports
Use TypeScript interfaces instead of union types for better maintainability
Do not nest ternary expressions
Prefer interface for defining object shapes in TypeScript
Use path mappings and import aliases as configured in tsconfig.json for imports
Files:
apps/lfx-one/src/server/routes/user.route.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tspackages/shared/src/constants/cookie.constants.tsapps/lfx-one/src/app/shared/services/hidden-actions.service.tsapps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.tsapps/lfx-one/src/app/shared/services/project.service.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/services/user.service.tspackages/shared/src/constants/index.tsapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.tsapps/lfx-one/src/server/controllers/user.controller.tsapps/lfx-one/src/app/shared/services/account-context.service.tsapps/lfx-one/src/app/shared/services/cookie-registry.service.tsapps/lfx-one/src/app/shared/services/project-context.service.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts
**/*.{html,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/server/routes/user.route.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tspackages/shared/src/constants/cookie.constants.tsapps/lfx-one/src/app/shared/services/hidden-actions.service.tsapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.htmlapps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.tsapps/lfx-one/src/app/shared/services/project.service.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/services/user.service.tspackages/shared/src/constants/index.tsapps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.htmlapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.tsapps/lfx-one/src/server/controllers/user.controller.tsapps/lfx-one/src/app/shared/services/account-context.service.tsapps/lfx-one/src/app/shared/services/cookie-registry.service.tsapps/lfx-one/src/app/shared/services/project-context.service.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.htmlapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: License headers are required on all source files
Prepend 'Generated with Claude Code (https://claude.ai/code)' if assisted with the code
Files:
apps/lfx-one/src/server/routes/user.route.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tspackages/shared/src/constants/cookie.constants.tsapps/lfx-one/src/app/shared/services/hidden-actions.service.tsapps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.tsapps/lfx-one/src/app/shared/services/project.service.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/services/user.service.tspackages/shared/src/constants/index.tsapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.tsapps/lfx-one/src/server/controllers/user.controller.tsapps/lfx-one/src/app/shared/services/account-context.service.tsapps/lfx-one/src/app/shared/services/cookie-registry.service.tsapps/lfx-one/src/app/shared/services/project-context.service.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts
apps/**/**/server/**/*.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/**/server/**/*.{ts,js}: Use Pino for structured JSON logs with sensitive data redaction in all backend services
Always use err field for errors to leverage Pino's error serializer - correct: req.log.error({ err: error, ...metadata }, 'message')
Log INFO level for business operation completions (created, updated, deleted) and successful data retrieval
Log WARN level for error conditions leading to exceptions and data quality issues
Log DEBUG level for internal operations, preparation steps, and intent statements
Log ERROR level for system failures, unhandled exceptions, and critical errors
Files:
apps/lfx-one/src/server/routes/user.route.tsapps/lfx-one/src/server/server.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/server/controllers/user.controller.ts
**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Angular 19 zoneless change detection with signals for component state management
Files:
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.tsapps/lfx-one/src/app/shared/components/sidebar/sidebar.component.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts
packages/shared/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
All shared types, interfaces, and constants are centralized in @lfx-one/shared package
Files:
packages/shared/src/constants/cookie.constants.tspackages/shared/src/constants/index.ts
**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
Use data-testid naming convention - [section]-[component]-[element] for hierarchical structure
Files:
apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.htmlapps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.htmlapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html
apps/**/**/server/**/*controller*.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
All controller functions must use Logger helper methods: Logger.start(), Logger.success(), Logger.error(), Logger.warning(), Logger.validation()
Files:
apps/lfx-one/src/server/controllers/user.controller.ts
**/*.{css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind CSS with PrimeUI plugin and LFX custom colors for styling
Files:
apps/lfx-one/src/styles.scss
🧠 Learnings (6)
📚 Learning: 2025-10-21T21:19:13.599Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-ui PR: 125
File: apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts:345-350
Timestamp: 2025-10-21T21:19:13.599Z
Learning: In the Angular meeting card component (apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts), when selecting between `summary.summary_data.edited_content` and `summary.summary_data.content`, the logical OR operator (`||`) is intentionally used instead of nullish coalescing (`??`) because empty string edited_content should fall back to the original content rather than being displayed as empty.
Applied to files:
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.tsapps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.tsapps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.htmlapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.tsapps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Applies to packages/shared/src/**/*.{ts,tsx} : All shared types, interfaces, and constants are centralized in lfx-one/shared package
Applied to files:
packages/shared/src/constants/index.ts
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Follow Angular commit conventions: type(scope): description with valid types - feat, fix, docs, style, refactor, perf, test, build, ci, chore, revert
Applied to files:
apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Applies to **/*.{css,scss} : Use Tailwind CSS with PrimeUI plugin and LFX custom colors for styling
Applied to files:
apps/lfx-one/src/styles.scss
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Applies to **/*.component.ts : Use Angular 19 zoneless change detection with signals for component state management
Applied to files:
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: All PrimeNG components are wrapped in LFX components for UI library independence
Applied to files:
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
🧬 Code graph analysis (7)
apps/lfx-one/src/server/routes/user.route.ts (1)
apps/lfx-one/src/server/controllers/user.controller.ts (1)
UserController(14-104)
apps/lfx-one/src/app/shared/services/project.service.ts (2)
packages/shared/src/interfaces/components.interface.ts (1)
PendingActionItem(386-401)apps/lfx-one/src/server/helpers/logger.ts (1)
error(52-65)
apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts (1)
packages/shared/src/interfaces/components.interface.ts (1)
SidebarMenuItem(305-330)
apps/lfx-one/src/server/controllers/user.controller.ts (4)
apps/lfx-one/src/server/services/user.service.ts (1)
UserService(37-616)apps/lfx-one/src/server/helpers/logger.ts (2)
Logger(10-129)error(52-65)apps/lfx-one/src/server/errors/index.ts (1)
ServiceValidationError(7-7)packages/shared/src/interfaces/persona.interface.ts (1)
PersonaType(8-8)
apps/lfx-one/src/app/shared/services/cookie-registry.service.ts (4)
apps/lfx-one/src/app/shared/services/account-context.service.ts (1)
Injectable(11-108)apps/lfx-one/src/app/shared/services/hidden-actions.service.ts (1)
Injectable(15-98)apps/lfx-one/src/app/shared/services/project-context.service.ts (1)
Injectable(10-102)packages/shared/src/constants/cookie.constants.ts (1)
COOKIE_REGISTRY_KEY(8-8)
apps/lfx-one/src/app/shared/services/project-context.service.ts (1)
packages/shared/src/interfaces/project.interface.ts (1)
ProjectContext(167-174)
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
apps/lfx-one/src/server/helpers/logger.ts (1)
error(52-65)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (28)
.vscode/settings.json (1)
13-13: Good catch on spell-check hygiene.Adding "DATEADD" to the spell-check dictionary prevents false warnings on SQL queries (likely used in the Snowflake aggregation logic for pending surveys). This is appropriate housekeeping for the codebase.
apps/lfx-one/src/styles.scss (1)
147-147: CSS selector refinement aligns with Tailwind-first styling strategy.The updated selector
.p-button:not([class*='text-sm']).p-button-smis consistent with the pattern used elsewhere in the file (e.g., line 27) and prevents the small-button sizing from conflicting with Tailwind's text-utility classes. This refinement supports the cohesive styling approach across updated components and aligns with the coding guideline to use Tailwind CSS with PrimeUI plugin.apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html (5)
4-11: Header structure and data-testid naming look good.All data-testid attributes follow the hierarchical [section]-[component]-[element] convention correctly. The icon and text layout with gap-2 spacing is clean.
14-14: Scrollable container padding applied correctly.The p-2 padding on the scrollable content aligns with the header padding (px-2), maintaining visual consistency.
29-56: Badge styling with nested header container looks well-structured.The new nested header with px-2 py-1 padding, rounded corners, and dynamic -300 background variants creates a clear visual separation for the item type. Icon sizing reduction (w-4 → w-3) on line 44 is appropriate for the text-xs context.
60-89: Text and button styling simplified consistently.Typography has been unified with text-base font-normal for action text and text-sm added to button classes for both link and non-link variants. Styling is now more maintainable with fewer per-variant overrides.
4-4: Excellent data-testid coverage for testing.All key component sections have hierarchical data-testid attributes: section root, list container, individual items with dynamic type, and empty state. This enables reliable test targeting across all states per coding guidelines.
Also applies to: 16-16, 26-26, 95-95
apps/lfx-one/src/server/server.ts (1)
30-30: LGTM!The
userRouterimport and mounting follow the established patterns for protected API routes. The route is correctly placed after the authentication middleware and before the error handler.Also applies to: 226-226
apps/lfx-one/src/server/routes/user.route.ts (1)
1-14: LGTM!The route module follows the established patterns in the codebase. License header is present, controller delegation is correct, and the route is properly exported.
apps/lfx-one/src/server/controllers/user.controller.ts (1)
99-102: LGTM!Error handling correctly logs the error and forwards it to the error middleware via
next(error).apps/lfx-one/src/server/services/user.service.ts (5)
40-49: LGTM!The new service dependencies and the
twoWeeksMsconstant are properly initialized. The pattern follows existing code structure.
462-468: LGTM!The method correctly routes to persona-specific handlers and returns an empty array for unimplemented personas. The comment clearly indicates future extension points.
474-491: LGTM!The parallel fetching with graceful error handling is well implemented. Using
Promise.allwith individual.catch()handlers ensures partial results are returned even if one source fails. The WARN level logging is appropriate per coding guidelines.
496-521: LGTM!The transformation logic correctly:
- Filters to only upcoming meetings within the 2-week window
- Excludes cancelled occurrences
- Handles both recurring (with occurrences) and non-recurring meetings
535-545: Password in meeting URL query string exposes security risk.The buttonLink includes the meeting password in a URL query parameter (
?password=${meeting.password}), which can expose it to:
- Browser history and autocomplete
- Referrer headers sent with subsequent requests
- Server/proxy access logs
- Client-side analytics and monitoring tools
Server-side logging sanitization does not protect against these client-side and network-level exposures.
Consider:
- Using a short-lived token instead of the plain password
- Passing the password via POST request or session storage on the frontend
- Implementing access log redaction for query parameters at the infrastructure level
const buttonLink = meeting.password ? `/meetings/${meeting.uid}?password=${meeting.password}` : `/meetings/${meeting.uid}`;packages/shared/src/constants/index.ts (1)
27-27: LGTM!The export follows the existing pattern and correctly exposes the COOKIE_REGISTRY_KEY constant from the shared constants package.
Based on learnings, all shared types, interfaces, and constants are centralized in @lfx-one/shared package.
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (2)
199-199: Verify null handling for dynamic subtitle.The subtitle now uses
${this.projectContextService.selectedFoundation()?.name}which could render as "Total undefined projects" ifselectedFoundation()returns null or undefined. Consider adding a fallback value.Apply this diff to add a fallback:
- subtitle: `Total ${this.projectContextService.selectedFoundation()?.name} projects`, + subtitle: `Total ${this.projectContextService.selectedFoundation()?.name || 'foundation'} projects`,
243-243: Verify null handling for dynamic subtitle.The subtitle now uses
${this.projectContextService.selectedFoundation()?.name}which could render as "Total undefined members" ifselectedFoundation()returns null or undefined. Consider adding a fallback value.Apply this diff to add a fallback:
- subtitle: `Total ${this.projectContextService.selectedFoundation()?.name} members`, + subtitle: `Total ${this.projectContextService.selectedFoundation()?.name || 'foundation'} members`,apps/lfx-one/src/app/shared/services/hidden-actions.service.ts (1)
8-8: LGTM!The integration with CookieRegistryService is correctly implemented. The cookie is registered immediately after being set, enabling centralized cookie tracking for the clear cache functionality.
Also applies to: 20-20, 38-39
apps/lfx-one/src/app/shared/services/account-context.service.ts (1)
9-9: LGTM!The integration with CookieRegistryService follows the same pattern as hidden-actions.service.ts. The cookie is properly registered after being persisted, enabling SSR-compatible cookie tracking.
Also applies to: 16-16, 93-94
apps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.ts (1)
47-54: LGTM - Improved foundation lookup.The change from hard-coded values to dynamic lookup is a good improvement. The code safely handles the case where the 'tlf' project might not be in availableProjects by checking
if (tlfProject)before callingsetFoundation.Note: The TODO comment indicates this is demo code that should be replaced with proper permissions implementation.
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts (1)
84-94: The review comment's concern is incorrect. Bothproject.uidandproject.slugare required non-optional properties in the Project interface, so one cannot exist without the other.The guard condition
if (!project?.slug || !project?.uid)is appropriate and correctly implemented. It filters out Foundation objects (which have neither slug nor uid) while allowing Project objects (which have both). The server-side implementation only supports Projects and requires bothprojectSlugandprojectUid, making this guard condition correct and necessary.apps/lfx-one/src/app/shared/services/project.service.ts (1)
67-83: LGTM! New pending actions endpoint integration looks good.The implementation follows the established pattern from
getPendingActionSurveys, with proper error handling and parameter construction. The method correctly aggregates surveys and meetings from the unified backend endpoint.apps/lfx-one/src/app/shared/services/project-context.service.ts (1)
8-8: LGTM! Cookie registry service properly integrated.The import and injection follow Angular best practices and align with the existing pattern for
cookieService.Also applies to: 15-15
apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts (1)
84-93: LGTM! Feature flag and menu item properly configured.The clear cache functionality is correctly guarded by a feature flag with a safe default (disabled). The menu item follows the SidebarMenuItem interface and includes a test ID for e2e testing.
apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts (2)
98-98: LGTM! Mode-aware step tracking properly implemented.The introduction of
internalStepfor create mode cleanly separates URL-based navigation (edit mode) from internal state (create mode). The initialization logic correctly handles both modes with appropriate fallbacks.Also applies to: 128-150
178-222: LGTM! Consistent mode-aware navigation across all step methods.The
goToStep,nextStep, andpreviousStepmethods correctly implement mode-aware navigation, updating query parameters in edit mode and internal state in create mode. The logic is consistent and easy to follow.apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (1)
35-54: The review comment is incorrect. No defensive handling is needed.The
PersonaService.currentPersonasignal is typed asWritableSignal<PersonaType>(wherePersonaType = 'core-developer' | 'maintainer' | 'projects' | 'board-member') and is guaranteed to always hold a valid value. It is initialized with'maintainer'in the constructor, andinitializeFromAuth()explicitly callssetPersona('maintainer')if a null/undefined persona is received, ensuring the signal never holds null or undefined. The equality check at line 48 is safe and correct.
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html
Show resolved
Hide resolved
...lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
This PR introduces a unified pending actions API endpoint, implements a cookie registry service for cache management, improves the meeting creation UX flow, and polishes dashboard UI styling across multiple components.
Key Changes:
- Adds
/api/user/pending-actionsendpoint that consolidates surveys and meetings into a single response, with persona-based filtering (currently supporting board-member persona) - Implements a
CookieRegistryServicethat tracks all LFX cookies and provides aclearAllCookies()method for cache clearing functionality - Improves meeting creation flow by using internal signal-based step tracking (vs URL params) to keep URLs clean during creation while maintaining URL-based navigation for edit mode
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
packages/shared/src/constants/index.ts |
Exports new cookie constants for registry key |
packages/shared/src/constants/cookie.constants.ts |
Defines COOKIE_REGISTRY_KEY constant for tracking cookie names |
apps/lfx-one/src/styles.scss |
Updates button styling to exclude text-sm classes from p-button-sm styling |
apps/lfx-one/src/server/services/user.service.ts |
Adds getPendingActions() method to consolidate surveys and meetings with 2-week filtering |
apps/lfx-one/src/server/server.ts |
Registers new /api/user router |
apps/lfx-one/src/server/routes/user.route.ts |
New route file defining /pending-actions endpoint |
apps/lfx-one/src/server/controllers/user.controller.ts |
New controller handling pending actions requests with validation |
apps/lfx-one/src/app/shared/services/project.service.ts |
Adds client-side method to call unified pending actions API |
apps/lfx-one/src/app/shared/services/project-context.service.ts |
Integrates cookie registry for tracking context cookies |
apps/lfx-one/src/app/shared/services/hidden-actions.service.ts |
Registers hidden action cookies in registry |
apps/lfx-one/src/app/shared/services/cookie-registry.service.ts |
New service implementing cookie tracking and batch deletion |
apps/lfx-one/src/app/shared/services/account-context.service.ts |
Registers account context cookies in registry |
apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts |
Adds clear cache button functionality with page reload |
apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html |
Adds conditional clear cache menu item behind feature flag |
apps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.ts |
Uses dynamic TLF project lookup from availableProjects instead of hardcoded values |
apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts |
Implements dual-mode step navigation (internal signals for create, URL params for edit) and removes project selector form control |
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts |
Removes project selector component and related logic, filters meeting types by persona |
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.html |
Removes project selector UI |
apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html |
Updates styling with icons, refined color scheme, and consistent spacing |
apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html |
Adds calendar icon to section header |
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts |
Makes subtitle text dynamic based on selected foundation name |
apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts |
Switches from separate survey API to unified pending actions endpoint |
.vscode/settings.json |
Adds 'DATEADD' to spell checker dictionary |
💡 Add Copilot custom instructions for smarter, more guided reviews. 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.
...lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts
Show resolved
Hide resolved
...c/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
Show resolved
Hide resolved
...lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts
Show resolved
Hide resolved
LFXV2-828 LFXV2-829 LFXV2-830 LFXV2-831 - Add unified /api/user/pending-actions endpoint consolidating surveys and meetings - Implement cookie registry service with clear cache functionality - Improve meeting creation flow UX with internal step tracking - Polish pending actions, foundation health, and my meetings UI styling - Fix persona selector to use dynamic project UID - Remove email PII from INFO level logs for compliance Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
bb02336 to
486883a
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
apps/lfx-one/src/server/services/user.service.ts (1)
537-544: Type/buttonText/text inconsistency already flagged.The mismatch between
type: 'Review Minutes',buttonText: 'Review Agenda', andtext: 'Review ... Agenda and Materials'was already noted in a previous review.apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts (1)
417-442: Still need to validateproject_uidbefore submitting meeting data
project_uidis derived as:project_uid: this.projectContextService.selectedProject()?.uid || this.projectContextService.selectedFoundation()?.uid || '',If neither a project nor foundation is selected, this sends
project_uid: ''to the API, which conflicts withCreateMeetingRequest/UpdateMeetingRequestexpectations and was already flagged in earlier reviews.Recommend enforcing a non-empty project context before calling
prepareMeetingData()(and thus beforecreateMeeting/updateMeeting), e.g.:
- Block progression / submission when
!this.project()and show a clear error message, or- Add a guard at the top of
prepareMeetingDatathat throws or returns early if no project/foundation UID is available.This prevents opaque backend failures and makes the requirement visible in the UX.
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts (1)
11-12: Confirm persona-based meeting type filtering logicFor
currentPersona() === 'maintainer', only MAINTAINERS/TECHNICAL/OTHER types are shown, while other personas see all options (including Board/Marketing/Legal). If the intent was instead to restrict non-maintainers and let maintainers see everything, this condition should be inverted.Please confirm the product requirement; if inverted, adjust to:
const filteredOptions = this.personaService.currentPersona() === 'maintainer' ? allOptions : allOptions.filter((option) => allowedTypes.includes(option.value));Also applies to: 24-25, 28-29, 35-53
🧹 Nitpick comments (3)
apps/lfx-one/src/server/services/user.service.ts (2)
453-468: Consider adding structured logging for this operation.Per coding guidelines, INFO level logs should be used for business operation completions and successful data retrieval. Consider adding logging at the start and completion of this operation.
public async getPendingActions(req: Request, persona: PersonaType, projectUid: string, email: string, projectSlug: string): Promise<PendingActionItem[]> { + req.log.debug({ persona, projectUid, projectSlug }, 'Fetching pending actions'); if (persona === 'board-member') { - return this.getBoardMemberActions(req, email, projectSlug, projectUid); + const actions = await this.getBoardMemberActions(req, email, projectSlug, projectUid); + req.log.info({ persona, actionsCount: actions.length }, 'Fetched pending actions'); + return actions; } // Future personas: maintainer, core-developer can be added here + req.log.debug({ persona }, 'No pending actions handler for persona'); return []; }
530-533: Hardcoded locale may not match user preferences.The date formatting uses a hardcoded
'en-US'locale. If the application supports internationalization, consider using the user's preferred locale or a configurable setting.apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts (1)
192-200: Guard againstundefinedfoundation name in subtitles
subtitle: \Total ${this.projectContextService.selectedFoundation()?.name} projects/members`will render “Total undefined …” if no foundation is selected orname` is missing. Consider a fallback, e.g.:const foundationName = this.projectContextService.selectedFoundation()?.name ?? 'foundation'; subtitle: `Total ${foundationName} projects`;and similarly for members, or rely on
hasFoundationSelectedto hide these cards whenever no foundation is selected.Also applies to: 236-244
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (25)
.vscode/settings.json(1 hunks)apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.ts(2 hunks)apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html(1 hunks)apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html(4 hunks)apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.html(0 hunks)apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts(3 hunks)apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts(5 hunks)apps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.ts(1 hunks)apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html(1 hunks)apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts(5 hunks)apps/lfx-one/src/app/shared/services/account-context.service.ts(2 hunks)apps/lfx-one/src/app/shared/services/cookie-registry.service.ts(1 hunks)apps/lfx-one/src/app/shared/services/hidden-actions.service.ts(3 hunks)apps/lfx-one/src/app/shared/services/project-context.service.ts(2 hunks)apps/lfx-one/src/app/shared/services/project.service.ts(1 hunks)apps/lfx-one/src/server/controllers/analytics.controller.ts(0 hunks)apps/lfx-one/src/server/controllers/project.controller.ts(0 hunks)apps/lfx-one/src/server/controllers/user.controller.ts(1 hunks)apps/lfx-one/src/server/routes/user.route.ts(1 hunks)apps/lfx-one/src/server/server.ts(2 hunks)apps/lfx-one/src/server/services/user.service.ts(4 hunks)apps/lfx-one/src/styles.scss(1 hunks)packages/shared/src/constants/cookie.constants.ts(1 hunks)packages/shared/src/constants/index.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/lfx-one/src/server/controllers/analytics.controller.ts
- apps/lfx-one/src/server/controllers/project.controller.ts
- apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.html
🚧 Files skipped from review as they are similar to previous changes (15)
- apps/lfx-one/src/server/routes/user.route.ts
- packages/shared/src/constants/index.ts
- apps/lfx-one/src/app/modules/dashboards/components/my-meetings/my-meetings.component.html
- apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.html
- apps/lfx-one/src/app/shared/services/account-context.service.ts
- apps/lfx-one/src/app/shared/services/project-context.service.ts
- apps/lfx-one/src/app/shared/components/persona-selector/persona-selector.component.ts
- .vscode/settings.json
- apps/lfx-one/src/app/shared/services/cookie-registry.service.ts
- apps/lfx-one/src/app/modules/dashboards/board-member/board-member-dashboard.component.ts
- apps/lfx-one/src/app/modules/dashboards/components/pending-actions/pending-actions.component.html
- apps/lfx-one/src/server/server.ts
- packages/shared/src/constants/cookie.constants.ts
- apps/lfx-one/src/app/shared/components/sidebar/sidebar.component.ts
- apps/lfx-one/src/server/controllers/user.controller.ts
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Always use direct imports for standalone components - no barrel exports
Use TypeScript interfaces instead of union types for better maintainability
Do not nest ternary expressions
Prefer interface for defining object shapes in TypeScript
Use path mappings and import aliases as configured in tsconfig.json for imports
Files:
apps/lfx-one/src/app/shared/services/project.service.tsapps/lfx-one/src/app/shared/services/hidden-actions.service.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
**/*.{html,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Always add data-testid attributes when creating new components for reliable test targeting
Files:
apps/lfx-one/src/app/shared/services/project.service.tsapps/lfx-one/src/app/shared/services/hidden-actions.service.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx,js,jsx}: License headers are required on all source files
Prepend 'Generated with Claude Code (https://claude.ai/code)' if assisted with the code
Files:
apps/lfx-one/src/app/shared/services/project.service.tsapps/lfx-one/src/app/shared/services/hidden-actions.service.tsapps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/server/services/user.service.tsapps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
**/*.{css,scss}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Tailwind CSS with PrimeUI plugin and LFX custom colors for styling
Files:
apps/lfx-one/src/styles.scss
**/*.component.ts
📄 CodeRabbit inference engine (CLAUDE.md)
Use Angular 19 zoneless change detection with signals for component state management
Files:
apps/lfx-one/src/app/modules/dashboards/components/foundation-health/foundation-health.component.tsapps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
apps/**/**/server/**/*.{ts,js}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/**/**/server/**/*.{ts,js}: Use Pino for structured JSON logs with sensitive data redaction in all backend services
Always use err field for errors to leverage Pino's error serializer - correct: req.log.error({ err: error, ...metadata }, 'message')
Log INFO level for business operation completions (created, updated, deleted) and successful data retrieval
Log WARN level for error conditions leading to exceptions and data quality issues
Log DEBUG level for internal operations, preparation steps, and intent statements
Log ERROR level for system failures, unhandled exceptions, and critical errors
Files:
apps/lfx-one/src/server/services/user.service.ts
🧠 Learnings (3)
📚 Learning: 2025-10-21T21:19:13.599Z
Learnt from: andrest50
Repo: linuxfoundation/lfx-v2-ui PR: 125
File: apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts:345-350
Timestamp: 2025-10-21T21:19:13.599Z
Learning: In the Angular meeting card component (apps/lfx-one/src/app/modules/project/meetings/components/meeting-card/meeting-card.component.ts), when selecting between `summary.summary_data.edited_content` and `summary.summary_data.content`, the logical OR operator (`||`) is intentionally used instead of nullish coalescing (`??`) because empty string edited_content should fall back to the original content rather than being displayed as empty.
Applied to files:
apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.tsapps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: Applies to **/*.component.ts : Use Angular 19 zoneless change detection with signals for component state management
Applied to files:
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
📚 Learning: 2025-11-24T17:42:04.908Z
Learnt from: CR
Repo: linuxfoundation/lfx-v2-ui PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-24T17:42:04.908Z
Learning: All PrimeNG components are wrapped in LFX components for UI library independence
Applied to files:
apps/lfx-one/src/app/modules/meetings/components/meeting-type-selection/meeting-type-selection.component.ts
🧬 Code graph analysis (2)
apps/lfx-one/src/app/shared/services/project.service.ts (2)
packages/shared/src/interfaces/components.interface.ts (1)
PendingActionItem(386-401)apps/lfx-one/src/server/helpers/logger.ts (1)
error(52-65)
apps/lfx-one/src/server/services/user.service.ts (3)
packages/shared/src/interfaces/persona.interface.ts (1)
PersonaType(8-8)packages/shared/src/interfaces/components.interface.ts (1)
PendingActionItem(386-401)packages/shared/src/interfaces/meeting.interface.ts (2)
Meeting(90-167)MeetingOccurrence(173-186)
🔇 Additional comments (8)
apps/lfx-one/src/styles.scss (1)
147-148: Use Tailwind v3 important modifier syntax and simplify the defensive selector since no buttons combinep-button-smwithtext-sm.The selector is overly defensive; no buttons in the codebase apply both
.p-button-smand.text-smtogether. Additionally, the original review suggests incorrect Tailwind syntax: the project uses Tailwind v3.4.17, which requires!text-xs(important modifier at the beginning), nottext-xs!(v4 syntax).Simplify to:
.p-button.p-button-sm { @apply !text-xs tracking-wide px-2.5 py-1.5 gap-2; .p-button-icon { @apply w-3.5 h-3.5; } }The
!text-xsensures the text size is enforced regardless of other utilities, while eliminating the unnecessary:not([class*='text-sm'])condition that never activates in practice.apps/lfx-one/src/app/shared/services/project.service.ts (1)
66-83: LGTM!The new
getPendingActionsmethod follows the established patterns in this service:
- Consistent error handling with
catchErrorreturning an empty array- Proper use of
HttpParamsfor query parameters- Clear JSDoc documentation
apps/lfx-one/src/server/services/user.service.ts (3)
40-49: LGTM!The new service dependencies and the
twoWeeksMsconstant are properly initialized. The pattern follows the existing service instantiation approach used fornatsServiceandsnowflakeService.
474-491: LGTM - Good parallel fetching pattern with resilient error handling.The use of
Promise.allwith individual.catch()handlers ensures that a failure in one data source doesn't block the other. The WARN level logging is appropriate per coding guidelines for error conditions that don't cause exceptions.
496-521: LGTM!The transformation logic correctly handles:
- Meetings with multiple occurrences (recurring meetings)
- Single meetings without occurrences
- Cancelled occurrence filtering
- The two-week window constraint
apps/lfx-one/src/app/shared/services/hidden-actions.service.ts (1)
8-9: Cookie registry integration inhideActionlooks correctRegistering the hashed cookie name with
CookieRegistryServiceright after setting it is consistent with the registry’s purpose and doesn’t affect existing hide/show behavior.Also applies to: 20-21, 38-40
apps/lfx-one/src/app/modules/meetings/meeting-manage/meeting-manage.component.ts (2)
97-99: Internal step signal for create mode is reasonableUsing a private
internalStepsignal to drivecurrentStepin create mode cleanly decouples it from URL-based navigation and matches the described UX change.
127-147: Mode-aware step navigation logic is consistentRouting step changes through query params only in edit mode and using
internalStepin create mode keeps URLs stable for create flows while preserving deep-linking for edits. The computedcurrentStepwiring and scroll behavior remain coherent with existing validation (canNavigateToStep/isStepValid).Also applies to: 178-187, 191-220
| day: 'numeric', | ||
| }); | ||
|
|
||
| const buttonLink = meeting.password ? `/meetings/${meeting.uid}?password=${meeting.password}` : `/meetings/${meeting.uid}`; |
There was a problem hiding this comment.
Security consideration: Avoid exposing meeting password in URL query parameters.
Passwords in URL query strings can be logged in server access logs, browser history, and referrer headers. Consider an alternative approach such as requiring authentication to retrieve the password server-side, or using a session-based mechanism.
- const buttonLink = meeting.password ? `/meetings/${meeting.uid}?password=${meeting.password}` : `/meetings/${meeting.uid}`;
+ // Password should be retrieved securely when user accesses the meeting
+ const buttonLink = `/meetings/${meeting.uid}`;Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In apps/lfx-one/src/server/services/user.service.ts around line 535, the code
builds a meeting link that exposes the meeting.password in the URL query string;
remove the password from the query string and instead implement a server-side
flow to provide access: generate a short-lived, unguessable access token or
session entry tied to the meeting UID and authenticated user, return a link
containing only the token/UID (or require the user to fetch the password via an
authenticated API POST call), and update the meeting join flow to validate the
token/server-side session and inject the password server-side rather than
relying on URL parameters; ensure tokens expire and are logged/rotated securely.
Summary
/api/user/pending-actionsendpoint consolidating surveys and meetings (LFXV2-828)Related JIRA Tickets
🤖 Generated with Claude Code