fix: Hofixes for the localunit #1624

shreeyash07 · 2025-01-15T08:40:00Z

Summary

Small hotfixes for the locaunit

Addresses

Issue(s): No issues was Created

Depends On

Add global validator on userme serializer go-api#2385

Changes

Add new logic for the isNewLocalUnit
Remove pristine from LocalUnitValidationButton
Add missing form fields in localunit view component
Add global validator permission
Add check of the changes in localunitview component

This PR Ensures:

No typos or grammatical errors
No conflict markers left in the code
No unwanted comments, temporary files, or auto-generated files
No inclusion of secret keys or sensitive data
No console.log statements meant for debugging
All CI checks have passed

changeset-bot · 2025-01-15T08:40:04Z

⚠️ No Changeset found

Latest commit: 53d8804

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

samshara · 2025-01-16T05:47:25Z

...ewContextAndStructure/NationalSocietyLocalUnits/LocalUnitsFormModal/LocalUnitsForm/index.tsx

+    const isNewLocalUnit = useMemo(() => {
+        if (isObject(previousData)) {
+            if (Object.keys(previousData).length <= 0) {
+                return true;
+            }
+        }
+        return false;
+    }, [previousData]);


Suggested change

const isNewLocalUnit = useMemo(() => {

if (isObject(previousData)) {

if (Object.keys(previousData).length <= 0) {

return true;

}

}

return false;

}, [previousData]);

const isNewLocalUnit = useMemo(() => {

return isNotDefined(previousData) || Object.keys(previousData).length === 0;

}, [previousData]);

frozenhelium · 2025-01-17T05:32:05Z

app/src/hooks/domain/usePermissions.ts

@@ -45,6 +45,8 @@ function usePermissions() {

            const isSuperUser = !isGuestUser && !!userMe?.is_superuser;

+            const isGlobalValidator = !isGuestUser && !!userMe?.is_global_validator;


Is this role generic or specific to local units?

Also I feel like this should be permission instead of role @thenav56 @tnagorra @samshara

@frozenhelium this is specific to local units.

frozenhelium · 2025-01-17T05:33:01Z

app/src/utils/localUnits.ts

+        health,
+        ...formValues
+        // Note: the cast is safe as we're only trying to
+        // remove fields if they exist
+    } = removeNull(value) as LocalUnitResponse;
+
+    const {
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        modified_by_details: healthModifiedByDetails,
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        modified_at: healthModifiedAt,
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        modified_by: healthModifiedby,
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        created_at: healthCreatedAt,
+        // eslint-disable-next-line @typescript-eslint/no-unused-vars
+        created_by_details: healthCreatedByDetails,
+        ...formHealthValues
+    } = health ?? {};


Suggested change

health,

...formValues

// Note: the cast is safe as we're only trying to

// remove fields if they exist

} = removeNull(value) as LocalUnitResponse;

const {

// eslint-disable-next-line @typescript-eslint/no-unused-vars

modified_by_details: healthModifiedByDetails,

// eslint-disable-next-line @typescript-eslint/no-unused-vars

modified_at: healthModifiedAt,

// eslint-disable-next-line @typescript-eslint/no-unused-vars

modified_by: healthModifiedby,

// eslint-disable-next-line @typescript-eslint/no-unused-vars

created_at: healthCreatedAt,

// eslint-disable-next-line @typescript-eslint/no-unused-vars

created_by_details: healthCreatedByDetails,

...formHealthValues

} = health ?? {};

health = {},

...formValues

// Note: the cast is safe as we're only trying to

// remove fields if they exist

} = removeNull(value) as LocalUnitResponse;

const {

// eslint-disable-next-line @typescript-eslint/no-unused-vars

modified_by_details: healthModifiedByDetails,

// eslint-disable-next-line @typescript-eslint/no-unused-vars

modified_at: healthModifiedAt,

// eslint-disable-next-line @typescript-eslint/no-unused-vars

modified_by: healthModifiedby,

// eslint-disable-next-line @typescript-eslint/no-unused-vars

created_at: healthCreatedAt,

// eslint-disable-next-line @typescript-eslint/no-unused-vars

created_by_details: healthCreatedByDetails,

...formHealthValues

} = health;

frozenhelium · 2025-01-17T05:36:50Z

app/src/utils/localUnits.ts

+        const newValueKeys = Object.keys(removeNull(newValue));
+        const oldValueKeys = Object.keys(removeNull(oldValue));
+
+        if (newValueKeys.length !== oldValueKeys.length) {
+            return true;
+        }


Lets verify removeNull might not actually remove null, lets verify it

Alternatively we can check length for list of defined values only

frozenhelium · 2025-01-17T05:38:45Z

...views/CountryNsOverviewContextAndStructure/NationalSocietyLocalUnits/LocalUnitView/index.tsx

-    // FIXME: Handle case when there is no change.
-    // We need to display message to the user
+    const hasDifference = useMemo(() => {
+        if (!newValue || !oldValue) {


Let's use isNotDefined just in case

frozenhelium · 2025-01-17T05:41:24Z

...NsOverviewContextAndStructure/NationalSocietyLocalUnits/LocalUnitViewModal/styles.module.css

@@ -0,0 +1,3 @@
+.local-unit-view-children-container {
+    max-height: 36rem;


Are we trying to set max-height? Lets use min function to ensure that it does not go beyond 100vh for smaller screens

frozenhelium · 2025-01-17T05:42:54Z

...ewContextAndStructure/NationalSocietyLocalUnits/LocalUnitsFormModal/LocalUnitsForm/index.tsx

@@ -430,22 +435,40 @@ function LocalUnitsForm(props: Props) {
    const healthFormError = getErrorObject(error?.health);
    const revertChangesFormError = getErrorObject(revertChangesError);

+    const previousData = (
+        localUnitPreviousResponse?.previous_data_details as unknown as LocalUnitResponse


Let's add FIXME or NOTE why we're casting this

Change logic for the isNewLocalUnit

aa4fd41

shreeyash07 requested review from frozenhelium and samshara January 15, 2025 08:40

shreeyash07 assigned samshara Jan 15, 2025

shreeyash07 temporarily deployed to test January 15, 2025 08:40 — with GitHub Actions Inactive

shreeyash07 temporarily deployed to test January 15, 2025 08:41 — with GitHub Actions Inactive

shreeyash07 temporarily deployed to test January 15, 2025 11:21 — with GitHub Actions Inactive

shreeyash07 temporarily deployed to test January 15, 2025 11:22 — with GitHub Actions Inactive

Add missing form fields in localUnitView component

d43db1d

shreeyash07 temporarily deployed to test January 15, 2025 11:22 — with GitHub Actions Inactive

samshara reviewed Jan 16, 2025

View reviewed changes

Add hasDifference logic for localUnitView component empty message

dbbe0cf

shreeyash07 temporarily deployed to test January 16, 2025 09:37 — with GitHub Actions Inactive

shreeyash07 temporarily deployed to test January 16, 2025 09:38 — with GitHub Actions Inactive

shreeyash07 requested a review from samshara January 16, 2025 09:38

Update isNewLocalUnit logic

7b38a4b

shreeyash07 temporarily deployed to test January 16, 2025 09:39 — with GitHub Actions Inactive

shreeyash07 temporarily deployed to test January 17, 2025 04:44 — with GitHub Actions Inactive

Add local unit global validator permission for validation and edit

53d8804

shreeyash07 temporarily deployed to test January 17, 2025 04:45 — with GitHub Actions Inactive

frozenhelium requested changes Jan 17, 2025

View reviewed changes

roshni73 approved these changes Jan 20, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: Hofixes for the localunit #1624

fix: Hofixes for the localunit #1624

shreeyash07 commented Jan 15, 2025 •

edited

Loading

changeset-bot bot commented Jan 15, 2025 •

edited

Loading

samshara Jan 16, 2025

frozenhelium Jan 17, 2025

samshara Jan 17, 2025

frozenhelium Jan 17, 2025

frozenhelium Jan 17, 2025

frozenhelium Jan 17, 2025

frozenhelium Jan 17, 2025

frozenhelium Jan 17, 2025

		@@ -45,6 +45,8 @@ function usePermissions() {

		const isSuperUser = !isGuestUser && !!userMe?.is_superuser;

		const isGlobalValidator = !isGuestUser && !!userMe?.is_global_validator;

		@@ -0,0 +1,3 @@
		.local-unit-view-children-container {
		max-height: 36rem;

fix: Hofixes for the localunit #1624

Are you sure you want to change the base?

fix: Hofixes for the localunit #1624

Conversation

shreeyash07 commented Jan 15, 2025 • edited Loading

Summary

Addresses

Depends On

Changes

This PR Ensures:

changeset-bot bot commented Jan 15, 2025 • edited Loading

⚠️ No Changeset found

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shreeyash07 commented Jan 15, 2025 •

edited

Loading

changeset-bot bot commented Jan 15, 2025 •

edited

Loading