fix(crowdfunding): expose type-specific fields for event and security audit initiatives (DO NOT MERGE)#939
Open
mlehotskylf wants to merge 6 commits into
Open
fix(crowdfunding): expose type-specific fields for event and security audit initiatives (DO NOT MERGE)#939mlehotskylf wants to merge 6 commits into
mlehotskylf wants to merge 6 commits into
Conversation
… audit initiatives Event dates (event_start_date / event_end_date) and security audit details (ostif_detail / contacts) were returned by the CF backend but not surfaced through the Self Serve BFF or edit UI, making it impossible for owners to view or update these fields after creation. - Add BackendOSTIFDetail, BackendContact, BackendContactInput types to the server wire shape (crowdfunding.types.ts) - Extend BackendInitiative with ostif_detail, contacts, entity_details, coc_url, accept_funding, is_online, and eventbrite_url - Extend BackendUpdateInitiativeInput with event_start_date, event_end_date, ostif_detail, and contacts - Add OSTIFDetail, InitiativeContact, UpdateOSTIFDetailInput, and UpdateContactInput to the shared interfaces - Extend InitiativeDetail with all type-specific read fields - Extend UpdateInitiativeInput with eventStartDate, eventEndDate, ostifDetail, and contacts - Map new fields in mapToInitiativeDetail (crowdfunding-mapper.ts) - Forward new fields in updateInitiative() in the server service and controller - Add event date pickers (shown only for event type) and security audit detail fields (shown only for security_audit type) to the settings drawer Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Contributor
|
Warning Review limit reached
More reviews will be available in 30 minutes and 37 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
WalkthroughAdds initiative-type-specific fields and contact management across the full crowdfunding stack. New ChangesInitiative-type-specific fields (Event, Security Audit, Contacts)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the crowdfunding initiative detail/update data model end-to-end (shared interfaces → BFF wire types/mappers/controllers/services → settings drawer UI) so type-specific fields (event dates and security-audit/OSTIF details) are visible and editable after initiative creation.
Changes:
- Added shared and server-side wire interfaces/fields for event initiative fields, OSTIF detail, and contacts.
- Updated the BFF mapper and update flow to map/forward the new fields to/from the upstream CF API.
- Added conditional UI sections in the initiative settings drawer for event dates and security-audit fields.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/interfaces/crowdfunding.interface.ts | Adds shared types and extends initiative detail/update inputs to include event dates, OSTIF detail, and contacts. |
| apps/lfx-one/src/server/utils/crowdfunding-mapper.ts | Maps new backend snake_case fields (OSTIF detail, contacts, and other entity fields) into shared camelCase shapes. |
| apps/lfx-one/src/server/types/crowdfunding.types.ts | Extends upstream CF wire types and update payload types with OSTIF detail and contacts. |
| apps/lfx-one/src/server/services/crowdfunding.service.ts | Forwards event dates, OSTIF detail, and contacts in the PATCH payload to the upstream CF API. |
| apps/lfx-one/src/server/controllers/crowdfunding.controller.ts | Parses/validates new fields from the client PATCH body into UpdateInitiativeInput. |
| apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts | Adds form controls, prepopulation, and payload wiring for event dates + OSTIF fields. |
| apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.html | Adds conditional UI blocks for event date inputs and security audit details. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts (2)
177-215:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftThe new
contactscontract never reaches this drawer.The shared/server layers now accept
contacts, but this component never patchesinitiative().contactsinto local state and never includesinput.contactson save. As shipped, security-audit initiatives still have no way to view or edit contact information from this drawer, so that part of the feature remains incomplete.
apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts#L177-L215: addcontactsto the save payload instead of only sending event/OSTIF fields.apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts#L133-L146: patch existinginitiative().contactsinto form state when the drawer opens.apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.html#L128-L181: render contact inputs in the security-audit section so the new contract is actually editable.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts` around lines 177 - 215, The contacts field from the initiative is not being integrated into the settings drawer, making it impossible to view or edit contact information for security-audit initiatives. Fix this across three locations: In initiative-settings-drawer.component.ts at lines 133-146 (the initialization section), patch the existing initiative().contacts into form state when the drawer opens by reading the contacts from the initiative signal and populating the form control. At lines 177-215 (the save payload section), extract contacts from this.form.value and add input.contacts to the UpdateInitiativeInput object being sent to the server. In initiative-settings-drawer.component.html at lines 128-181 (the security-audit template section), add form inputs that render and allow editing of the contact information fields for security-audit initiatives, binding them to the form controls established in the TypeScript logic.
69-83:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPrevent inverted event date ranges before saving.
The controller/service path just forwards
eventStartDateandeventEndDate, so this drawer currently allows an event whose end date is earlier than its start date. That creates invalid initiative data and only surfaces later in downstream views.
apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts#L69-L83: add a cross-field validator for the event date controls.apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts#L202-L205: block save and surface an error wheneventEndDate < eventStartDate.apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.html#L116-L123: bind the end-date input’sminto the selected start date so the browser UI prevents obviously invalid picks.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts` around lines 69 - 83, Prevent inverted event date ranges by implementing validation at three locations: In the form definition at lines 69-83 of initiative-settings-drawer.component.ts, add a cross-field validator to the FormGroup that checks if eventEndDate is earlier than eventStartDate and returns an error if so. At lines 202-205 in the same file (the save method), check for this validation error and block the save operation by displaying an error message to the user if the validator detects that eventEndDate is less than eventStartDate. In the initiative-settings-drawer.component.html template at lines 116-123, bind the min attribute of the eventEndDate input element to the eventStartDate control's value so the browser UI prevents obviously invalid date selections and provides immediate visual feedback.
🧹 Nitpick comments (3)
apps/lfx-one/src/server/controllers/crowdfunding.controller.ts (1)
284-285: ⚡ Quick winConsider validating date format for
eventStartDateandeventEndDate.The controller accepts any string as event dates without format validation. While the crowdfunding backend may validate, adding basic ISO 8601 format checks here would catch errors earlier and provide better error messages.
const isValidISODate = (s: string): boolean => /^\d{4}-\d{2}-\d{2}(T\d{2}:\d{2}:\d{2}(\.\d{3})?Z?)?$/.test(s); if (typeof body['eventStartDate'] === 'string') { if (body['eventStartDate'] && !isValidISODate(body['eventStartDate'])) { throw ServiceValidationError.forField('eventStartDate', 'eventStartDate must be in ISO 8601 format', { operation: 'update_initiative' }); } input.eventStartDate = body['eventStartDate']; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/server/controllers/crowdfunding.controller.ts` around lines 284 - 285, The eventStartDate and eventEndDate fields are accepted as any string without format validation, which can lead to invalid data being processed. Add a helper function to validate ISO 8601 date format using the provided regex pattern, then modify the type checks for eventStartDate and eventEndDate to validate the format before assignment. If the format is invalid, throw a ServiceValidationError with appropriate error messaging indicating the expected ISO 8601 format. Apply this validation to both date fields in the same way shown in the comment example.apps/lfx-one/src/server/utils/crowdfunding-mapper.ts (1)
84-84: 💤 Low valueConsider returning
undefinedinstead of empty array whencontactsis absent.The mapper uses nullish coalescing to default to an empty array:
contacts: (b.contacts ?? []).map(mapContact),This means if the backend doesn't provide
contacts, the result iscontacts: [](empty array) instead ofcontacts: undefined. While functionally similar, it's semantically different: "no contacts field" vs "zero contacts".For consistency with other optional fields, consider:
-contacts: (b.contacts ?? []).map(mapContact), +contacts: b.contacts ? b.contacts.map(mapContact) : undefined,This preserves the backend's absence signal and aligns with the optional field semantics in
InitiativeDetail.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/server/utils/crowdfunding-mapper.ts` at line 84, The contacts field mapping uses nullish coalescing to default to an empty array when contacts are absent, which converts undefined to an empty array and loses the semantic distinction between "no contacts field" and "zero contacts". Replace the nullish coalescing pattern in the contacts mapping with optional chaining so that when b.contacts is undefined, the result preserves undefined instead of defaulting to an empty array, making it consistent with other optional fields in InitiativeDetail.packages/shared/src/interfaces/crowdfunding.interface.ts (1)
302-310: ⚡ Quick winVerify
contactTypehandling in controller.
contactTypeis required (non-optional) inUpdateContactInput, but the controller parsing incrowdfunding.controller.tsLine 300 defaults it to an empty string when the field is missing or invalid:contactType: typeof c['contactType'] === 'string' ? c['contactType'] : '',This allows invalid contacts with empty
contactTypeto pass through. Consider either:
- Making
contactTypeoptional in this interface, or- Adding validation in the controller to reject contacts with missing/empty
contactType🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shared/src/interfaces/crowdfunding.interface.ts` around lines 302 - 310, Resolve the inconsistency between the UpdateContactInput interface definition where contactType is marked as required and the controller's parsing logic that defaults it to an empty string when invalid. Choose one approach: either make contactType optional by adding a question mark in the UpdateContactInput interface definition, or add validation in the controller to explicitly reject or handle cases where contactType is missing or empty, ensuring that the interface contract and controller implementation align.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts`:
- Around line 207-214: The totalBudgetCents value is stored as cents in the
backend but the UI displays and accepts it as dollars, creating a 100× mismatch.
At line 139-145 in initiative-settings-drawer.component.ts, divide the
totalBudgetCents value by 100 before patching it into the form so users see
dollar amounts. At line 207-214 (the current anchor location), multiply the
user-entered dollar amount by 100 before assigning it to
input.ostifDetail.totalBudgetCents so the backend receives cents. At line
160-170 in initiative-settings-drawer.component.html, verify the input label and
help text accurately describe the unit as USD (dollars) to match the UI behavior
after these conversions.
In `@apps/lfx-one/src/server/types/crowdfunding.types.ts`:
- Around line 66-69: There is a type mismatch where accept_funding and is_online
are declared as required fields in the BackendInitiative interface, but they are
mapped to optional fields (acceptFunding? and isOnline?) in the InitiativeDetail
interface. Determine whether the crowdfunding backend always provides these two
fields. If it does, update the InitiativeDetail interface to make acceptFunding
and isOnline required by removing the optional operator. If the backend does not
guarantee these fields, update the BackendInitiative interface to make
accept_funding and is_online optional instead. Ensure the mapper correctly
handles the chosen approach without defensive checks if fields are required.
In `@packages/shared/src/interfaces/crowdfunding.interface.ts`:
- Around line 140-146: The OSTIFDetail interface declares totalBudgetCents and
termsConditions as required fields, but the UI code defensively handles them as
potentially missing using nullish coalescing operators. Make these two
properties optional in the OSTIFDetail interface by adding a question mark after
each property name (totalBudgetCents? and termsConditions?) to align the type
definition with the actual runtime behavior where the backend may not always
provide these fields.
---
Outside diff comments:
In
`@apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts`:
- Around line 177-215: The contacts field from the initiative is not being
integrated into the settings drawer, making it impossible to view or edit
contact information for security-audit initiatives. Fix this across three
locations: In initiative-settings-drawer.component.ts at lines 133-146 (the
initialization section), patch the existing initiative().contacts into form
state when the drawer opens by reading the contacts from the initiative signal
and populating the form control. At lines 177-215 (the save payload section),
extract contacts from this.form.value and add input.contacts to the
UpdateInitiativeInput object being sent to the server. In
initiative-settings-drawer.component.html at lines 128-181 (the security-audit
template section), add form inputs that render and allow editing of the contact
information fields for security-audit initiatives, binding them to the form
controls established in the TypeScript logic.
- Around line 69-83: Prevent inverted event date ranges by implementing
validation at three locations: In the form definition at lines 69-83 of
initiative-settings-drawer.component.ts, add a cross-field validator to the
FormGroup that checks if eventEndDate is earlier than eventStartDate and returns
an error if so. At lines 202-205 in the same file (the save method), check for
this validation error and block the save operation by displaying an error
message to the user if the validator detects that eventEndDate is less than
eventStartDate. In the initiative-settings-drawer.component.html template at
lines 116-123, bind the min attribute of the eventEndDate input element to the
eventStartDate control's value so the browser UI prevents obviously invalid date
selections and provides immediate visual feedback.
---
Nitpick comments:
In `@apps/lfx-one/src/server/controllers/crowdfunding.controller.ts`:
- Around line 284-285: The eventStartDate and eventEndDate fields are accepted
as any string without format validation, which can lead to invalid data being
processed. Add a helper function to validate ISO 8601 date format using the
provided regex pattern, then modify the type checks for eventStartDate and
eventEndDate to validate the format before assignment. If the format is invalid,
throw a ServiceValidationError with appropriate error messaging indicating the
expected ISO 8601 format. Apply this validation to both date fields in the same
way shown in the comment example.
In `@apps/lfx-one/src/server/utils/crowdfunding-mapper.ts`:
- Line 84: The contacts field mapping uses nullish coalescing to default to an
empty array when contacts are absent, which converts undefined to an empty array
and loses the semantic distinction between "no contacts field" and "zero
contacts". Replace the nullish coalescing pattern in the contacts mapping with
optional chaining so that when b.contacts is undefined, the result preserves
undefined instead of defaulting to an empty array, making it consistent with
other optional fields in InitiativeDetail.
In `@packages/shared/src/interfaces/crowdfunding.interface.ts`:
- Around line 302-310: Resolve the inconsistency between the UpdateContactInput
interface definition where contactType is marked as required and the
controller's parsing logic that defaults it to an empty string when invalid.
Choose one approach: either make contactType optional by adding a question mark
in the UpdateContactInput interface definition, or add validation in the
controller to explicitly reject or handle cases where contactType is missing or
empty, ensuring that the interface contract and controller implementation align.
🪄 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: b33451c6-b136-4c73-8672-9c5f11b4452b
📒 Files selected for processing (7)
apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.htmlapps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.tsapps/lfx-one/src/server/controllers/crowdfunding.controller.tsapps/lfx-one/src/server/services/crowdfunding.service.tsapps/lfx-one/src/server/types/crowdfunding.types.tsapps/lfx-one/src/server/utils/crowdfunding-mapper.tspackages/shared/src/interfaces/crowdfunding.interface.ts
…tings drawer Follow-up to the initial type-specific fields fix. After auditing the CF initiative creation flow, several fields were found missing from the settings drawer and the BFF layer for each initiative type. All types: - Add Code of Conduct URL (coc_url) field Event type (expanded from just dates): - Add Registration URL (application_url) - Add Eventbrite URL (eventbrite_url) - Add City and Country location fields - Add Online event toggle (is_online) Project type: - Add CII Project ID (cii_project_id) field Security audit type: - Add contact management (primary, secondary, technical lead) with first/last name, email, phone, preferred contact method - Each contact type can only be added once (enforced client-side) Backend (BFF): - Extend BackendUpdateInitiativeInput and UpdateInitiativeInput with all new fields (cocUrl, ciiProjectId, applicationUrl, eventbriteUrl, country, city, isOnline, acceptFunding) - Add cii_project_id to BackendInitiative wire type - Map ciiProjectId in mapToInitiativeBase - Forward all new fields through controller and service to CF API - Fix date format: convert YYYY-MM-DD from date picker to RFC3339 (T00:00:00Z suffix) required by Go's time.Time JSON decoder Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/lfx-one/src/server/types/crowdfunding.types.ts (1)
43-44:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winRequired
contact_typemay receive empty string from controller.
BackendContactInputcorrectly requirescontact_type(line 44), but the controller (context snippet 2, line ~328) defaults missingcontactTypeto an empty string:contactType: typeof c['contactType'] === 'string' ? c['contactType'] : ''This allows invalid contacts with empty type strings to reach the backend. The controller should either throw a validation error when
contactTypeis missing or the backend should reject empty contact types.🛡️ Proposed fix in the controller
In
apps/lfx-one/src/server/controllers/crowdfunding.controller.tsaround line 328, validate thatcontactTypeis present:input.contacts = (body['contacts'] as Record<string, unknown>[]).map((c) => ({ - contactType: typeof c['contactType'] === 'string' ? c['contactType'] : '', + contactType: typeof c['contactType'] === 'string' && c['contactType'].trim() ? c['contactType'].trim() : (() => { throw ServiceValidationError.forField('contacts.contactType', 'Contact type is required'); })(),🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/server/types/crowdfunding.types.ts` around lines 43 - 44, The controller validation logic for contactType is allowing empty strings to reach the backend, which violates the BackendContactInput type requirement. In the crowdfunding.controller.ts file around line 328, instead of defaulting missing contactType to an empty string, add validation to check if contactType is present and is a non-empty string. When contactType is missing or empty, throw a validation error rather than passing an empty string through to the backend BackendContactInput.
🧹 Nitpick comments (2)
apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.html (1)
234-236: 💤 Low valueConsider a helper method for contact type label lookup.
The nested
@forloop to find a matching label is O(n) on every render. With only 3 contact types this has negligible performance impact, but a simple lookup method would be cleaner:protected getContactTypeLabel(type: string): string { return this.CONTACT_TYPES.find(ct => ct.value === type)?.label ?? type; }Then in the template:
<span class="text-sm font-medium text-gray-700">{{ getContactTypeLabel(group.value['contactType']) }}</span>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.html` around lines 234 - 236, Replace the nested `@for` loop in the template that searches through CONTACT_TYPES to find a matching label with a call to a helper method. Create a protected method named getContactTypeLabel in the component class that accepts a contact type string parameter, uses the find method on the CONTACT_TYPES array to locate the matching contact type object, and returns its label property with a fallback to the type string if no match is found. Update the template to call getContactTypeLabel(group.value['contactType']) instead of the `@for` loop iteration.apps/lfx-one/src/server/types/crowdfunding.types.ts (1)
192-204: 💤 Low valueLGTM! Consider discriminated unions for type-specific fields in the future.
The inline comments (lines 195, 197, 205) clearly document which fields apply to which initiative types. All fields are appropriately optional for a PATCH input.
For stronger compile-time safety, the input could eventually use TypeScript discriminated unions to enforce that event-only fields are only present in event updates, project-only fields only in project updates, etc. However, that refactor is outside the scope of this PR, and runtime validation likely happens in the backend service.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/lfx-one/src/server/types/crowdfunding.types.ts` around lines 192 - 204, This comment is a positive review approval (LGTM) with a future enhancement suggestion. No changes are required for the current PR. The inline comments in the CrowdfundingPatchInput type already appropriately document which fields apply to projects versus events (cii_project_id for project-only, and event_start_date, event_end_date, application_url, eventbrite_url, country, city, and is_online for event-only). The reviewer is suggesting that in a future PR you could consider refactoring to use TypeScript discriminated unions to enforce type safety at compile time, ensuring event-only fields only appear in event updates and project-only fields only in project updates, but acknowledges this is a future improvement outside the current scope since runtime validation likely handles this.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.ts`:
- Line 131: The isProjectType computed property is comparing to the string
'project' cast as FundType, but this value does not exist in the FundType enum
(which only contains GENERAL_FUND, SECURITY_AUDIT, MENTORSHIP, and EVENT). Fix
this by either replacing the 'project' string with the correct existing enum
value from FundType (likely EVENT based on the context), or if 'project' is a
valid type, add it as a new enum value to the FundType enum definition in the
crowdfunding.enum.ts file. Ensure the comparison in isProjectType uses a valid
enum value that matches the intended business logic.
---
Outside diff comments:
In `@apps/lfx-one/src/server/types/crowdfunding.types.ts`:
- Around line 43-44: The controller validation logic for contactType is allowing
empty strings to reach the backend, which violates the BackendContactInput type
requirement. In the crowdfunding.controller.ts file around line 328, instead of
defaulting missing contactType to an empty string, add validation to check if
contactType is present and is a non-empty string. When contactType is missing or
empty, throw a validation error rather than passing an empty string through to
the backend BackendContactInput.
---
Nitpick comments:
In
`@apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.html`:
- Around line 234-236: Replace the nested `@for` loop in the template that
searches through CONTACT_TYPES to find a matching label with a call to a helper
method. Create a protected method named getContactTypeLabel in the component
class that accepts a contact type string parameter, uses the find method on the
CONTACT_TYPES array to locate the matching contact type object, and returns its
label property with a fallback to the type string if no match is found. Update
the template to call getContactTypeLabel(group.value['contactType']) instead of
the `@for` loop iteration.
In `@apps/lfx-one/src/server/types/crowdfunding.types.ts`:
- Around line 192-204: This comment is a positive review approval (LGTM) with a
future enhancement suggestion. No changes are required for the current PR. The
inline comments in the CrowdfundingPatchInput type already appropriately
document which fields apply to projects versus events (cii_project_id for
project-only, and event_start_date, event_end_date, application_url,
eventbrite_url, country, city, and is_online for event-only). The reviewer is
suggesting that in a future PR you could consider refactoring to use TypeScript
discriminated unions to enforce type safety at compile time, ensuring event-only
fields only appear in event updates and project-only fields only in project
updates, but acknowledges this is a future improvement outside the current scope
since runtime validation likely handles this.
🪄 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: 167eabb6-27a3-40d1-99cc-a1b3f3c1ee2c
📒 Files selected for processing (7)
apps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.htmlapps/lfx-one/src/app/modules/crowdfunding/initiative-detail/components/initiative-settings-drawer/initiative-settings-drawer.component.tsapps/lfx-one/src/server/controllers/crowdfunding.controller.tsapps/lfx-one/src/server/services/crowdfunding.service.tsapps/lfx-one/src/server/types/crowdfunding.types.tsapps/lfx-one/src/server/utils/crowdfunding-mapper.tspackages/shared/src/interfaces/crowdfunding.interface.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/lfx-one/src/server/services/crowdfunding.service.ts
- apps/lfx-one/src/server/utils/crowdfunding-mapper.ts
- apps/lfx-one/src/server/controllers/crowdfunding.controller.ts
- packages/shared/src/interfaces/crowdfunding.interface.ts
…ettings drawer Terms acceptance is a one-time attestation made at initiative creation and should not be editable after submission. Exposing it as a toggle in the settings drawer could allow owners to retract their acceptance, corrupting the compliance record. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
mlehotskylf
added
the
do-not-merge
mlehotskylf
changed the title
…pe comparison Address review comments from coderabbitai and copilot-pull-request-reviewer: - initiative-settings-drawer.component.ts: divide totalBudgetCents by 100 when patching form (display dollars, not cents) (per coderabbitai, copilot-pull-request-reviewer) - initiative-settings-drawer.component.ts: multiply by 100 before saving totalBudgetCents so backend receives cents not dollars (per coderabbitai, copilot-pull-request-reviewer) - initiative-settings-drawer.component.ts: fix isProjectType computed — replace non-existent 'project' FundType cast with FundType.GENERAL_FUND so the CII Project ID field is shown and saved for general fund initiatives (per coderabbitai, copilot-pull-request-reviewer) - crowdfunding.interface.ts: make OSTIFDetail.totalBudgetCents and termsConditions optional to match UI defensive handling and UpdateOSTIFDetailInput (per coderabbitai) Resolves 7 review threads. Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Contributor
Author
Review Feedback AddressedCommit: 9406c64 Changes Made
Declined
Threads Resolved13 of 13 unresolved threads addressed in this iteration. |
…-drawer component Fix pre-existing formatting failures in Quality Checks CI. Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
…ngs drawer Address review comments from copilot-pull-request-reviewer: - initiative-settings-drawer.component.ts: always set input.contacts for security audit initiatives (remove contactGroups.length guard) so clearing all contacts sends an empty array and the upstream service can clear the list - crowdfunding.controller.ts: normalize eventStartDate/eventEndDate with .trim() || undefined, consistent with all other string fields in the block, to avoid forwarding empty strings to toRFC3339Date() and the upstream API Resolves 2 review threads. Signed-off-by: Michal Lehotsky <mlehotsky@linuxfoundation.org>
Contributor
Author
Review Feedback Addressed (iteration 2)Commit: 50b1d88d5e45c9e7e7cfe8b6b20ec5bbc059c1e1 Changes Made
Threads Resolved2 of 2 new unresolved threads addressed in this iteration. |
Contributor
Author
|
|
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.
Summary
Changes
Server-side wire types (
apps/lfx-one/src/server/types/crowdfunding.types.ts):BackendOSTIFDetail,BackendContact,BackendContactInputinterfacesBackendInitiativewithostif_detail,contacts,entity_details,coc_url,accept_funding,is_online,eventbrite_urlBackendUpdateInitiativeInputwithevent_start_date,event_end_date,ostif_detail,contactsShared interfaces (
packages/shared/src/interfaces/crowdfunding.interface.ts):OSTIFDetail,InitiativeContact,UpdateOSTIFDetailInput,UpdateContactInputInitiativeDetailandUpdateInitiativeInputwith all new fieldsMapper (
apps/lfx-one/src/server/utils/crowdfunding-mapper.ts):mapToInitiativeDetailnow maps OSTIF detail, contacts, and remaining entity fieldsServer service + controller:
updateInitiative()forwards event dates and OSTIF detail to the CF backendSettings drawer UI:
eventtype initiativessecurity_audittypeNo backend changes needed — the CF API already returns and accepts all these fields.
Test plan
yarn ng build --configuration development🤖 Generated with Claude Code