fix(meetings): prevent form data loss when navigating steps in edit mode

[asithade]

Summary

• Fixed issue where meeting form data was lost when navigating between steps in edit mode
• Separated step navigation effect from meeting data population logic
• Form now populates only once on initial load, preserving user edits

Problem

When editing a meeting in the meeting manage component, user edits were lost every time they navigated between steps. The form would repopulate with the original meeting data, discarding any changes the user had made.

Root Cause

The effect() hook in the meeting-manage component was watching both currentStep() and meeting() signals, and calling populateFormWithMeetingData() whenever either changed. This caused the form to reset to original values on every step navigation.

Solution

Separated the concerns into:

1. One effect for step changes (only handles validation)
2. A separate subscription using toObservable(this.meeting) with take(1) operator to populate the form only once when the meeting data initially loads

Test Plan

• Navigate to edit meeting page
• Make changes to form fields
• Navigate between steps using next/previous buttons
• Verify that user edits persist across step navigation
• Verify that the form still populates correctly on initial load
• Test both create and edit modes

Files Modified

• apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts

JIRA Ticket

LFXV2-600

🤖 Generated with Claude Code

[coderabbitai]

Walkthrough

Refactors meeting-manage component to populate the form via an observable stream gated for edit mode, adjusts attachment initialization to use a scoped destroy reference, adds explicit typing in find operations, and clarifies an effect dedicated to step-change validation with minor comment updates.

Changes

Cohort / File(s)	Summary
Meeting manage component observable/validation refactor apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts	Switch to toObservable with filter and take(1) for one-time form population in edit mode; use takeUntilDestroyed(this.destroyRef) for subscriptions; add explicit MeetingAttachment typing in find calls; retain effect labeled for step-change validation; update comments accordingly.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant Comp as MeetingManageComponent
  participant Stream as toObservable(meeting)
  participant Form as FormModel

  rect rgba(230,240,255,0.6)
    note over Comp: Initialization
    User->>Comp: Open edit meeting
    Comp->>Stream: Subscribe with filter(isEdit && meeting exists)
    Stream-->>Comp: Meeting (once via take(1))
    Comp->>Form: Populate fields
  end

  rect rgba(240,255,230,0.6)
    note over Comp: Attachments init (lifecycle-scoped)
    Comp->>Comp: initAttachments()
    Comp->>Comp: subscribe(...).pipe(takeUntilDestroyed(destroyRef))
  end

  rect rgba(255,245,230,0.6)
    note over Comp: Step change validation effect
    User->>Comp: Change step
    Comp->>Comp: Effect runs validation for step change
    Comp-->>User: Proceed or block based on validation
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• feat(meetings): add attachments display to meeting join page #104 — Similar attachment initialization refactor using toObservable and adjusted destruction handling.
• fix(meetings): resolve registrants filtering and UI display issues #76 — Related changes to meeting loading and form population logic within the same component.
• feat(permissions): implement role-based permission system #72 — Overlapping edits in meeting-manage component around step indexing/navigation and effect handling.

Suggested labels

deploy-preview

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Out of Scope Changes Check	⚠️ Warning	In addition to the scoped changes for form population and validation, the pull request also updates the attachment subscription to use takeUntilDestroyed(this.destroyRef) and adds explicit typing in find calls, which are improvements not specified in the linked issue objectives.	Consider extracting the attachment subscription update and type safety enhancements into a separate refactoring pull request to keep this change focused on the meeting form navigation fix.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title succinctly describes the main functional fix—preventing form data loss during step navigation in edit mode—and directly matches the primary change implemented in the meeting-manage component.
Linked Issues Check	✅ Passed	The implementation directly addresses the objectives of LFXV2-600 by separating the step-change validation effect from the initial form population, using toObservable(this.meeting).pipe(take(1)) to load data only once, thereby preventing form resets during navigation and preserving user edits.
Description Check	✅ Passed	The description thoroughly outlines the problem, root cause, solution, and test plan for the form data loss issue in the meeting-manage component, matching the changeset and linked issue requirements.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

✨ Finishing touches

• 📝 Generate Docstrings

🧪 Generate unit tests

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/LFXV2-600

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Jira integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 4a63dd2 and 1284ffa.

📒 Files selected for processing (1)

• apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts (5 hunks)

🧰 Additional context used

📓 Path-based instructions (3)

**/*.{ts,tsx}

📄 CodeRabbit inference engine (CLAUDE.md)

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

Files:

• apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts

**/*.{ts,tsx,js,jsx,mjs,cjs,html,css,scss}

📄 CodeRabbit inference engine (CLAUDE.md)

Include required license headers on all source files

Files:

• apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts

**/*.{ts,tsx,js,jsx}

📄 CodeRabbit inference engine (CLAUDE.md)

Do not nest ternary expressions

Files:

• apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts

🧬 Code graph analysis (1)

apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts (2)

packages/shared/src/interfaces/meeting.interface.ts (1)

• Meeting (75-148)

packages/shared/src/interfaces/meeting-attachment.interface.ts (1)

• MeetingAttachment (8-27)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)

• GitHub Check: E2E Tests / Playwright E2E Tests

🔇 Additional comments (5)

apps/lfx-one/src/app/modules/project/meetings/components/meeting-manage/meeting-manage.component.ts (5)

6-6: LGTM! Imports are correct.

The new imports toObservable and filter are necessary for the refactored meeting data population logic and are used correctly in the subscription at lines 143-146.

Also applies to: 48-48

---

134-140: LGTM! Effect correctly handles step validation only.

The updated effect now focuses solely on step-change validation, which correctly addresses the root cause of form data loss during step navigation. The separation of concerns is properly implemented.

---

226-226: LGTM! Type annotation improves type safety.

The explicit type annotation (att: MeetingAttachment) in the find() callback enhances type safety and clarity, aligning with TypeScript best practices.

---

781-781: LGTM! Explicit destroyRef improves consistency.

Using takeUntilDestroyed(this.destroyRef) with an explicit reference improves consistency across the component (see line 129) and makes the destruction handling more explicit and portable.

---

142-150: LGTM — meeting form population logic is correct and self-contained

• Subscription uses take(1) for a single emission and automatic cleanup
• Filter type guard ensures population only in edit mode with non-null data
• No other calls to populateFormWithMeetingData, preventing unintended form overrides

---

🧪 Early access (Sonnet 4.5): enabled

We are currently testing the Sonnet 4.5 model, which is expected to improve code review quality. However, this model may lead to increased noise levels in the review comments. Please disable the early access features if the noise level causes any inconvenience.

Note:

• Public repositories are always opted into early access features.
• You can enable or disable early access features from the CodeRabbit UI or by updating the CodeRabbit configuration file.

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

[Copilot]

Pull Request Overview

This PR fixes a critical user experience issue where meeting form data was being lost when users navigated between steps in edit mode. The solution separates concerns by handling step validation and form population independently.

• Decoupled step navigation effects from meeting data population logic
• Added one-time form population using toObservable with take(1) operator
• Fixed form resetting to original values on every step change

---

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

[github-actions]

✅ E2E Tests Passed

Browser: chromium
Status: passed

All E2E tests passed successfully.

Test Configuration

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