-
Notifications
You must be signed in to change notification settings - Fork 24
885: fix, adds autocomplete operator edit in AgentOrganisationDetails #912
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
4ba6b37
77fe7e4
41bb5b3
a4f9e4d
ff2442b
545cf82
b75dbfe
e072dcd
34d87e1
546c861
c233c0f
de765c2
0d93c00
5f6c94b
15cb761
847825c
e18b9ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -47,5 +47,8 @@ dev/ | |
| dev.* | ||
| .env.local | ||
|
|
||
| # claude code local worktree scratch | ||
| .claude/worktrees/ | ||
|
|
||
| # prettier | ||
| .prettierignore | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -18,6 +18,7 @@ import { apiLanguagesToFormValues, toLanguagesForForm } from "./formatters"; | |
| import { OrganisationDetailsDisplay } from "./OrganisationDetailsDisplay"; | ||
| import { OrganisationDetailsEdit } from "./OrganisationDetailsEdit"; | ||
| import { createOrganisationDetailsSchema, OrganisationDetailsFormData } from "./organisationDetailsSchema"; | ||
| import { useGetOrganization } from "@/hooks/useGetOrganization"; | ||
|
|
||
| type Props = { | ||
| agent: ApiAgentProfileGet; | ||
|
|
@@ -35,8 +36,10 @@ export const OrganisationDetails = forwardRef<EditableSectionRef, Props>(functio | |
| const { data: apiLanguages } = useApiLanguages(); | ||
| const { data: apiAgentTypes = [] } = useApiAgentTypes(); | ||
| const { data: apiServices = [] } = useApiServices(); | ||
| const { data: organizations = [] } = useGetOrganization(); | ||
| const agentTypeMapping = useMemo(() => createMapping(apiAgentTypes), [apiAgentTypes]); | ||
| const serviceMapping = useMemo(() => createMapping(apiServices), [apiServices]); | ||
| const organizationMapping = useMemo(() => createMapping(organizations), [organizations]); | ||
|
|
||
| const details = agent.agentDetails; | ||
| const languagesForForm = toLanguagesForForm(apiLanguages, i18n.language); | ||
|
|
@@ -96,6 +99,7 @@ export const OrganisationDetails = forwardRef<EditableSectionRef, Props>(functio | |
| const serviceIds = values.services | ||
| .map((title) => serviceMapping.titleToId[title]) | ||
| .filter((id): id is number => id !== undefined); | ||
| const organizationId = organizationMapping.titleToId[values.operator]; | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CONFIRMED] Operator save silently no-ops when the typed text doesn't exactly match an organization title. If the user types an operator name but doesn't click a dropdown row (case/whitespace differs, or they ignore the list and hit Save), |
||
|
|
||
| updateOrganization( | ||
| { | ||
|
|
@@ -106,6 +110,7 @@ export const OrganisationDetails = forwardRef<EditableSectionRef, Props>(functio | |
| addressPostcode: values.addressPostcode, | ||
| ...(typeId !== undefined && { typeId }), | ||
| serviceIds, | ||
| ...(organizationId !== undefined && { organizationId }), | ||
| }, | ||
| { | ||
| onSuccess: () => { | ||
|
|
@@ -124,6 +129,7 @@ export const OrganisationDetails = forwardRef<EditableSectionRef, Props>(functio | |
| languagesForForm={languagesForForm} | ||
| organizationTypeOptions={apiAgentTypes.map((agentType) => agentType.title)} | ||
| servicesOptions={apiServices.map((service) => service.title)} | ||
| operatorOptions={organizations?.map((org) => org.title)} | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Redundant optional chaining: |
||
| onCancel={handleCancel} | ||
| onSubmit={handleSubmit(onSubmit)} | ||
| /> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,13 +7,15 @@ import { Controller, useFormContext } from "react-hook-form"; | |
| import { useTranslation } from "react-i18next"; | ||
| import { FormButtonRow, FormDetails } from "../shared/styles"; | ||
| import { OrganisationDetailsFormData } from "./organisationDetailsSchema"; | ||
| import { useCallback } from "react"; | ||
|
|
||
| const i18nPrefix = "dashboard.agentProfile.organisationDetails"; | ||
|
|
||
| type Props = { | ||
| languagesForForm: Option[]; | ||
| organizationTypeOptions: string[]; | ||
| servicesOptions: string[]; | ||
| operatorOptions: string[]; | ||
| onCancel: () => void; | ||
| onSubmit: () => void; | ||
| }; | ||
|
|
@@ -22,6 +24,7 @@ export const OrganisationDetailsEdit = ({ | |
| languagesForForm, | ||
| organizationTypeOptions, | ||
| servicesOptions, | ||
| operatorOptions, | ||
| onCancel, | ||
| onSubmit, | ||
| }: Props) => { | ||
|
|
@@ -31,6 +34,16 @@ export const OrganisationDetailsEdit = ({ | |
| formState: { errors, isDirty, isValid }, | ||
| } = useFormContext<OrganisationDetailsFormData>(); | ||
|
|
||
| const displayOperators = useCallback( | ||
| (value: string) => { | ||
| if (value?.length >= 3) { | ||
| return operatorOptions.filter((op) => op.toLowerCase().includes(value.toLowerCase())); | ||
| } else { | ||
| return []; | ||
| } | ||
| }, | ||
| [operatorOptions], | ||
| ); | ||
| return ( | ||
| <> | ||
| <FormDetails data-testid="organisation-details-edit"> | ||
|
|
@@ -123,14 +136,19 @@ export const OrganisationDetailsEdit = ({ | |
| name="operator" | ||
| control={control} | ||
| render={({ field }) => ( | ||
| <EditableField | ||
| mode="edit" | ||
| type="text" | ||
| label={t(`${i18nPrefix}.operator`)} | ||
| value={field.value} | ||
| setValue={field.onChange} | ||
| errorMessage={errors.operator?.message} | ||
| /> | ||
| <> | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The operator |
||
| <EditableField | ||
| mode="edit" | ||
| type="autocomplete" | ||
| label={t(`${i18nPrefix}.operator`)} | ||
| value={field.value} | ||
| setValue={field.onChange} | ||
| errorMessage={errors.operator?.message} | ||
| options={displayOperators(field.value)} | ||
| placeholder={t(`${i18nPrefix}.placeholders.operatorPlaceholder`)} | ||
| hint={t(`dashboard.agentProfile.organisationDetails.validation.operatorHint`)} | ||
| /> | ||
| </> | ||
| )} | ||
| /> | ||
| <Controller | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -264,7 +264,7 @@ const StepperValue = styled.span` | |
| user-select: none; | ||
| `; | ||
|
|
||
| type EditableFieldType = "text" | "textarea" | "number" | "stepper" | "checkbox-list" | "radio-list"; | ||
| type EditableFieldType = "text" | "textarea" | "number" | "stepper" | "checkbox-list" | "radio-list" | "autocomplete"; | ||
|
|
||
| export interface EditableFieldRef<T> { | ||
| getValue: () => T; | ||
|
|
@@ -559,6 +559,72 @@ export const EditableField = forwardRef(function EditableField<T extends string | |
| )} | ||
| </DropdownWrapper> | ||
| )} | ||
|
|
||
| {type === "autocomplete" && ( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplification: This new autocomplete block duplicates the existing checkbox-list/radio-list dropdown structure (DropdownWrapper/DropdownButton/DropdownList/OptionRow) almost line-for-line instead of reusing it. ~55 lines at 506-561 and ~55 lines at 563-620 both implement the same wrapper/button/list/option-row pattern, differing only in the button's inner content and single- vs multi-select. Any future change to dropdown behavior (styling, keyboard nav, outside-click handling) now has to be made twice and can drift out of sync.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. originally I wanted to reuse the checkbox-list/radio-list but I found it was easier to create a new type, as otherwise you are complicating this checkbox/radio-lists with extra props and conditions (also risks accidentally mutating their existing behaviour). Also now they have different behaviours and slightly different appearences (now no chevron arrow for I think also if there is a future change to the checkbox-list/radio-list it shouldn't neccessarily mean that the same change should apply to the autocomplete?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Simplification: The new Failure scenario: radio-list's button toggles open/closed, autocomplete's only opens and never closes on click; radio-list's list renders on
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The new |
||
| <DropdownWrapper ref={wrapperRef}> | ||
| <DropdownButton | ||
| $hasError={!!errorMessage} | ||
| onClick={() => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [CONFIRMED] The autocomplete dropdown closes on a plain click inside its own input, not just on option selection or outside click. With the dropdown open and a non-empty value, clicking inside the |
||
| if (localValue) setOpen((o) => !o); | ||
| }} | ||
| > | ||
| <InputWrapper> | ||
| <input | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correctness: The new autocomplete Failure scenario: Dormant today since no current |
||
| type="text" | ||
| value={localValue} | ||
| placeholder={placeholder} | ||
| onChange={(e) => { | ||
| const v = e.target.value as T; | ||
| setLocalValue(v); | ||
| setValue(v); | ||
| setOpen(true); | ||
| }} | ||
| onBlur={handleSubmit} | ||
| onKeyDown={handleKeyDown} | ||
| style={{ border: "none", padding: 0 }} | ||
| /> | ||
| {Boolean(localValue) && ( | ||
| <ClearButton | ||
| type="button" | ||
| onClick={(e) => { | ||
| e.stopPropagation(); | ||
| const v = "" as T; | ||
| setLocalValue(v); | ||
| setValue(v); | ||
| setOpen(false); | ||
| }} | ||
| style={{ alignSelf: "flex-start", marginTop: "0px", marginRight: "0px" }} | ||
| > | ||
| <XCircleIcon size={20} weight="bold" /> | ||
| </ClearButton> | ||
| )} | ||
| </InputWrapper> | ||
| </DropdownButton> | ||
|
|
||
| {open && ( | ||
| <DropdownList> | ||
| {options.map((option) => { | ||
| const isSelected = localValue === option; | ||
| return ( | ||
| <OptionRow | ||
| key={option} | ||
| $isSelected={isSelected} | ||
| onClick={() => { | ||
| const v = option as T; | ||
| setLocalValue(v); | ||
| setValue(v); | ||
| setOpen(false); | ||
| }} | ||
| > | ||
| <input type="radio" name={label} value={option} checked={isSelected} onChange={() => {}} /> | ||
| <Text>{option}</Text> | ||
| </OptionRow> | ||
| ); | ||
| })} | ||
| </DropdownList> | ||
| )} | ||
| </DropdownWrapper> | ||
| )} | ||
| </FieldWrapper> | ||
| {error && ( | ||
| <p style={{ color: "var(--editableField-error-color)", paddingLeft: "var(--editableField-error-paddingLeft)" }}> | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { ApiOrganizationGetList } from "need4deed-sdk"; | ||
| import { useGetQuery } from "./useGetQuery"; | ||
| import { apiPathOrganization, cacheTTL } from "@/config/constants"; | ||
|
|
||
| export const useGetOrganization = () => { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Efficiency: Failure scenario: Backend |
||
| const { data, isLoading, isError, error } = useGetQuery<ApiOrganizationGetList[]>({ | ||
| queryKey: ["organization"], | ||
| apiPath: `${apiPathOrganization}`, | ||
| staleTime: cacheTTL, | ||
| addLang: false, | ||
| }); | ||
|
|
||
| return { | ||
| data, | ||
| isLoading, | ||
| isError, | ||
| error, | ||
| }; | ||
| }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New i18n keys
operatorRequiredandoperatorInvalid(and theirdecounterparts) are added but never referenced by anyt(...)call outside the translation files. This copy looks like it was meant to surface the exact failure described in the operator-save finding, but it was never wired into the schema or theEditableFielderror path — translators now maintain strings nothing displays.