-
Notifications
You must be signed in to change notification settings - Fork 0
fix(meeting-join): refactor auto-join logic to prevent infinite loop #126
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Remove OnDestroy lifecycle and timeout cleanup - Simplify auto-join using RxJS filter and take(1) operators - Create fetchedJoinUrl signal for preemptive URL fetching - Convert join buttons to use href instead of click handlers - Add security attributes to button component links - Fix project logo centering and sizing LFXV2-669 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
|
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. WalkthroughRefactors meeting join to a reactive signal-driven flow: adds Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Template
participant MeetingComponent
participant JoinURLService
participant Browser
User->>Template: page loads / interacts
Template->>MeetingComponent: evaluate canJoinMeeting & form state
MeetingComponent->>MeetingComponent: initializeFetchedJoinUrl()
MeetingComponent->>JoinURLService: request join URL (auth/form paths)
JoinURLService-->>MeetingComponent: returns URL or error
MeetingComponent->>Template: update fetchedJoinUrl / joinUrlError / isLoadingJoinUrl
alt fetchedJoinUrl available & auto-open allowed
MeetingComponent-->>Browser: window.open(fetchedJoinUrl) (new tab)
alt popup blocked
Browser-->>MeetingComponent: popup blocked -> set joinUrlError
MeetingComponent->>Template: show popup-blocked warning
else opened
MeetingComponent->>Template: show opened state
end
end
alt user clicks manual join
Template->>Browser: navigate via anchor [href] with target/rel
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the meeting join component to eliminate an auto-join infinite loop issue by replacing effect-based logic with RxJS operators (filter and take(1)). The join buttons now function as direct links instead of using click handlers, improving user experience and popup blocker handling.
Key Changes:
- Replaced effect-based auto-join with RxJS subscription using
take(1)to prevent multiple executions - Converted join buttons from click handlers to direct
hreflinks with preemptively fetched URLs - Added security attributes (
target="_blank",rel="noopener noreferrer") to button component links
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| button.component.html | Added security attributes to anchor tags for external link safety |
| meeting-join.component.ts | Refactored auto-join logic from effect to RxJS subscription; removed deprecated methods and lifecycle hooks; added fetchedJoinUrl signal for preemptive URL fetching |
| meeting-join.component.html | Changed join buttons to use href with fetched URLs; removed auto-joined message; fixed logo centering |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/button/button.component.html
Outdated
Show resolved
Hide resolved
Signed-off-by: Asitha de Silva <asithade@gmail.com>
Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (1)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (1)
318-323: Avoid deprecated unescape() for Base64; use TextEncoder.unescape is deprecated and fails on certain Unicode. Use TextEncoder for UTF‑8 safe Base64.
Apply:
- const encodedName = btoa(unescape(encodeURIComponent(displayName))); + const encodedName = + typeof window !== 'undefined' + ? btoa(String.fromCharCode(...new TextEncoder().encode(displayName))) + : displayName; // SSR-safe fallback; optional: inject a server-side encoder if needed
📜 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 (3)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html(4 hunks)apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts(6 hunks)apps/lfx-one/src/app/shared/components/button/button.component.html(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/lfx-one/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]
Files:
apps/lfx-one/src/app/shared/components/button/button.component.htmlapps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html
**/*.{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/shared/components/button/button.component.htmlapps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.htmlapps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
**/*.{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/meeting/meeting-join/meeting-join.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (1)
packages/shared/src/interfaces/meeting-attachment.interface.ts (1)
MeetingAttachment(8-27)
🔇 Additional comments (4)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (1)
98-142: Auto‑join flow looks solid.take(1) + filter gate the action and eliminate the loop. Popup‑blocked and success toasts are handled. LGTM.
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html (3)
11-13: Logo layout tweak looks good.
239-243: Direct link strategy LGTM; depends on fetchedJoinUrl reactivity.Using [href] with disabled fallback is clean. Ensure fetchedJoinUrl updates with user/email changes per TS fix.
339-343: Form join button logic LGTM; same dependency on fetchedJoinUrl fix.
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/shared/components/button/button.component.html
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/lfx-one/src/app/shared/components/button/button.component.ts (1)
56-57: LGTM! Clean addition of navigation inputs.The new
targetandrelinputs are well-placed in the Navigation section and follow the existing component pattern. They enable secure anchor behavior (e.g.,target="_blank"withrel="noopener noreferrer") as intended.Optional: Consider adding a security helper.
For improved developer ergonomics, you could add a computed signal or helper that automatically sets
rel="noopener noreferrer"whentarget="_blank"is detected, reducing the chance of forgetting this security best practice in future usages.Example approach:
// In the component class protected readonly computedRel = computed(() => { const relValue = this.rel(); const targetValue = this.target(); if (targetValue === '_blank' && !relValue) { return 'noopener noreferrer'; } return relValue; });Then bind
[rel]="computedRel()"in the template instead of[rel]="rel()".
📜 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 (3)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html(4 hunks)apps/lfx-one/src/app/shared/components/button/button.component.html(1 hunks)apps/lfx-one/src/app/shared/components/button/button.component.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/lfx-one/src/app/shared/components/button/button.component.html
- apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html
🧰 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/shared/components/button/button.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/shared/components/button/button.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/shared/components/button/button.component.ts
LFXV2-669 - Add loading spinner while fetching join URL - Display error messages when join URL fetch fails - Separate authenticated and unauthenticated user flows - Add debouncing (300ms) to form status changes to prevent rapid re-fetches - Improve user feedback with loading states in both authenticated and form sections - Reset error state on each fetch attempt Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 5 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
♻️ Duplicate comments (2)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (2)
100-145: Auto-join still uses programmatic window.open(), conflicting with PR objectives.The PR objectives state: "Replace programmatic popup opening and manual auto-join (openMeetingSecurely(), performAutoJoin()) with direct link navigation from buttons." However, line 123 still programmatically opens a window using
window.open(), which may be blocked by popup blockers and conflicts with the stated design goal of using direct link navigation.Consider removing the auto-join feature entirely or documenting why programmatic opening is still necessary despite the PR's goal to eliminate it.
356-437: URL fetch doesn't react to form value or user changes.The
initializeFetchedJoinUrlmethod only reacts tocanJoinMeetingandform.statusChanges(line 358). This creates reactivity gaps:
- Authenticated users: If the user signal changes after initial load, the URL won't refetch.
- Unauthenticated users: If the form is already VALID and the user changes the email field,
statusChangeswon't emit (status remains VALID), so the URL won't refetch with the new email.This can cause the "Join Meeting" button to remain disabled even when all conditions are met.
The previous review provided a comprehensive solution using
combineLatestwith all relevant signals including formvalueChanges. Please refer to that comment for the detailed fix.
🧹 Nitpick comments (2)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html (2)
215-263: Simplify href binding to avoid ternary expression.The conditional href binding on line 252 uses a ternary expression with an
&&guard. While not deeply nested, the coding guidelines prohibit nested ternary expressions. Consider using computed signals in the TypeScript component for cleaner template bindings.As per coding guidelines.
Alternative approach in the component:
public authenticatedJoinHref: Signal<string | undefined> = computed(() => { return this.canJoinMeeting() && this.fetchedJoinUrl() ? this.fetchedJoinUrl() : undefined; });Then in the template:
-[href]="canJoinMeeting() && fetchedJoinUrl() ? fetchedJoinUrl() : undefined" +[href]="authenticatedJoinHref()"
360-365: Extract complex conditional bindings to computed signals.The href binding on line 360 contains multiple conditions combined with ternary logic, and the disabled binding on line 361 is also complex with four conditions. These violate the guideline against nested ternary expressions and hurt readability.
As per coding guidelines.
Extract to computed signals in the component:
public formJoinHref: Signal<string | undefined> = computed(() => { return this.joinForm.valid && this.canJoinMeeting() && this.fetchedJoinUrl() ? this.fetchedJoinUrl() : undefined; }); public formJoinDisabled: Signal<boolean> = computed(() => { return this.joinForm.invalid || !this.canJoinMeeting() || !this.fetchedJoinUrl() || this.isLoadingJoinUrl(); });Then simplify the template:
-[href]="joinForm.valid && canJoinMeeting() && fetchedJoinUrl() ? fetchedJoinUrl() : undefined" -[disabled]="joinForm.invalid || !canJoinMeeting() || !fetchedJoinUrl() || isLoadingJoinUrl()" +[href]="formJoinHref()" +[disabled]="formJoinDisabled()"
📜 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 (2)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html(3 hunks)apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/lfx-one/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]
Files:
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html
**/*.{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/meeting/meeting-join/meeting-join.component.htmlapps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
**/*.{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/meeting/meeting-join/meeting-join.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (1)
packages/shared/src/interfaces/meeting-attachment.interface.ts (1)
MeetingAttachment(8-27)
🔇 Additional comments (1)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html (1)
11-13: LGTM! Logo centering improved.The flexbox wrapper with
items-center justify-centerproperly centers the logo, and changing fromw-fulltow-autoprevents distortion.
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
Outdated
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
Outdated
Show resolved
Hide resolved
Simplified button href and disabled bindings by removing redundant canJoinMeeting() checks since fetchedJoinUrl() already returns undefined when the meeting cannot be joined. Refactored initializeFetchedJoinUrl() to eliminate code duplication: - Extracted common join URL fetching logic into fetchJoinUrl() method - Removed duplicate API call handling for authenticated/unauthenticated flows - Removed signal mutation anti-pattern (meeting.join_url assignment) - Improved maintainability with single source of truth for fetch logic Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (2)
100-145: Remove programmatic auto-join (window.open) to align with PR goals.
Direct-link navigation is the new strategy; keeping auto-open risks popup blockers and contradicts the spec.- private hasAutoJoined: WritableSignal<boolean> = signal<boolean>(false); @@ - this.initializeAutoJoin(); + // Auto-join removed: users click explicit links per design @@ - private initializeAutoJoin(): void { - // Use toObservable to create an Observable from the signals, then subscribe once - // This executes only when all conditions are met - toObservable(this.fetchedJoinUrl) - .pipe( - // Take only the first emission where we have a valid URL and haven't auto-joined yet - filter((url) => { - const authenticated = this.authenticated(); - const user = this.user(); - const canJoin = this.canJoinMeeting(); - const alreadyJoined = this.hasAutoJoined(); - - return !!url && authenticated && !!user && !!user.email && canJoin && !alreadyJoined; - }), - // Take only the first valid URL - take(1) - ) - .subscribe((url) => { - // Mark as auto-joined to prevent multiple attempts - this.hasAutoJoined.set(true); - - // Open the meeting URL in a new tab - if (typeof window !== 'undefined' && url) { - const newWindow = window.open(url, '_blank', 'noopener,noreferrer'); - - // Check if popup was blocked - if (!newWindow || newWindow.closed || typeof newWindow.closed === 'undefined') { - // Popup was blocked - this.messageService.add({ - severity: 'warn', - summary: 'Popup Blocked', - detail: 'Your browser blocked the meeting window. Please click the "Join Meeting" button to open it manually.', - life: 5000, - }); - } else { - // Popup opened successfully - this.messageService.add({ - severity: 'success', - summary: 'Meeting Opened', - detail: 'The meeting has been opened in a new tab.', - life: 3000, - }); - } - } - }); - }#!/bin/bash # Ensure no lingering programmatic opens remain rg -nP "window\.open\s*\(" -g "!**/node_modules/**"Also applies to: 76-77, 97-97
356-403: Remove nested ternary expression; refactor email logic to if-else structure to comply with coding guidelines.The review's core concern is valid: email, user, and authenticated changes aren't monitored reactively, causing stale join URLs. The proposed refactor is architecturally sound but violates the coding guideline prohibiting nested ternaries (line in proposed diff:
const effectiveEmail = authenticated ? user?.email : (formStatus === 'VALID' ? email : undefined);).Rewrite the logic using if-else instead:
private initializeFetchedJoinUrl(): Signal<string | undefined> { const canJoin$ = toObservable(this.canJoinMeeting); const authenticated$ = toObservable(this.authenticated); const user$ = toObservable(this.user); const status$ = this.joinForm.statusChanges.pipe(startWith(this.joinForm.status)); const emailControl = this.joinForm.get('email') as FormControl<string>; const email$ = emailControl.valueChanges.pipe( startWith(emailControl.value), debounceTime(200), distinctUntilChanged() ); return toSignal( combineLatest([canJoin$, authenticated$, user$, status$, email$]).pipe( map(([canJoin, authenticated, user, formStatus, email]) => ({ canJoin, authenticated, user, formStatus, email })), distinctUntilChanged( (a, b) => a.canJoin === b.canJoin && a.authenticated === b.authenticated && a.user?.email === b.user?.email && a.formStatus === b.formStatus && a.email === b.email ), switchMap(({ canJoin, authenticated, user, formStatus, email }) => { const meeting = this.meeting(); // Reset error state this.joinUrlError.set(null); // Only fetch when meeting is joinable and we have necessary user info if (!canJoin || !meeting?.uid) { this.isLoadingJoinUrl.set(false); return of(undefined); } // Determine effective email let effectiveEmail: string | undefined; if (authenticated) { effectiveEmail = user?.email; } else if (formStatus === 'VALID') { effectiveEmail = email; } if (!effectiveEmail) { this.isLoadingJoinUrl.set(false); return of(undefined); } // Fetch join URL with the determined email return this.fetchJoinUrl(meeting, effectiveEmail); }) ), { initialValue: undefined } ); }And add the missing import:
-import { catchError, combineLatest, debounceTime, filter, map, Observable, of, startWith, switchMap, take, tap } from 'rxjs'; +import { catchError, combineLatest, debounceTime, distinctUntilChanged, filter, map, Observable, of, startWith, switchMap, take, tap } from 'rxjs';
🧹 Nitpick comments (2)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html (2)
11-13: Add image loading hints for perf.
Lazy-load and async-decode the project logo.- <div class="flex items-center justify-center"> - <img src="{{ project()?.logo_url }}" alt="{{ project()?.name }}" class="w-auto h-20 mb-6" /> - </div> + <div class="flex items-center justify-center"> + <img + src="{{ project()?.logo_url }}" + alt="{{ project()?.name }}" + class="w-auto h-20 mb-6" + loading="lazy" + decoding="async" + /> + </div>
223-227: Expose status changes to AT users.
Mark the status message as polite live region; indicate loading state.- <lfx-message + <lfx-message [severity]="messageSeverity()" [icon]="messageIcon()" - styleClass="flex items-center justify-between w-full" + styleClass="flex items-center justify-between w-full" + [attr.aria-live]="'polite'" + [attr.aria-busy]="isLoadingJoinUrl()" [attr.data-testid]="'join-status-message'">
📜 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 (3)
.vscode/settings.json(1 hunks)apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html(3 hunks)apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts(6 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/lfx-one/src/**/*.html
📄 CodeRabbit inference engine (CLAUDE.md)
apps/lfx-one/src/**/*.html: Always add data-testid attributes when creating new Angular components for reliable test targeting
Use data-testid naming convention [section]-[component]-[element]
Files:
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.html
**/*.{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/meeting/meeting-join/meeting-join.component.htmlapps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
**/*.{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/meeting/meeting-join/meeting-join.component.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not nest ternary expressions
Files:
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts
🧬 Code graph analysis (1)
apps/lfx-one/src/app/modules/meeting/meeting-join/meeting-join.component.ts (2)
packages/shared/src/interfaces/meeting-attachment.interface.ts (1)
MeetingAttachment(8-27)packages/shared/src/interfaces/meeting.interface.ts (1)
Meeting(75-150)
🔇 Additional comments (1)
.vscode/settings.json (1)
23-24: LGTM — dictionary additions align with link security work.
No issues.
…126) * fix(meeting-join): refactor auto-join logic to prevent infinite loop - Remove OnDestroy lifecycle and timeout cleanup - Simplify auto-join using RxJS filter and take(1) operators - Create fetchedJoinUrl signal for preemptive URL fetching - Convert join buttons to use href instead of click handlers - Add security attributes to button component links - Fix project logo centering and sizing LFXV2-669 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(ui): pass in link target and rel for buttons Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(ui): pass in link target and rel for buttons Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(meeting-join): add loading and error handling for join URL LFXV2-669 - Add loading spinner while fetching join URL - Display error messages when join URL fetch fails - Separate authenticated and unauthenticated user flows - Add debouncing (300ms) to form status changes to prevent rapid re-fetches - Improve user feedback with loading states in both authenticated and form sections - Reset error state on each fetch attempt Signed-off-by: Asitha de Silva <asithade@gmail.com> * refactor(meeting-join): simplify bindings and eliminate duplication Simplified button href and disabled bindings by removing redundant canJoinMeeting() checks since fetchedJoinUrl() already returns undefined when the meeting cannot be joined. Refactored initializeFetchedJoinUrl() to eliminate code duplication: - Extracted common join URL fetching logic into fetchJoinUrl() method - Removed duplicate API call handling for authenticated/unauthenticated flows - Removed signal mutation anti-pattern (meeting.join_url assignment) - Improved maintainability with single source of truth for fetch logic Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com> --------- Signed-off-by: Asitha de Silva <asithade@gmail.com>
…126) * fix(meeting-join): refactor auto-join logic to prevent infinite loop - Remove OnDestroy lifecycle and timeout cleanup - Simplify auto-join using RxJS filter and take(1) operators - Create fetchedJoinUrl signal for preemptive URL fetching - Convert join buttons to use href instead of click handlers - Add security attributes to button component links - Fix project logo centering and sizing LFXV2-669 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(ui): pass in link target and rel for buttons Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(ui): pass in link target and rel for buttons Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(meeting-join): add loading and error handling for join URL LFXV2-669 - Add loading spinner while fetching join URL - Display error messages when join URL fetch fails - Separate authenticated and unauthenticated user flows - Add debouncing (300ms) to form status changes to prevent rapid re-fetches - Improve user feedback with loading states in both authenticated and form sections - Reset error state on each fetch attempt Signed-off-by: Asitha de Silva <asithade@gmail.com> * refactor(meeting-join): simplify bindings and eliminate duplication Simplified button href and disabled bindings by removing redundant canJoinMeeting() checks since fetchedJoinUrl() already returns undefined when the meeting cannot be joined. Refactored initializeFetchedJoinUrl() to eliminate code duplication: - Extracted common join URL fetching logic into fetchJoinUrl() method - Removed duplicate API call handling for authenticated/unauthenticated flows - Removed signal mutation anti-pattern (meeting.join_url assignment) - Improved maintainability with single source of truth for fetch logic Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com> --------- Signed-off-by: Asitha de Silva <asithade@gmail.com>
…126) * fix(meeting-join): refactor auto-join logic to prevent infinite loop - Remove OnDestroy lifecycle and timeout cleanup - Simplify auto-join using RxJS filter and take(1) operators - Create fetchedJoinUrl signal for preemptive URL fetching - Convert join buttons to use href instead of click handlers - Add security attributes to button component links - Fix project logo centering and sizing LFXV2-669 Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(ui): pass in link target and rel for buttons Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(ui): pass in link target and rel for buttons Signed-off-by: Asitha de Silva <asithade@gmail.com> * fix(meeting-join): add loading and error handling for join URL LFXV2-669 - Add loading spinner while fetching join URL - Display error messages when join URL fetch fails - Separate authenticated and unauthenticated user flows - Add debouncing (300ms) to form status changes to prevent rapid re-fetches - Improve user feedback with loading states in both authenticated and form sections - Reset error state on each fetch attempt Signed-off-by: Asitha de Silva <asithade@gmail.com> * refactor(meeting-join): simplify bindings and eliminate duplication Simplified button href and disabled bindings by removing redundant canJoinMeeting() checks since fetchedJoinUrl() already returns undefined when the meeting cannot be joined. Refactored initializeFetchedJoinUrl() to eliminate code duplication: - Extracted common join URL fetching logic into fetchJoinUrl() method - Removed duplicate API call handling for authenticated/unauthenticated flows - Removed signal mutation anti-pattern (meeting.join_url assignment) - Improved maintainability with single source of truth for fetch logic Generated with [Claude Code](https://claude.ai/code) Signed-off-by: Asitha de Silva <asithade@gmail.com> --------- Signed-off-by: Asitha de Silva <asithade@gmail.com> Signed-off-by: Mauricio Zanetti Salomao <msalomao@contractor.linuxfoundation.org>
Summary
Refactor the meeting join component to fix the auto-join infinite loop issue and improve user experience by using direct link navigation instead of programmatic window opening.
Changes
Meeting Join Component (meeting-join.component.ts)
OnDestroylifecycle hook and auto-join timeout cleanup logicisJoiningsignal andonJoinMeeting()methodfilterandtake(1)operators to prevent multiple executionsfetchedJoinUrlsignal that preemptively fetches the join URL when conditions are metopenMeetingSecurely()andperformAutoJoin())Meeting Join Component (meeting-join.component.html)
w-fulltow-autohrefattribute withfetchedJoinUrl()instead of click handlersButton Component (button.component.html)
target="_blank"andrel="noopener noreferrer"to anchor tags for securityBenefits
take(1)operatorTesting
Related
Closes LFXV2-669
Generated with Claude Code