Skip to content

[ENG-580] feat: allow orgs to be cleared in Role org selector #16464

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.

Sign up for GitHub

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

Open
Jacobjeevan wants to merge 2 commits into
base: develop
Choose a base branch
from
+17 −19
Open

[ENG-580] feat: allow orgs to be cleared in Role org selector #16464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f826b8c
feat: allow orgs to be cleared in roleorg/facilityorg selector
Jacobjeevan Jun 18, 2026
ded55b9
Merge branch 'develop' into ENG-580-tag-managing-org
nihal467 Jun 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/components/Common/RoleOrgSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ export default function RoleOrgSelector(props: RoleOrgSelectorProps) {

const [selectedOrganizations, setSelectedOrganizations] = useState<
Organization[]
>([]);
>(currentOrganizations || []);
Comment on lines 44 to +47

@greptile-apps greptile-apps Bot Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Stale initial state — existing org not displayed when editing

useState only consumes its initializer on the very first render. In TagConfigForm, existingConfig is fetched asynchronously, so currentOrganizations is [] at the time RoleOrgSelector mounts. By the time the query resolves and currentOrganizations changes to [existingConfig.organization], selectedOrganizations is already frozen at [].

FacilityOrganizationSelector already solves this with a dedicated useEffect (lines 292–305) that calls setSelectedOrganizations(resolvedOrganizations) whenever value or currentOrganizations changes. RoleOrgSelector needs the same pattern; without it, the organization selector appears empty when opening the edit drawer for an existing tag config, even though the underlying form value is correctly set.

Comment on lines 45 to +47

@coderabbitai coderabbitai Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify whether selectedOrganizations is synchronized after mount.
rg -n -C3 'useState<\s*Organization\[\]\s*>\(|setSelectedOrganizations\(|useEffect\(' src/components/Common/RoleOrgSelector.tsx
rg -n -C3 'RoleOrgSelector|currentOrganizations|value=' src/pages/Admin/TagConfig/TagConfigForm.tsx

Repository: ohcnetwork/care_fe

Length of output: 4171

🏁 Script executed:

cat -n src/components/Common/RoleOrgSelector.tsx

Repository: ohcnetwork/care_fe

Length of output: 14712

Line 45–47: useState(currentOrganizations || []) doesn't sync when async props update post-mount.

The initial state from currentOrganizations is used only on first render. When currentOrganizations or value changes after mount (e.g., edit form data loading), selectedOrganizations becomes stale. The existing useEffect at line 114 only handles auto-selection; it doesn't synchronize state with prop changes. Add a sync effect keyed to value and currentOrganizations:

Suggested fix 
   const [selectedOrganizations, setSelectedOrganizations] = useState<
     Organization[]
   >(currentOrganizations || []);
+
+  useEffect(() => {
+    if (value && value.length > 0) {
+      const resolvedOrganizations = value
+        .map((id) => currentOrganizations?.find((org) => org.id === id))
+        .filter((org): org is Organization => Boolean(org));
+      setSelectedOrganizations(resolvedOrganizations);
+      return;
+    }
+    setSelectedOrganizations(currentOrganizations || []);
+  }, [value, currentOrganizations]);
🤖 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 `@src/components/Common/RoleOrgSelector.tsx` around lines 45 - 47, The
selectedOrganizations state in RoleOrgSelector is initialized with
currentOrganizations only once at component mount, so it becomes stale when
currentOrganizations or value props change after the initial render (such as
when async data loads). Add a separate useEffect hook that watches for changes
to currentOrganizations and value props and updates selectedOrganizations state
accordingly. This sync effect should be distinct from the existing useEffect at
line 114 that handles auto-selection, with dependencies on [value,
currentOrganizations] to ensure the state stays synchronized whenever these
props change.

const [currentSelection, setCurrentSelection] = useState<Organization | null>(
null,
);
Expand Down
24 changes: 10 additions & 14 deletions src/pages/Admin/TagConfig/TagConfigForm.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,8 @@ export default function TagConfigForm({
resource: z.nativeEnum(TagResource, {
required_error: t("field_required"),
}),
facility_organization: z.string().optional(),
organization: z.string().optional(),
facility_organization: z.string().nullable(),
organization: z.string().nullable(),
});

type TagConfigFormValues = z.infer<typeof tagConfigSchema>;
Expand All @@ -99,8 +99,8 @@ export default function TagConfigForm({
priority: parentTag?.priority || 100,
status: TagStatus.ACTIVE,
resource: parentTag?.resource || TagResource.PATIENT,
facility_organization: undefined,
organization: undefined,
facility_organization: null,
organization: null,
},
});

Expand All @@ -124,8 +124,8 @@ export default function TagConfigForm({
priority: existingConfig.priority,
status: existingConfig.status,
resource: existingConfig.resource,
facility_organization: existingConfig.facility_organization?.id,
organization: existingConfig.organization?.id,
facility_organization: existingConfig.facility_organization?.id ?? null,
organization: existingConfig.organization?.id ?? null,
});
}
}, [existingConfig, isEditing, form]);
Expand All @@ -140,8 +140,8 @@ export default function TagConfigForm({
priority: parentTag.priority,
status: TagStatus.ACTIVE,
resource: parentTag.resource,
facility_organization: undefined,
organization: undefined,
facility_organization: null,
organization: null,
});
}
}, [parentTag, isCreatingChild, form]);
Expand Down Expand Up @@ -184,12 +184,8 @@ export default function TagConfigForm({
resource: data.resource,
...(parentId && { parent: parentId }),
...(facilityId && { facility: facilityId }),
...(data.facility_organization && {
facility_organization: data.facility_organization,
}),
...(data.organization && {
organization: data.organization,
}),
facility_organization: data.facility_organization,
organization: data.organization,
Comment thread
Jacobjeevan marked this conversation as resolved.
};

if (isEditing) {
Expand Down
2 changes: 2 additions & 0 deletions src/pages/Admin/TagConfig/TagConfigView.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ export default function TagConfigView({
priority: childData.priority,
status: TagStatus.ARCHIVED,
resource: childData.resource,
facility_organization: childData.facility_organization?.id ?? null,
organization: childData.organization?.id ?? null,
parent:
childData.parent &&
typeof childData.parent === "object" &&
Expand Down
4 changes: 2 additions & 2 deletions src/pages/Facility/settings/organizations/components/FacilityOrganizationSelector.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ export default function FacilityOrganizationSelector(

const handleConfirmSelection = useCallback(
(org: FacilityOrganizationRead) => {
if (!selectedOrganizations.includes(org)) {
if (!selectedOrganizations.find((o) => o.id === org.id)) {
const newSelection = singleSelection
? [org]
: [...selectedOrganizations, org];
Expand Down Expand Up @@ -302,7 +302,7 @@ export default function FacilityOrganizationSelector(
// Reset the auto-select flag when value is cleared (e.g., form reset)
// setHasAutoSelectedPreferred(false);
}
}, [value, currentOrganizations, showAllOrgs]);
}, [value, currentOrganizations]);

// Auto-select preferred departments
useEffect(() => {
Expand Down
4 changes: 2 additions & 2 deletions src/types/emr/tagConfig/tagConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,9 @@ export interface TagConfigRequest {
status: TagStatus;
parent?: string | null;
resource: TagResource;
organization?: string;
organization: string | null;
facility?: string;
facility_organization?: string;
facility_organization: string | null;
Comment on lines +78 to +80

@coderabbitai coderabbitai Bot Jun 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Description: Search for backend TagConfig serializer/model validation that may need to handle the new required-nullable contract

# Search for TagConfig model/serializer definitions
rg -n -C5 -i 'class.*tagconfig.*(serializer|model|viewset)' --type py

# Search for organization field validation
rg -n -C3 'organization.*field|facility_organization.*field' --type py -g '*tag*config*'

Repository: ohcnetwork/care_fe

Length of output: 211

🏁 Script executed:

# First, locate and examine the tagConfig.ts file
fd -n "tagConfig.ts" -t f

Repository: ohcnetwork/care_fe

Length of output: 293

🏁 Script executed:

# Check the context around lines 78-80 in the tagConfig file
if [ -f "src/types/emr/tagConfig/tagConfig.ts" ]; then
  cat -n "src/types/emr/tagConfig/tagConfig.ts" | sed -n '70,90p'
fi

Repository: ohcnetwork/care_fe

Length of output: 753

🏁 Script executed:

# Search for TagConfigRequest usage in mutations and queries
rg -n "TagConfigRequest" -t ts -t tsx -C 3

Repository: ohcnetwork/care_fe

Length of output: 90

🏁 Script executed:

# Look for API endpoint definitions for tag config
rg -n "tag_config" -t ts -t tsx -A 2 -B 2 -g "*Api.ts"

Repository: ohcnetwork/care_fe

Length of output: 90

🏁 Script executed:

# Check git history for changes to this interface
git log -p --follow -S "organization: string | null" -- "src/types/emr/tagConfig/tagConfig.ts" | head -100

Repository: ohcnetwork/care_fe

Length of output: 44

🏁 Script executed:

# Search for TagConfigRequest usage in mutations and queries
rg "TagConfigRequest" -t ts -A 3 -B 3

Repository: ohcnetwork/care_fe

Length of output: 2651

🏁 Script executed:

# Look for form usage or mutation definitions related to tag config
rg -i "tagconfig" -t ts -l | head -20

Repository: ohcnetwork/care_fe

Length of output: 1059

🏁 Script executed:

# Check API route definitions for tag config
rg "tag_config" -t ts --max-count 20

Repository: ohcnetwork/care_fe

Length of output: 3707

🏁 Script executed:

# View TagConfigForm to see how the payload is constructed, particularly organization and facility_organization fields
cat -n "src/pages/Admin/TagConfig/TagConfigForm.tsx" | head -150

Repository: ohcnetwork/care_fe

Length of output: 5816

🏁 Script executed:

# See more of the TagConfigForm to understand the form structure and how organization/facility_organization are handled
cat -n "src/pages/Admin/TagConfig/TagConfigForm.tsx" | sed -n '1,250p'

Repository: ohcnetwork/care_fe

Length of output: 9717

🏁 Script executed:

# Continue viewing the form to see organization and facility_organization field rendering
cat -n "src/pages/Admin/TagConfig/TagConfigForm.tsx" | sed -n '250,400p'

Repository: ohcnetwork/care_fe

Length of output: 6074

🏁 Script executed:

# Check the git history to see if organization/facility_organization were changed from optional to required-nullable
git log --oneline -20 -- "src/types/emr/tagConfig/tagConfig.ts"

Repository: ohcnetwork/care_fe

Length of output: 134

🏁 Script executed:

# Check the specific change in the git commit to see what changed from previous state
git show f826b8cf -- "src/types/emr/tagConfig/tagConfig.ts" | head -100

Repository: ohcnetwork/care_fe

Length of output: 2738

🏁 Script executed:

# Also check the full diff of that commit
git show f826b8cf --stat

Repository: ohcnetwork/care_fe

Length of output: 50374

🏁 Script executed:

# Let's verify if this file existed before this commit or if it's truly new
git log --all --full-history -- "src/types/emr/tagConfig/tagConfig.ts" | head -50

Repository: ohcnetwork/care_fe

Length of output: 1751

🏁 Script executed:

# Check the parent commit to see if the file existed before
git show f826b8cf^:src/types/emr/tagConfig/tagConfig.ts 2>&1 | head -20

Repository: ohcnetwork/care_fe

Length of output: 102

🏁 Script executed:

# Check if this is truly a new file or if it was modified
git log --diff-filter=A -- "src/types/emr/tagConfig/tagConfig.ts"

Repository: ohcnetwork/care_fe

Length of output: 260

🏁 Script executed:

# Let's look at the parent commit to see what existed before
git show 01dad4c040aca11befba547f2e4a0a9ebffbaa41:src/types/emr/tagConfig/tagConfig.ts 2>&1 | head -100

Repository: ohcnetwork/care_fe

Length of output: 2478

🏁 Script executed:

# Let's verify the form data handling one more time to ensure null serialization is correct
cat -n "src/pages/Admin/TagConfig/TagConfigForm.tsx" | sed -n '177,196p'

Repository: ohcnetwork/care_fe

Length of output: 807

Verify backend API compatibility with required-nullable organization fields.

This change alters the API contract: organization and facility_organization are now required fields that can be null, rather than optional fields. Previously, these fields could be omitted entirely from the request payload; now they must always be present (set to null when cleared).

The form data construction confirms the payload will always include both fields:

  • Default values are explicitly set to null (lines 102-103)
  • Zod schema requires both fields as nullable strings (lines 77-78)
  • The mutation payload includes both fields unconditionally (lines 187-188)

Ensure the backend endpoints (POST /api/v1/tag_config/ and PUT /api/v1/tag_config/{external_id}/) correctly accept and handle null values for both fields in request payloads.

🤖 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 `@src/types/emr/tagConfig/tagConfig.ts` around lines 78 - 80, The tag config
type definition now requires `organization` and `facility_organization` to
always be present in API payloads as nullable fields (rather than optional).
Verify that the backend endpoints POST /api/v1/tag_config/ and PUT
/api/v1/tag_config/{external_id}/ correctly accept and handle null values for
both the organization and facility_organization fields when they are sent in
request payloads. Update backend validation logic if needed to allow null values
for these required fields instead of treating them as missing fields.

}

export function getTagHierarchyDisplay(
Expand Down
Loading