feat(events): add visa letter and travel fund application dialogs#415
Open
feat(events): add visa letter and travel fund application dialogs#415
Conversation
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds multi-step visa and travel-fund application flows (dialogs, forms, dashboards), event-selection UI, new frontend service methods and server endpoints for listing/submitting requests, shared interfaces/constants, router/route adjustments, and assorted template/style updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User / Browser
participant Dialog as TravelFundDialog
participant EventSel as EventSelection
participant About as AboutMeForm
participant Expenses as ExpensesForm
participant Service as EventsService
participant API as Backend API
User->>Dialog: Open travel fund dialog
Dialog->>EventSel: render select-event step
EventSel->>Service: fetch events & countries
Service->>API: GET /api/events, /api/events/countries
API-->>Service: events + countries
Service-->>EventSel: events list
User->>EventSel: select event
EventSel-->>Dialog: selectedEvent
User->>Dialog: Next -> about-me
Dialog->>About: render about-me form
About->>About: prefill user data
User->>About: fill form
About-->>Dialog: formValidityChange + formData
User->>Dialog: Next -> expenses
Dialog->>Expenses: render expenses form
User->>Expenses: fill costs
Expenses-->>Dialog: expensesData
User->>Dialog: Submit
Dialog->>Service: submitTravelFundApplication(payload)
Service->>API: POST /api/events/travel-fund-applications
API-->>Service: { success: true }
Service-->>Dialog: success
Dialog->>User: show success & close
sequenceDiagram
participant User as User / Browser
participant Dialog as VisaDialog
participant EventSel as EventSelection
participant Apply as VisaApplyForm
participant Service as EventsService
participant API as Backend API
User->>Dialog: Open visa request dialog
Dialog->>EventSel: render select-event step
EventSel->>Service: fetch events & countries
Service->>API: GET /api/events, /api/events/countries
API-->>Service: events + countries
Service-->>EventSel: events list
User->>EventSel: select event
EventSel-->>Dialog: selectedEvent
User->>Dialog: Next -> apply
Dialog->>Apply: render applicant form
Apply->>Apply: prefill user data
User->>Apply: fill passport & address
Apply-->>Dialog: formValidityChange + applicantData
User->>Dialog: Submit
Dialog->>Service: submitVisaRequestApplication(payload)
Service->>API: POST /api/events/visa-applications
API-->>Service: { success: true }
Service-->>Dialog: success
Dialog->>User: show success & close
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
github-actions
bot
requested review from
andrest50,
asithade,
dealako,
jordane and
mauriciozanettisalomao
Contributor
There was a problem hiding this comment.
Pull request overview
Adds “Visa Letter” and “Travel Funding” request flows to the My Events area, including new multi-step application dialogs, shared event selection/filtering UI, and corresponding (currently stubbed) API endpoints to submit and list requests.
Changes:
- Added shared request/application interfaces and extended My Events query params (registered-only + date/country filters).
- Implemented new server endpoints for request listings, country filters, and stub submission handlers for visa letters and travel funding.
- Added new My Events tabs/components for viewing requests and launching multi-step application dialogs using a shared event-selection component.
Reviewed changes
Copilot reviewed 46 out of 46 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/visa-request-application.interface.ts | Adds shared types for visa letter application payload/response. |
| packages/shared/src/interfaces/travel-fund-application.interface.ts | Adds shared types for travel fund application payload/response. |
| packages/shared/src/interfaces/my-event.interface.ts | Extends MyEvent with startDate; adds request/country/query option interfaces. |
| packages/shared/src/interfaces/index.ts | Re-exports the new application interfaces. |
| packages/shared/src/constants/events.constants.ts | Adds request status options, request sort fields, and empty responses. |
| apps/lfx-one/src/server/services/events.service.ts | Implements request list queries, country list query, and stub submit handlers; adds new My Events filters. |
| apps/lfx-one/src/server/routes/events.route.ts | Registers new API routes for countries, request lists, and application submissions. |
| apps/lfx-one/src/server/pdf-templates/visa-letter-manual/template.html | Updates visa-letter PDF template (includes whitespace-only changes). |
| apps/lfx-one/src/server/controllers/events.controller.ts | Adds controllers for new request/country endpoints and submission endpoints. |
| apps/lfx-one/src/app/shared/services/events.service.ts | Adds Angular client methods for new endpoints and extended My Events filters. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/my-events-dashboard.component.ts | Adds request-tab awareness and switches status options based on active tab. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/my-events-dashboard.component.html | Wires top-bar inputs to hide/show filters based on request tabs. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.ts | Adds Visa Letters tab table + dialog launcher. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.scss | New stylesheet (license header). |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.html | Visa Letters table markup with sorting/pagination/empty state. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-terms/visa-request-terms.component.ts | Adds Visa Letter terms step component (placeholder text). |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-terms/visa-request-terms.component.scss | New stylesheet (license header). |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-terms/visa-request-terms.component.html | Visa Letter terms placeholder UI. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.ts | Adds visa applicant info reactive form. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.scss | New stylesheet (license header). |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.html | Visa applicant info form template. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.ts | Adds 3-step visa application dialog orchestration + submit call. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.scss | Dialog scroll styling. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.html | Stepper UI and dialog footer actions. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.ts | Adds Travel Funding tab table + dialog launcher. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.scss | New stylesheet (license header). |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.html | Travel funding requests table markup with sorting/pagination/empty state. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-terms/travel-fund-terms.component.ts | Adds Travel Fund terms step component. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-terms/travel-fund-terms.component.scss | New stylesheet (license header). |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-terms/travel-fund-terms.component.html | Travel Fund terms/consent text. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.ts | Adds 4-step travel fund dialog orchestration + submit call. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.scss | Dialog scroll styling. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.html | Stepper UI and dialog footer actions. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.ts | Adds travel expenses form + estimated total computation. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.scss | New stylesheet (license header). |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.html | Expenses table UI + reminder banner. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/events-list/events-list.component.ts | Wires new request tab components into tab content. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/events-list/events-list.component.html | Renders visa/travel tab content instead of “Coming soon”. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.ts | New shared event picker with search/time/location filters + pagination. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.scss | Scroll + card selection styling. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.html | Event grid UI, filters, load-more, and empty state prompt. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.ts | Adds “About Me” form for travel fund application. |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.scss | New stylesheet (license header). |
| apps/lfx-one/src/app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.html | “About Me” form template. |
| apps/lfx-one/src/app/modules/events/foundation-event-dashboard/components/events-table/events-table.component.html | Formatting-only tag markup change. |
| apps/lfx-one/src/app/modules/events/components/events-top-bar/events-top-bar.component.ts | Clears status/role when filters/options change to avoid stale selections. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
...modules/events/my-events-dashboard/components/event-selection/event-selection.component.html
Outdated
Show resolved
Hide resolved
...p/modules/events/my-events-dashboard/components/event-selection/event-selection.component.ts
Outdated
Show resolved
Hide resolved
...p/modules/events/my-events-dashboard/components/event-selection/event-selection.component.ts
Outdated
Show resolved
Hide resolved
...hboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/app/modules/events/my-events-dashboard/my-events-dashboard.component.ts
Show resolved
Hide resolved
apps/lfx-one/src/server/pdf-templates/visa-letter-manual/template.html
Outdated
Show resolved
Hide resolved
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (11)
apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-terms/travel-fund-terms.component.ts (1)
12-12: Add a short JSDoc for the exported component class.A one-line class doc improves discoverability and aligns with repo standards for exported modules.
As per coding guidelines: "Add JSDoc comments for public functions and exported modules".📝 Suggested update
+/** + * Terms step content for the travel fund application dialog. + */ export class TravelFundTermsComponent {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-terms/travel-fund-terms.component.ts` at line 12, Add a one-line JSDoc comment for the exported TravelFundTermsComponent class explaining its purpose; locate the exported symbol TravelFundTermsComponent and add a concise /** ... */ JSDoc directly above the class declaration describing what the component represents or does (e.g., "Component displaying terms for the travel fund on the My Events dashboard"), following repo style for exported modules.apps/lfx-one/src/server/pdf-templates/visa-letter-manual/template.html (1)
96-133: Collapse excessive whitespace inside the Handlebars conditional.This keeps the template easier to read and less error-prone during future edits.
♻️ Suggested cleanup
- {{`#if` - event.state - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - }}{{ event.state }}, {{/if}} {{`#if` event.country }}{{ event.country }}{{/if}} + {{`#if` event.state }}{{ event.state }}, {{/if}} {{`#if` event.country }}{{ event.country }}{{/if}}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/server/pdf-templates/visa-letter-manual/template.html` around lines 96 - 133, The Handlebars conditional for event.state contains excessive internal whitespace; replace the spaced-out block with a compact conditional like {{`#if` event.state}}{{ event.state }},{{/if}} and similarly ensure the following {{`#if` event.country}}{{ event.country }}{{/if}} is compacted so the template no longer contains large blank regions between the {{`#if` event.state}} and its closing }} token; update the occurrences of the '{{`#if` event.state}}...{{/if}}' and '{{`#if` event.country}}...{{/if}}' constructs in template.html accordingly.apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-terms/visa-request-terms.component.ts (1)
12-12: Add a brief JSDoc on the exported component class.This keeps exported modules self-documented per repo standards.
As per coding guidelines: "Add JSDoc comments for public functions and exported modules".Proposed change
`@Component`({ selector: 'lfx-visa-request-terms', templateUrl: './visa-request-terms.component.html', styleUrl: './visa-request-terms.component.scss', changeDetection: ChangeDetectionStrategy.OnPush, }) +/** Renders visa letter request terms-and-conditions content in the application dialog. */ export class VisaRequestTermsComponent {}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-terms/visa-request-terms.component.ts` at line 12, Add a brief JSDoc block immediately above the exported class VisaRequestTermsComponent describing the component's purpose and role (one-line summary plus a short sentence), include any relevant params/props or usage notes if applicable, and tag it as exported/public per repo standards (e.g., use a leading /** ... */ with a description and optional `@export` or `@public` tag). Ensure the JSDoc sits directly above the "export class VisaRequestTermsComponent" declaration so the component is self-documented.apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.html (2)
65-68: MissingdataTestattribute on calendar input.For consistency with other form inputs that have
dataTestattributes, consider adding one to the passport expiry date calendar.♻️ Suggested fix
<div class="flex flex-col gap-1"> <label class="text-sm font-medium text-gray-700">Passport Expiry Date <span class="text-red-500">*</span></label> - <lfx-calendar [form]="form" control="passportExpiryDate" placeholder="MM/DD/YYYY" styleClass="w-full" /> + <lfx-calendar [form]="form" control="passportExpiryDate" placeholder="MM/DD/YYYY" styleClass="w-full" dataTest="visa-apply-passport-expiry" /> </div>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.html` around lines 65 - 68, The lfx-calendar used for the passport expiry date is missing a dataTest attribute; update the lfx-calendar element (the one with control="passportExpiryDate" in the VisaRequestApplyForm template) to include a descriptive dataTest value (e.g., "passportExpiryDate" or "passport-expiry-date-input") so automated tests and selectors can target it consistently.
39-49: Consider addingautocomplete="off"for sensitive passport field.The passport number field contains sensitive personal information. Adding
autocomplete="off"can help prevent browsers from storing and suggesting this data.🔒 Suggested enhancement
<lfx-input-text [form]="form" control="passportNumber" id="visa-passport-number" size="small" placeholder="Enter passport number" styleClass="w-full" + autocomplete="off" dataTest="visa-apply-passport-number" />Note: Verify that
lfx-input-textsupports theautocompleteattribute pass-through.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.html` around lines 39 - 49, Add autocomplete="off" to the passport input element: update the lfx-input-text tag that binds to control="passportNumber" (id="visa-passport-number") so the browser won’t autofill this sensitive field; if lfx-input-text does not pass attributes through, modify the lfx-input-text component to accept and forward an autocomplete input/property to the native input element (or expose a dedicated `@Input` like autocomplete for that component) and ensure the form binding (form, control="passportNumber") remains unchanged.apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.html (1)
26-29: Consider preventing negative cost values.The
type="number"inputs for cost fields (airfare, hotel, ground transport) allow users to enter negative values by default. If negative costs are invalid for this use case, consider addingmin="0"to prevent negative input at the HTML level, or add validation in the form component.♻️ Suggested enhancement for one of the cost inputs
<div class="flex items-center gap-1"> <span class="text-sm text-gray-500">$</span> - <lfx-input-text [form]="form" control="airfareCost" type="number" size="small" styleClass="w-full" dataTest="expenses-airfare-cost" /> + <lfx-input-text [form]="form" control="airfareCost" type="number" [min]="0" size="small" styleClass="w-full" dataTest="expenses-airfare-cost" /> </div>Note: Verify that
lfx-input-textsupports theminattribute pass-through before applying.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.html` around lines 26 - 29, The airfare cost input (lfx-input-text with control="airfareCost") currently allows negative numbers; add a min="0" attribute to that lfx-input-text (and the equivalent hotel and ground transport inputs: controls "hotelCost" and "groundTransportCost") to prevent negative entry at the HTML level if lfx-input-text forwards attributes, and also add form-level validation in the TravelExpensesFormComponent FormGroup by attaching Validators.min(0) to the FormControl definitions for airfareCost, hotelCost, and groundTransportCost to ensure negatives are rejected even if the input bypasses the attribute.apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.html (1)
43-54: Minor: Inconsistent scroll container across steps.The
scroll-areaclass withoverflow-y-autois only applied to theselect-eventstep content (line 45), whiletermsandapplysteps don't have this wrapper. If those steps can have content that exceeds the dialog height, consider wrapping them consistently.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.html` around lines 43 - 54, The dialog applies the scroll container only for the 'select-event' branch, causing inconsistent scrolling for other steps; wrap the content of every step rendered by step() — including the lfx-event-selection, lfx-visa-request-terms, and lfx-visa-request-apply-form branches — in the same <div class="scroll-area overflow-y-auto pr-1"> wrapper so all steps get consistent overflow-y-auto behavior and identical layout; update the template around the step() checks to reuse that container for each branch while leaving the existing bindings ([(selectedEvent)]="selectedEvent", (formValidityChange)="applyFormValid.set($event)", (formChange)="applicantData.set($event)") unchanged.apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.html (2)
4-12: Same simplification opportunity as travel-funding.The static
data-testidcan be a plain attribute instead of using[attr.data-testid].✨ Suggested simplification
<lfx-button label="New Letter Application" icon="fa-light fa-plus" severity="primary" size="small" (onClick)="openApplicationDialog()" - [attr.data-testid]="'visa-request-discover-button'" /> + data-testid="visa-request-discover-button" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.html` around lines 4 - 12, The template uses a bound attribute for a static test id; change the lfx-button's [attr.data-testid] binding to a plain static attribute so the test id is not bound at runtime—for the lfx-button element that calls openApplicationDialog(), replace the [attr.data-testid]="'visa-request-discover-button'" binding with a simple data-testid="visa-request-discover-button" attribute to simplify the markup.
30-53: Inconsistentdata-testidnaming for sort headers.The sort header test IDs use different naming patterns between this component and
travel-funding.component.html:
- Here:
sort-event-name,sort-location,sort-application-date- Travel funding:
travel-funding-sort-event-name,travel-funding-sort-location, etc.For consistency and to avoid selector collisions in tests, consider prefixing with
visa-request-:✨ Suggested naming consistency
<th scope="col" [attr.aria-sort]="sortField() === 'EVENT_NAME' ? (sortOrder() === 'ASC' ? 'ascending' : 'descending') : 'none'" - data-testid="sort-event-name"> + data-testid="visa-request-sort-event-name"> <button type="button" class="flex cursor-pointer select-none items-center gap-1" (click)="onHeaderClick('EVENT_NAME')"> Event Name <i [class]="sortIcons()['EVENT_NAME']"></i> </button> </th> <th scope="col" [attr.aria-sort]="sortField() === 'EVENT_CITY' ? (sortOrder() === 'ASC' ? 'ascending' : 'descending') : 'none'" - data-testid="sort-location"> + data-testid="visa-request-sort-location"> <button type="button" class="flex cursor-pointer select-none items-center gap-1" (click)="onHeaderClick('EVENT_CITY')"> Location <i [class]="sortIcons()['EVENT_CITY']"></i> </button> </th> <th scope="col" [attr.aria-sort]="sortField() === 'APPLICATION_DATE' ? (sortOrder() === 'ASC' ? 'ascending' : 'descending') : 'none'" - data-testid="sort-application-date"> + data-testid="visa-request-sort-application-date"> <button type="button" class="flex cursor-pointer select-none items-center gap-1" (click)="onHeaderClick('APPLICATION_DATE')"> Application Date <i [class]="sortIcons()['APPLICATION_DATE']"></i> </button> </th>🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.html` around lines 30 - 53, The data-testid attributes for the table header sort buttons (currently "sort-event-name", "sort-location", "sort-application-date") are inconsistent with the travel-funding component; update them to a consistent, unique prefix such as "visa-request-sort-event-name", "visa-request-sort-location", and "visa-request-sort-application-date" in the template where the <button> elements call onHeaderClick('EVENT_NAME'/'EVENT_CITY'/'APPLICATION_DATE') and use sortField(), sortOrder(), and sortIcons() so tests can reliably target these headers without collisions.apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.html (1)
4-12: Simplifydata-testidattribute binding.Using
[attr.data-testid]="'travel-funding-discover-button'"with a static string is unnecessary. You can use the plain attribute directly.✨ Suggested simplification
<lfx-button label="New Funding Application" icon="fa-light fa-plus" severity="primary" size="small" (onClick)="openApplicationDialog()" - [attr.data-testid]="'travel-funding-discover-button'" /> + data-testid="travel-funding-discover-button" />🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.html` around lines 4 - 12, The data-testid binding on the lfx-button is using an unnecessary property binding with a static string; in the travel-funding template change the lfx-button attribute from [attr.data-testid]="'travel-funding-discover-button'" to a plain data-testid="travel-funding-discover-button" (leave the lfx-button element, its label and the openApplicationDialog() click handler untouched).apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.ts (1)
41-52: Consider emitting initialformChangevalue.The
formValidityChangeemitstrueimmediately in the constructor, butformChangeonly emits whenvalueChangesfires. If the parent component needs the initial expense values (all zeros), it won't receive them until the user interacts with the form.This may be intentional if the parent's
expensesDatasignal has a suitable default, but it's worth considering for consistency withaboutMeFormComponentpatterns.✨ Optional: emit initial form value
public constructor() { // No required fields — form is always valid this.formValidityChange.emit(true); + this.formChange.emit(this.buildExpensesValue()); this.form.statusChanges.pipe(takeUntilDestroyed()).subscribe(() => { this.formValidityChange.emit(this.form.valid); }); this.form.valueChanges.pipe(takeUntilDestroyed()).subscribe(() => { this.formChange.emit(this.buildExpensesValue()); }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.ts` around lines 41 - 52, Emit the initial form value in the constructor to match the immediate formValidityChange behavior: after the existing this.formValidityChange.emit(true) call, call this.formChange.emit(this.buildExpensesValue()) so the parent receives the initial expenses (all zeros) without waiting for user interaction; keep existing subscriptions for this.form.statusChanges and this.form.valueChanges unchanged and reference the constructor, formChange, buildExpensesValue(), and formValidityChange symbols.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.ts`:
- Around line 56-75: The component currently only emits validity and value on
statusChanges/valueChanges, so initial state isn’t sent; update the
subscriptions in the constructor that use this.form.statusChanges and
this.form.valueChanges to prepend the current state using startWith — for
statusChanges use startWith(this.form.status) (and continue emitting
this.form.valid to formValidityChange.emit), and for valueChanges use
startWith(this.buildFormValue()) (and continue emitting the built value to
formChange.emit), keeping takeUntilDestroyed() and existing emit calls
(references: constructor, this.form.statusChanges, this.form.valueChanges,
formValidityChange.emit, formChange.emit, buildFormValue).
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.scss`:
- Around line 5-7: Add an empty line between the `@apply` rule and the scrollbar
declarations and replace the hardcoded hex in scrollbar-color with Tailwind's
theme() reference for consistency; specifically, update the block containing
`@apply` max-h-[340px]; so it has a blank line before scrollbar-width: thin; and
change scrollbar-color: `#e2e8f0` transparent; to use the theme token (e.g.,
scrollbar-color: theme('colors.gray.200') transparent;) ensuring you modify the
declarations named scrollbar-width and scrollbar-color in the
event-selection.component.scss.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.ts`:
- Line 53: Update the user-facing label for the default location option in the
EventSelection component: change the availableLocations signal's first item's
label from "Any Where" to "Anywhere" (and make the same change for the other
occurrence in this file). Locate the availableLocations declaration in
event-selection.component (and the duplicate at the second occurrence) and
replace the label string to "Anywhere" so the UI shows the correct copy.
- Around line 88-102: onLoadMore advances currentOffset before the network call
and uses EMPTY_MY_EVENTS_RESPONSE on errors, which can skip pages and reset
totalFromServer; fix by computing a local requestOffset = this.currentOffset +
PAGE_SIZE (or use currentOffset as-is) and pass that into
eventsService.getMyEvents without mutating currentOffset before the call, then
only increment this.currentOffset += PAGE_SIZE inside the successful subscribe
handler (eventSelection.component.ts: onLoadMore, PAGE_SIZE, currentOffset,
eventsService.getMyEvents, EMPTY_MY_EVENTS_RESPONSE), and ensure
totalFromServer.set(response.total) is only called when response is a real
successful payload (do not update totalFromServer on the error fallback).
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.scss`:
- Around line 5-7: In travel-fund-application-dialog.component.scss adjust
spacing to satisfy stylelint by inserting a blank line before the declaration
"scrollbar-width: thin;" (i.e., add an empty line between the "@apply
max-h-[440px];" rule and the "scrollbar-width" declaration) so the
declaration-empty-line-before rule is not triggered for the scrollbar rules.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.ts`:
- Around line 91-103: The next handler for submitTravelFundApplication currently
treats any HTTP 2xx as success; update the subscribe next callback to accept the
TravelFundApplicationResponse, check response.success before showing success UI
and calling this.ref.close, and if response.success is false call
this.submitting.set(false) and show an error via this.messageService.add (using
response.message if present) instead of closing the dialog; keep the error
callback as-is for network failures.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.ts`:
- Around line 101-106: The loading flag is set outside the switchMap causing
finalize on cancelled inner observables to clear loading prematurely; move the
loading.set(true) into the inner stream so it is tied to each request.
Specifically, inside the switchMap that invokes
eventsService.getTravelFundRequests({ offset, pageSize, searchQuery, status,
sortField, sortOrder }) set loading.set(true) before subscribing to that inner
observable (e.g., via a tap/defer inside the switchMap) and keep the finalize(()
=> this.loading.set(false)) on that inner observable so cancellation of a prior
request won't clear the loading state for the new one.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.scss`:
- Around line 5-7: Add a blank line before the scrollbar-width declaration to
satisfy stylelint's declaration-empty-line-before rule: edit
visa-request-application-dialog.component.scss and insert an empty line between
the `@apply` max-h-[440px]; line and the scrollbar-width: thin; declaration so the
scrollbar-width and scrollbar-color rules are preceded by the required empty
line.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.ts`:
- Around line 79-91: The next handler for submitVisaRequestApplication currently
treats any emission as success; update the subscribe next callback used with
eventsService.submitVisaRequestApplication(payload) to inspect the returned
VisaRequestApplicationResponse (e.g., const response) and check
response.success: if true, call messageService.add with a success toast and
ref.close({ submitted: true }); if false, call messageService.add with an
error/warning toast using response.message (or a default message) and ensure
submitting.set(false) is called so the form remains active; keep the error
handler as-is to set submitting.set(false) on exceptions.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.ts`:
- Around line 43-61: The component never emits the initial form validity/value,
so revisit of the dialog reuses stale parent signals; in the constructor (after
you patch the form with values from userService.user() and before or after
registering the statusChanges/valueChanges subscriptions) call
this.formValidityChange.emit(this.form.valid) and
this.formChange.emit(this.buildFormValue()) to emit the current validity and
form value immediately; reference the constructor, this.form,
this.formValidityChange, this.formChange, and buildFormValue to locate where to
add these two emits.
In `@apps/lfx-one/src/server/controllers/events.controller.ts`:
- Around line 315-323: Add a validation that requires payload.termsAccepted to
be true before calling this.eventsService.submitVisaRequestApplication: if
payload.termsAccepted is missing or false, throw
ServiceValidationError.forField('termsAccepted', 'termsAccepted must be true', {
operation: 'submit_visa_request_application' }); place this check after the
existing eventId/applicantInfo validations and before the call to
submitVisaRequestApplication so the service is never invoked when terms are not
accepted.
- Around line 343-352: Add controller-side validation for payload.expenses and
payload.termsAccepted before calling
this.eventsService.submitTravelFundApplication: if payload.expenses is missing
or payload.expenses.estimatedTotal is missing/invalid, throw
ServiceValidationError.forField('expenses', 'expenses.estimatedTotal is
required', { operation: 'submit_travel_fund_application' }); and if
payload.termsAccepted is not true, throw
ServiceValidationError.forField('termsAccepted', 'termsAccepted is required', {
operation: 'submit_travel_fund_application' }); keep the same error constructor
pattern used for eventId/aboutMe and then call
this.eventsService.submitTravelFundApplication and logger.success as before.
In `@apps/lfx-one/src/server/services/events.service.ts`:
- Around line 92-95: The registeredOnlyFilter currently uses "r.EVENT_ID IS NOT
NULL" which also matches cancelled/pending rows; change the condition in
events.service.ts (registeredOnlyFilter) to only match accepted registrations by
checking r.REGISTRATION_STATUS = 'Accepted' (or replace the IS NOT NULL check
with r.REGISTRATION_STATUS = 'Accepted') so queries only treat truly registered
users as registered; update any SQL parameter ordering/usage related to
registeredOnlyFilter if needed.
In `@packages/shared/src/interfaces/visa-request-application.interface.ts`:
- Around line 4-20: The shared contract currently types
VisaRequestApplicantInfo.passportExpiryDate as Date | null but the wire format
is a string; update VisaRequestApplicantInfo to use passportExpiryDate: string |
null and keep VisaRequestApplication unchanged, then ensure the form component
serializes/deserializes dates (e.g., in your form submission helper such as
buildFormValue) by converting a Date to ISO string (date instanceof Date ?
date.toISOString() : null) before sending and parsing ISO strings back to Date
when populating the form (follow the pattern used in
survey-manage.component.ts).
---
Nitpick comments:
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.html`:
- Around line 26-29: The airfare cost input (lfx-input-text with
control="airfareCost") currently allows negative numbers; add a min="0"
attribute to that lfx-input-text (and the equivalent hotel and ground transport
inputs: controls "hotelCost" and "groundTransportCost") to prevent negative
entry at the HTML level if lfx-input-text forwards attributes, and also add
form-level validation in the TravelExpensesFormComponent FormGroup by attaching
Validators.min(0) to the FormControl definitions for airfareCost, hotelCost, and
groundTransportCost to ensure negatives are rejected even if the input bypasses
the attribute.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.ts`:
- Around line 41-52: Emit the initial form value in the constructor to match the
immediate formValidityChange behavior: after the existing
this.formValidityChange.emit(true) call, call
this.formChange.emit(this.buildExpensesValue()) so the parent receives the
initial expenses (all zeros) without waiting for user interaction; keep existing
subscriptions for this.form.statusChanges and this.form.valueChanges unchanged
and reference the constructor, formChange, buildExpensesValue(), and
formValidityChange symbols.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-terms/travel-fund-terms.component.ts`:
- Line 12: Add a one-line JSDoc comment for the exported
TravelFundTermsComponent class explaining its purpose; locate the exported
symbol TravelFundTermsComponent and add a concise /** ... */ JSDoc directly
above the class declaration describing what the component represents or does
(e.g., "Component displaying terms for the travel fund on the My Events
dashboard"), following repo style for exported modules.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.html`:
- Around line 4-12: The data-testid binding on the lfx-button is using an
unnecessary property binding with a static string; in the travel-funding
template change the lfx-button attribute from
[attr.data-testid]="'travel-funding-discover-button'" to a plain
data-testid="travel-funding-discover-button" (leave the lfx-button element, its
label and the openApplicationDialog() click handler untouched).
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.html`:
- Around line 43-54: The dialog applies the scroll container only for the
'select-event' branch, causing inconsistent scrolling for other steps; wrap the
content of every step rendered by step() — including the lfx-event-selection,
lfx-visa-request-terms, and lfx-visa-request-apply-form branches — in the same
<div class="scroll-area overflow-y-auto pr-1"> wrapper so all steps get
consistent overflow-y-auto behavior and identical layout; update the template
around the step() checks to reuse that container for each branch while leaving
the existing bindings ([(selectedEvent)]="selectedEvent",
(formValidityChange)="applyFormValid.set($event)",
(formChange)="applicantData.set($event)") unchanged.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.html`:
- Around line 65-68: The lfx-calendar used for the passport expiry date is
missing a dataTest attribute; update the lfx-calendar element (the one with
control="passportExpiryDate" in the VisaRequestApplyForm template) to include a
descriptive dataTest value (e.g., "passportExpiryDate" or
"passport-expiry-date-input") so automated tests and selectors can target it
consistently.
- Around line 39-49: Add autocomplete="off" to the passport input element:
update the lfx-input-text tag that binds to control="passportNumber"
(id="visa-passport-number") so the browser won’t autofill this sensitive field;
if lfx-input-text does not pass attributes through, modify the lfx-input-text
component to accept and forward an autocomplete input/property to the native
input element (or expose a dedicated `@Input` like autocomplete for that
component) and ensure the form binding (form, control="passportNumber") remains
unchanged.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-terms/visa-request-terms.component.ts`:
- Line 12: Add a brief JSDoc block immediately above the exported class
VisaRequestTermsComponent describing the component's purpose and role (one-line
summary plus a short sentence), include any relevant params/props or usage notes
if applicable, and tag it as exported/public per repo standards (e.g., use a
leading /** ... */ with a description and optional `@export` or `@public` tag).
Ensure the JSDoc sits directly above the "export class
VisaRequestTermsComponent" declaration so the component is self-documented.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.html`:
- Around line 4-12: The template uses a bound attribute for a static test id;
change the lfx-button's [attr.data-testid] binding to a plain static attribute
so the test id is not bound at runtime—for the lfx-button element that calls
openApplicationDialog(), replace the
[attr.data-testid]="'visa-request-discover-button'" binding with a simple
data-testid="visa-request-discover-button" attribute to simplify the markup.
- Around line 30-53: The data-testid attributes for the table header sort
buttons (currently "sort-event-name", "sort-location", "sort-application-date")
are inconsistent with the travel-funding component; update them to a consistent,
unique prefix such as "visa-request-sort-event-name",
"visa-request-sort-location", and "visa-request-sort-application-date" in the
template where the <button> elements call
onHeaderClick('EVENT_NAME'/'EVENT_CITY'/'APPLICATION_DATE') and use sortField(),
sortOrder(), and sortIcons() so tests can reliably target these headers without
collisions.
In `@apps/lfx-one/src/server/pdf-templates/visa-letter-manual/template.html`:
- Around line 96-133: The Handlebars conditional for event.state contains
excessive internal whitespace; replace the spaced-out block with a compact
conditional like {{`#if` event.state}}{{ event.state }},{{/if}} and similarly
ensure the following {{`#if` event.country}}{{ event.country }}{{/if}} is
compacted so the template no longer contains large blank regions between the
{{`#if` event.state}} and its closing }} token; update the occurrences of the
'{{`#if` event.state}}...{{/if}}' and '{{`#if` event.country}}...{{/if}}' constructs
in template.html accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: c4c5c33f-0050-4707-96be-259076d25588
📒 Files selected for processing (46)
apps/lfx-one/src/app/modules/events/components/events-top-bar/events-top-bar.component.tsapps/lfx-one/src/app/modules/events/foundation-event-dashboard/components/events-table/events-table.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/events-list/events-list.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/events-list/events-list.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-expenses-form/travel-expenses-form.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-terms/travel-fund-terms.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-terms/travel-fund-terms.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-terms/travel-fund-terms.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-terms/visa-request-terms.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-terms/visa-request-terms.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-terms/visa-request-terms.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/my-events-dashboard.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/my-events-dashboard.component.tsapps/lfx-one/src/app/shared/services/events.service.tsapps/lfx-one/src/server/controllers/events.controller.tsapps/lfx-one/src/server/pdf-templates/visa-letter-manual/template.htmlapps/lfx-one/src/server/routes/events.route.tsapps/lfx-one/src/server/services/events.service.tspackages/shared/src/constants/events.constants.tspackages/shared/src/interfaces/index.tspackages/shared/src/interfaces/my-event.interface.tspackages/shared/src/interfaces/travel-fund-application.interface.tspackages/shared/src/interfaces/visa-request-application.interface.ts
...c/app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.ts
Show resolved
Hide resolved
...modules/events/my-events-dashboard/components/event-selection/event-selection.component.scss
Outdated
Show resolved
Hide resolved
...p/modules/events/my-events-dashboard/components/event-selection/event-selection.component.ts
Outdated
Show resolved
Hide resolved
...p/modules/events/my-events-dashboard/components/event-selection/event-selection.component.ts
Show resolved
Hide resolved
...oard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.scss
Outdated
Show resolved
Hide resolved
.../my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.ts
Show resolved
Hide resolved
MRashad26
reviewed
Apr 10, 2026
Contributor
MRashad26
left a comment
MRashad26
left a comment
There was a problem hiding this comment.
Code review: 12 issues found (2 critical, 4 major, 6 minor). See inline comments below.
...rd/components/visa-request-application-dialog/visa-request-application-dialog.component.html
Outdated
Show resolved
Hide resolved
...oard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.html
Outdated
Show resolved
Hide resolved
...modules/events/my-events-dashboard/components/event-selection/event-selection.component.html
Outdated
Show resolved
Hide resolved
...src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.ts
Show resolved
Hide resolved
.../my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.ts
Show resolved
Hide resolved
...y-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.html
Show resolved
Hide resolved
apps/lfx-one/src/server/pdf-templates/visa-letter-manual/template.html
Outdated
Show resolved
Hide resolved
Signed-off-by: emlimlf <52259294+emlimlf@users.noreply.github.com>
Address review comments from @MRashad26, copilot-pull-request-reviewer[bot], coderabbitai[bot]: - event-selection.component.ts: fix "Any Where" → "Anywhere" typo in filter options (per @MRashad26) - event-selection.component.ts: fix pagination — only advance currentOffset after successful response (per @MRashad26) - event-selection.component.html: make empty-state text generic "Register for an event to continue" (per coderabbitai[bot]) - event-selection.component.html: replace \$any() cast with template reference variable #inputEl (per coderabbitai[bot]) - event-selection.component.scss: add empty line before scrollbar-width, replace hardcoded hex with theme() (per coderabbitai[bot]) - visa-request-application-dialog.component.ts: track termsAccepted with signal, set on "I Agree" click (per @MRashad26) - visa-request-application-dialog.component.ts: check response.success before closing; show error toast if false (per @MRashad26) - visa-request-application-dialog.component.ts: replace isStepActive/isStepCompleted methods with stepStates computed signal (per coderabbitai[bot]) - visa-request-application-dialog.component.ts: move VisaRequestStep type to @lfx-one/shared/interfaces (per coderabbitai[bot]) - visa-request-application-dialog.component.scss: add empty line before scrollbar-width, replace hardcoded hex (per coderabbitai[bot]) - travel-fund-application-dialog.component.ts: same termsAccepted tracking and response.success check (per @MRashad26) - travel-fund-application-dialog.component.ts: same computed stepStates refactor (per coderabbitai[bot]) - travel-fund-application-dialog.component.scss: add empty line before scrollbar-width, replace hardcoded hex (per coderabbitai[bot]) - visa-request-apply-form.component.html: add autocomplete="off" to passport number input (per @MRashad26) - visa-request-apply-form.component.html: add validation error messages to all required fields (per @MRashad26) - about-me-form.component.html: add validation error messages to all required fields (per @MRashad26) - visa-request.component.ts: add ChangeDetectionStrategy.OnPush (per coderabbitai[bot]) - my-events-dashboard.component.ts: reset selectedSearchQuery on tab switch (per @MRashad26) - events.service.ts (frontend): remove redundant take(1) from submit methods (per copilot-pull-request-reviewer[bot]) - events.controller.ts: add auth identity check (extract + overwrite email from session) to both submit endpoints (per @MRashad26) - events.controller.ts: add server-side termsAccepted and expenses validation (per coderabbitai[bot]) - events.controller.ts: add logger.error() to getUpcomingCountries catch block (per coderabbitai[bot]) - events.controller.ts: add logger.error() to handleEventRequestsEndpoint catch block (per coderabbitai[bot]) - events.service.ts (server): fix registeredOnly filter to require REGISTRATION_STATUS = 'Accepted' (per @MRashad26) - template.html: collapse blank lines in PDF visa letter template (per @MRashad26) - my-event.interface.ts: move TravelFundStep and VisaRequestStep to shared package (per coderabbitai[bot]) - my-event.interface.ts: add eventId and projectName to GetEventRequestsParams (per @MRashad26) - my-event.interface.ts: update JSDoc to reflect actual Snowflake status values (per @MRashad26) Resolves 37 review threads. LFXV2-2196 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Contributor
Author
Review Feedback AddressedCommit: a3d62c3 Changes Madeevent-selection component
Visa Request Application Dialog
Travel Fund Application Dialog
Visa Request Apply Form
About Me Form
Visa Request component
My Events Dashboard
Frontend events.service.ts
Server events.controller.ts
Server events.service.ts
PDF template
Shared interfaces
No Change Needed
Threads Resolved37 of 37 unresolved threads addressed in this iteration. |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
apps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.ts (1)
95-103:⚠️ Potential issue | 🟠 Major
onLoadMore()still advances pagination on failed requests.Because Line 95 returns
EMPTY_MY_EVENTS_RESPONSE, the subscribe block still runs and Line 100 advancescurrentOffseteven on errors. That can skip pages and incorrectly affecthasMore()after transient failures.💡 Suggested fix
this.eventsService .getMyEvents({ isPast: false, pageSize: PAGE_SIZE, offset: nextOffset, registeredOnly: true, ...this.activeFilters() }) .pipe( - catchError(() => of(EMPTY_MY_EVENTS_RESPONSE)), + catchError(() => of(null)), finalize(() => this.loadingMore.set(false)), takeUntilDestroyed(this.destroyRef) ) .subscribe((response) => { + if (!response) return; this.currentOffset = nextOffset; this.allEvents.update((current) => [...current, ...response.data]); this.totalFromServer.set(response.total); });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.ts` around lines 95 - 103, The onLoadMore() flow advances currentOffset even when the request failed because catchError emits EMPTY_MY_EVENTS_RESPONSE and the subscribe block still runs; fix it so we only update pagination and append results when a real successful response is received. Concretely, change the error handling so the stream completes or emits a null/empty marker that you filter out before subscribe (or move the currentOffset/allEvents.update/totalFromServer.set logic behind a runtime check), referencing EMPTY_MY_EVENTS_RESPONSE, currentOffset, allEvents.update, totalFromServer.set and hasMore() to ensure pagination only advances on successful responses.
🧹 Nitpick comments (1)
apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.ts (1)
101-123: Consider showing an error toast on network failure.The
errorcallback only resetssubmittingwithout informing the user of the failure. While the dialog stays open allowing retry, users may not understand why their submission didn't complete.♻️ Suggested enhancement
error: () => { + this.messageService.add({ + severity: 'error', + summary: 'Error', + detail: 'Unable to submit your application. Please try again.', + }); this.submitting.set(false); },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.ts` around lines 101 - 123, The error branch of the submitTravelFundApplication subscription only resets submitting but doesn't notify the user; update the error callback for eventsService.submitTravelFundApplication (the subscribe block) to call messageService.add with severity 'error' and a helpful detail (use error.message if present, otherwise a generic message like "Network error: unable to submit your application"), and then call this.submitting.set(false) (keep existing behavior) so users see a toast when the network/request fails; reference the existing symbols eventsService.submitTravelFundApplication, messageService.add, this.submitting.set, and this.ref.close when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.html`:
- Around line 157-177: The two required selects (controls attendingForCompany
and willingToBlog in about-me-form.component.html) lack inline validation
feedback; update the markup for the lfx-select blocks for attendingForCompany
and willingToBlog to mirror the earlier required fields by adding an *ngIf that
checks form.get('attendingForCompany').touched &&
form.get('attendingForCompany').invalid (and similarly for willingToBlog) and
render the same styled error message/span used elsewhere (same CSS classes and
dataTest convention) so users see the "required" validation when the control is
touched and invalid.
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.html`:
- Around line 54-56: The submit is not prevented when the expenses form is
invalid because the template only listens to (formChange) and the submit button
is only disabled for in-flight requests; update the expenses form wiring so the
parent component tracks form validity (e.g., add an output like (validChange) or
have lfx-travel-expenses-form emit {value, valid}) and store that validity in a
reactive signal/variable (e.g., expensesValid) alongside expensesData.set; then
modify the submit button disable condition (the code around the submit handler
used when step() === 'expenses') to also check this validity (disable when
!expensesValid || submitting) so submission is gated by the expenses form's
validity.
---
Duplicate comments:
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.ts`:
- Around line 95-103: The onLoadMore() flow advances currentOffset even when the
request failed because catchError emits EMPTY_MY_EVENTS_RESPONSE and the
subscribe block still runs; fix it so we only update pagination and append
results when a real successful response is received. Concretely, change the
error handling so the stream completes or emits a null/empty marker that you
filter out before subscribe (or move the
currentOffset/allEvents.update/totalFromServer.set logic behind a runtime
check), referencing EMPTY_MY_EVENTS_RESPONSE, currentOffset, allEvents.update,
totalFromServer.set and hasMore() to ensure pagination only advances on
successful responses.
---
Nitpick comments:
In
`@apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.ts`:
- Around line 101-123: The error branch of the submitTravelFundApplication
subscription only resets submitting but doesn't notify the user; update the
error callback for eventsService.submitTravelFundApplication (the subscribe
block) to call messageService.add with severity 'error' and a helpful detail
(use error.message if present, otherwise a generic message like "Network error:
unable to submit your application"), and then call this.submitting.set(false)
(keep existing behavior) so users see a toast when the network/request fails;
reference the existing symbols eventsService.submitTravelFundApplication,
messageService.add, this.submitting.set, and this.ref.close when making the
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e42e99fb-3037-4960-9e4b-298a4dd1b2ba
📒 Files selected for processing (20)
apps/lfx-one/src/app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.scssapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-apply-form/visa-request-apply-form.component.htmlapps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.tsapps/lfx-one/src/app/modules/events/my-events-dashboard/my-events-dashboard.component.tsapps/lfx-one/src/app/modules/trainings/components/training-card/training-card.component.htmlapps/lfx-one/src/app/modules/trainings/trainings-dashboard/trainings-dashboard.component.htmlapps/lfx-one/src/app/shared/services/events.service.tsapps/lfx-one/src/server/controllers/events.controller.tsapps/lfx-one/src/server/pdf-templates/visa-letter-manual/template.htmlapps/lfx-one/src/server/services/events.service.tspackages/shared/src/interfaces/my-event.interface.ts
✅ Files skipped from review due to trivial changes (6)
- apps/lfx-one/src/app/modules/trainings/components/training-card/training-card.component.html
- apps/lfx-one/src/server/pdf-templates/visa-letter-manual/template.html
- apps/lfx-one/src/app/modules/trainings/trainings-dashboard/trainings-dashboard.component.html
- apps/lfx-one/src/app/modules/events/my-events-dashboard/components/event-selection/event-selection.component.scss
- apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request-application-dialog/visa-request-application-dialog.component.scss
- apps/lfx-one/src/app/modules/events/my-events-dashboard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.scss
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/lfx-one/src/app/modules/events/my-events-dashboard/components/visa-request/visa-request.component.ts
- apps/lfx-one/src/server/controllers/events.controller.ts
...app/modules/events/my-events-dashboard/components/about-me-form/about-me-form.component.html
Show resolved
Hide resolved
...oard/components/travel-fund-application-dialog/travel-fund-application-dialog.component.html
Show resolved
Hide resolved
🚀 Deployment StatusYour branch has been deployed to: https://ui-pr-415.dev.v2.cluster.linuxfound.info Deployment Details:
The deployment will be automatically removed when this PR is closed. |
MRashad26
reviewed
Apr 13, 2026
...app/modules/events/my-events-dashboard/components/travel-funding/travel-funding.component.ts
Show resolved
Hide resolved
MRashad26
reviewed
Apr 13, 2026
MRashad26
reviewed
Apr 13, 2026
MRashad26
reviewed
Apr 13, 2026
...s/events/my-events-dashboard/components/visa-request-terms/visa-request-terms.component.html
Show resolved
Hide resolved
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@apps/lfx-one/src/app/modules/events/events-dashboard/events-dashboard.component.ts`:
- Around line 13-17: The template currently treats any non-'me' lens as
Foundation; update the conditional so lfx-foundation-event-dashboard is only
rendered when the lens is explicitly Foundation by adding an explicit check
(e.g., introduce or use isFoundationLens()) and change the branches to: if
(isMeLens()) -> <lfx-my-events-dashboard />, else if (isFoundationLens()) ->
<lfx-foundation-event-dashboard />, else -> handle project/org appropriately
(render the correct component or nothing). Update or add the computed helper
(isFoundationLens) alongside isMeLens to detect the 'foundation' lens value used
by the component.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 23ff2154-803d-48e1-824e-fb9013dd30e3
📒 Files selected for processing (4)
apps/lfx-one/src/app/app.routes.tsapps/lfx-one/src/app/layouts/main-layout/main-layout.component.tsapps/lfx-one/src/app/modules/events/events-dashboard/events-dashboard.component.tsapps/lfx-one/src/app/modules/events/events.routes.ts
✅ Files skipped from review due to trivial changes (1)
- apps/lfx-one/src/app/layouts/main-layout/main-layout.component.ts
apps/lfx-one/src/app/modules/events/events-dashboard/events-dashboard.component.ts
Outdated
Show resolved
Hide resolved
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
Contributor
Author
Review Feedback AddressedCommit: ee67e08 Changes Made
Threads Resolved3 of 3 unresolved threads addressed in this iteration. |
Signed-off-by: emlimlf <52259294+emlimlf@users.noreply.github.com>
Signed-off-by: Efren Lim <elim@linuxfoundation.org>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Note: Both submission endpoints are stubs pending upstream microservice availability. The
TODOcomments inevents.service.tsmark the integration points.Summary
and a stub submission endpoint (
POST /api/events/visa-applications)POST /api/events/travel-fund-applications)registeredOnlyfilter, replacing the previousregisteredFirstsort) — shows a "register for an event" prompt when the list isempty