Skip to content

Opportunity type change#787

Open
kiborrdis wants to merge 6 commits into
need4deed-org:developfrom
kiborrdis:opportunity-type-change
Open

Opportunity type change#787
kiborrdis wants to merge 6 commits into
need4deed-org:developfrom
kiborrdis:opportunity-type-change

Conversation

@kiborrdis

Copy link
Copy Markdown
Collaborator

Description

Adds type switching dialog.
If switching to accompanying type, show accompanying details and forces to fill
If switching to event type, shows event date/time field and forces to fill

Related Issues

Closes #753

Changes

  • OpportunityEventDateTimeEdit — extracted from OpportunityDetailsEdit (event date/time using useFormContext; availability grid stays inline)
  • Added change type dialog shown for admin/coordinator

Screenshots

image image image

When moving away from accompanying:

image

Checklist

  • WITHIN THE SCOPE OF AN ISSUE; No unnecessary files included
  • Tests added/updated
  • Documentation updated
  • CI passes

@nadavosa nadavosa left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we with new boxes and not just the editable sections that appear?

Claude

What it does: Adds a full "Change type" dialog for opportunities — simple type switches for Regular, plus dedicated sub-forms when switching into Accompanying (collects accompanying details inline, required) or Events (collects event date/time, required). Compared to #755, this correctly accounts for the REGULAR_ACCOMPANYING compound status in the SDK's ProfileVolunteeringType enum — #755's simpler OpportunityType enum use would leave that case with nothing pre-selected.

🔴 Likely blocking — Events transition silently drops data:
useUpdateOpportunityType.ts sends event date/time via an ad-hoc eventDetails: { eventDate, eventTime } field, with a comment: "It's not yet implemented in api, for now keep it like that, change later." I checked the SDK's ApiOpportunityPatch type directly — there is no eventDetails (or equivalent) field, only schedule for recurring availability. So switching an opportunity to Events via this dialog will likely PATCH a field the backend doesn't recognize — the type flips to Events, a success toast fires, but the event date/time the coordinator just filled in (and which the form requires) probably isn't persisted. This isn't disclosed in the PR description and bypasses the SDK-contract-first convention this project follows elsewhere. I'd treat this as blocking until there's a corresponding sdk/be change, or the Events path is gated off until then.

🟡 Correctness bug (medium confidence) — REGULAR_ACCOMPANYING → ACCOMPANYING treated as no-op:

const toDialogInitial = (type) => type === REGULAR_ACCOMPANYING ? ACCOMPANYING : type;
const isNoop = selected === toDialogInitial(currentType);

If an opportunity's current type is REGULAR_ACCOMPANYING and a coordinator explicitly selects "Accompanying" (intending to drop the "regular" half), this collapses to ACCOMPANYING, matching selected, so isNoop = true. Save then just calls onCancel() and never sends the PATCH — the dialog closes as if it succeeded, but the type never actually changes. Worth confirming whether that transition is expected to be reachable; if so, this needs a fix.

Minor: AccompanyingTypeChangeForm.tsx's useEffect(() => methods.reset(getInitialFormValues(undefined)), [methods]) right after useForm is initialized with the exact same defaultValues is redundant, though harmless.

No tests, but that's consistent with the rest of the codebase's convention for this kind of feature.

@kiborrdis

kiborrdis commented Jul 13, 2026

Copy link
Copy Markdown
Collaborator Author

What do you mean by boxes and what do you mean by editable sections that appear?

Issue 1:
We discussed this in the slack, We need to finalize the decision on how do you want to approach eventTime/eventDate.
Separate field in the opportunity table, accompanying details, or store in the schedule. After that this point can be resolved.

Issue 2:
Probably should be able to change to accompanying in that case. Removed toDialogInitial

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: allow changing opportunity type in dashboard profile (accompanying / regular / event)

4 participants