feat: Make site/project/note data required for relevant screens (#2636)

dev-client/__tests__/integration/RestrictByRequirements.test.tsx → dev-client/__tests__/integration/ScreenDataRequirements.test.tsx

@@ -19,9 +19,9 @@ import {Text} from 'react-native';
 
 import {render} from '@testing/integration/utils';
 
-import {RestrictByRequirements} from 'terraso-mobile-client/components/dataRequirements/RestrictByRequirements';
+import {ScreenDataRequirements} from 'terraso-mobile-client/components/dataRequirements/ScreenDataRequirements';
 
-describe('RestrictByRequrements', () => {
+describe('ScreenDataRequirements', () => {
   test('renders children and triggers no actions when required data exists', () => {
     let thingsDone = '';
     const requirements = [
@@ -37,9 +37,9 @@ describe('RestrictByRequrements', () => {
     ];
 
     const {queryByText} = render(
-      <RestrictByRequirements requirements={requirements}>
+      <ScreenDataRequirements requirements={requirements}>
         {() => <Text>Hello world</Text>}
-      </RestrictByRequirements>,
+      </ScreenDataRequirements>,
     );
 
     expect(queryByText('Hello world')).toBeTruthy();
@@ -64,9 +64,9 @@ describe('RestrictByRequrements', () => {
     ];
 
     const {queryByText} = render(
-      <RestrictByRequirements requirements={requirements}>
+      <ScreenDataRequirements requirements={requirements}>
         {() => <Text>Hello world</Text>}
-      </RestrictByRequirements>,
+      </ScreenDataRequirements>,
     );
 
     expect(queryByText('Hello world')).toBeNull();
@@ -91,12 +91,31 @@ describe('RestrictByRequrements', () => {
     ];
 
     const {queryByText} = render(
-      <RestrictByRequirements requirements={requirements}>
+      <ScreenDataRequirements requirements={requirements}>
         {() => <Text>Hello world</Text>}
-      </RestrictByRequirements>,
+      </ScreenDataRequirements>,
     );
 
     expect(queryByText('Hello world')).toBeNull();
     expect(thingsDone).toEqual('null');
   });
+
+  test('renders children and triggers no action when required data exists, even if it is a boolean equal to false', () => {
+    let thingsDone = '';
+    const requirements = [
+      {
+        data: false,
+        doIfMissing: () => (thingsDone += 'false'),
+      },
+    ];
+
+    const {queryByText} = render(
+      <ScreenDataRequirements requirements={requirements}>
+        {() => <Text>Hello world</Text>}
+      </ScreenDataRequirements>,
+    );
+
+    expect(queryByText('Hello world')).toBeTruthy();
+    expect(thingsDone).toEqual('');
+  });
 });

dev-client/src/components/dataRequirements/RestrictByRequirements.tsx → dev-client/src/components/dataRequirements/ScreenDataRequirements.tsx

@@ -15,7 +15,7 @@
  * along with this program. If not, see https://www.gnu.org/licenses/.
  */
 
-import {useEffect} from 'react';
+import {useEffect, useMemo} from 'react';
 
 type Requirement = {
   data: any;
@@ -49,11 +49,31 @@ type Props = {
   children: () => React.ReactNode;
 };
 
-export const RestrictByRequirements = ({requirements, children}: Props) => {
+/*
+ * This is intended to wrap components (mostly screens as of 2024-12) so they only render
+ * if required data is truthy. This prevents screens from breaking if, for example, a pull
+ * happens that deletes data that is required to view the screen.
+ */
+export const ScreenDataRequirements = ({requirements, children}: Props) => {
   const requiredDataExists = useRequiredData(requirements);
-
   if (!requiredDataExists) {
     return null;
   }
   return <>{children()}</>;
 };
+
+/*
+ * I believe this is not strictly necessary; if a screen is re-rendering, the ScreenDataRequirements component
+ * will re-render regardless of if its props change (unless memoized)
+ */
+export const useMemoizedRequirements = (
+  requirementsAsNewObj: Requirement[],
+) => {
+  const deps = requirementsAsNewObj.flatMap(r => [r.data, r.doIfMissing]);
+  return useMemo(() => {
+    return requirementsAsNewObj;
+    // The linter doesn't like the dynamic dependency list, but the useMemo should only depend on the values of the required data
+    // and missing data handlers. If it depends on the list object, it's re-created every render.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, deps);
+};

dev-client/src/components/dataRequirements/handleMissingData.ts

@@ -21,7 +21,7 @@ import {isFlagEnabled} from 'terraso-mobile-client/config/featureFlags';
 import {useSyncNotificationContext} from 'terraso-mobile-client/context/SyncNotificationContext';
 import {useNavigation} from 'terraso-mobile-client/navigation/hooks/useNavigation';
 
-export const useHandleMissingSite = () => {
+export const useNavToBottomTabsAndShowSyncError = () => {
   const navigation = useNavigation();
   const syncNotifications = useSyncNotificationContext();
 
@@ -32,3 +32,15 @@ export const useHandleMissingSite = () => {
     }
   }, [navigation, syncNotifications]);
 };
+
+export const usePopNavigationAndShowSyncError = () => {
+  const navigation = useNavigation();
+  const syncNotifications = useSyncNotificationContext();
+
+  return useCallback(() => {
+    navigation.pop();
+    if (isFlagEnabled('FF_offline')) {
+      syncNotifications.showError();
+    }
+  }, [navigation, syncNotifications]);
+};

dev-client/src/navigation/screenDefinitions.tsx

@@ -28,9 +28,10 @@ import {CreateProjectScreen} from 'terraso-mobile-client/screens/CreateProjectSc
 import {CreateSiteScreen} from 'terraso-mobile-client/screens/CreateSiteScreen/CreateSiteScreen';
 import {DeleteAccountScreen} from 'terraso-mobile-client/screens/DeleteAccountScreen/DeleteAccountScreen';
 import {EditPinnedNoteScreen} from 'terraso-mobile-client/screens/EditPinnedNoteScreen';
-import {LocationSoilIdScreen} from 'terraso-mobile-client/screens/LocationScreens/LocationSoilIdScreen';
+import {SiteLocationSoilIdScreen} from 'terraso-mobile-client/screens/LocationScreens/SiteLocationSoilIdScreen';
 import {SiteTabsScreen} from 'terraso-mobile-client/screens/LocationScreens/SiteTabsScreen';
 import {TemporaryLocationScreen} from 'terraso-mobile-client/screens/LocationScreens/TemporaryLocationScreen';
+import {TemporaryLocationSoilIdScreen} from 'terraso-mobile-client/screens/LocationScreens/TemporaryLocationSoilIdScreen';
 import {LoginScreen} from 'terraso-mobile-client/screens/LoginScreen';
 import {ManageTeamMemberScreen} from 'terraso-mobile-client/screens/ManageTeamMemberScreen';
 import {ProjectListScreen} from 'terraso-mobile-client/screens/ProjectListScreen/ProjectListScreen';
@@ -59,6 +60,8 @@ import {TextureScreen} from 'terraso-mobile-client/screens/SoilScreen/TextureScr
 import {UserSettingsScreen} from 'terraso-mobile-client/screens/UserSettingsScreen/UserSettingsScreen';
 import {WelcomeScreen} from 'terraso-mobile-client/screens/WelcomeScreen';
 
+SiteLocationSoilIdScreen;
+
 export const bottomTabScreensDefinitions = {
   PROJECT_LIST: ProjectListScreen,
   SITES: SitesScreen,
@@ -75,7 +78,8 @@ export const screenDefinitions = {
   CREATE_SITE: CreateSiteScreen,
   TEMP_LOCATION: TemporaryLocationScreen,
   SITE_TABS: SiteTabsScreen,
-  LOCATION_SOIL_ID: LocationSoilIdScreen,
+  SITE_LOCATION_SOIL_ID: SiteLocationSoilIdScreen,
+  TEMPORARY_LOCATION_SOIL_ID: TemporaryLocationSoilIdScreen,
   SITE_SETTINGS: SiteSettingsScreen,
   SITE_TEAM_SETTINGS: SiteTeamSettingsScreen,
   ADD_USER_PROJECT: AddUserToProjectScreen,

dev-client/src/screens/AddUserToProjectScreen/AddUserToProjectRoleScreen.tsx

@@ -24,6 +24,14 @@ import {Button} from 'native-base';
 import {ProjectRole} from 'terraso-client-shared/project/projectTypes';
 
 import {ScreenContentSection} from 'terraso-mobile-client/components/content/ScreenContentSection';
+import {
+  useNavToBottomTabsAndShowSyncError,
+  usePopNavigationAndShowSyncError,
+} from 'terraso-mobile-client/components/dataRequirements/handleMissingData';
+import {
+  ScreenDataRequirements,
+  useMemoizedRequirements,
+} from 'terraso-mobile-client/components/dataRequirements/ScreenDataRequirements';
 import {
   Box,
   Column,
@@ -69,48 +77,59 @@ export const AddUserToProjectRoleScreen = ({projectId, userId}: Props) => {
     navigation.dispatch(TabActions.jumpTo(TabRoutes.TEAM));
   }, [dispatch, projectId, user, selectedRole, navigation]);
 
+  const handleMissingProject = useNavToBottomTabsAndShowSyncError();
+  const handleMissingUser = usePopNavigationAndShowSyncError();
+  const requirements = useMemoizedRequirements([
+    {data: project, doIfMissing: handleMissingProject},
+    {data: user, doIfMissing: handleMissingUser},
+  ]);
+
   return (
-    <ScreenScaffold AppBar={<AppBar title={project?.name} />}>
-      <ScreenContentSection title={t('projects.add_user.heading')}>
-        <Column>
-          <Box ml="md" my="lg">
-            <MinimalUserDisplay user={user} />
-          </Box>
+    <ScreenDataRequirements requirements={requirements}>
+      {() => (
+        <ScreenScaffold AppBar={<AppBar title={project?.name} />}>
+          <ScreenContentSection title={t('projects.add_user.heading')}>
+            <Column>
+              <Box ml="md" my="lg">
+                <MinimalUserDisplay user={user} />
+              </Box>
 
-          <ProjectRoleRadioBlock
-            onChange={setSelectedRole}
-            selectedRole={selectedRole}
-          />
+              <ProjectRoleRadioBlock
+                onChange={setSelectedRole}
+                selectedRole={selectedRole}
+              />
 
-          <Row
-            flex={0}
-            justifyContent="flex-end"
-            alignItems="center"
-            space="12px"
-            pt="md">
-            <Button
-              onPress={onCancel}
-              size="lg"
-              variant="outline"
-              borderColor="action.active"
-              _text={{textTransform: 'uppercase', color: 'action.active'}}>
-              {t('general.cancel')}
-            </Button>
-            {/* FYI: The 1px border is to visually match the size of the outline
+              <Row
+                flex={0}
+                justifyContent="flex-end"
+                alignItems="center"
+                space="12px"
+                pt="md">
+                <Button
+                  onPress={onCancel}
+                  size="lg"
+                  variant="outline"
+                  borderColor="action.active"
+                  _text={{textTransform: 'uppercase', color: 'action.active'}}>
+                  {t('general.cancel')}
+                </Button>
+                {/* FYI: The 1px border is to visually match the size of the outline
             variant, which appears to be 1px bigger than the solid variant due
             to its border. */}
-            <Button
-              borderWidth="1px"
-              borderColor="primary.main"
-              onPress={addUser}
-              size="lg"
-              variant="solid"
-              _text={{textTransform: 'uppercase'}}>
-              {t('general.add')}
-            </Button>
-          </Row>
-        </Column>
-      </ScreenContentSection>
-    </ScreenScaffold>
+                <Button
+                  borderWidth="1px"
+                  borderColor="primary.main"
+                  onPress={addUser}
+                  size="lg"
+                  variant="solid"
+                  _text={{textTransform: 'uppercase'}}>
+                  {t('general.add')}
+                </Button>
+              </Row>
+            </Column>
+          </ScreenContentSection>
+        </ScreenScaffold>
+      )}
+    </ScreenDataRequirements>
   );
 };

dev-client/src/screens/AddUserToProjectScreen/AddUserToProjectScreen.tsx

@@ -18,11 +18,17 @@
 import {useTranslation} from 'react-i18next';
 
 import {ScreenContentSection} from 'terraso-mobile-client/components/content/ScreenContentSection';
+import {useNavToBottomTabsAndShowSyncError} from 'terraso-mobile-client/components/dataRequirements/handleMissingData';
+import {
+  ScreenDataRequirements,
+  useMemoizedRequirements,
+} from 'terraso-mobile-client/components/dataRequirements/ScreenDataRequirements';
 import {Box, Text} from 'terraso-mobile-client/components/NativeBaseAdapters';
 import {AppBar} from 'terraso-mobile-client/navigation/components/AppBar';
 import {AddTeamMemberForm} from 'terraso-mobile-client/screens/AddUserToProjectScreen/components/AddTeamMemberForm';
 import {ScreenScaffold} from 'terraso-mobile-client/screens/ScreenScaffold';
 import {useSelector} from 'terraso-mobile-client/store';
+import {selectProject} from 'terraso-mobile-client/store/selectors';
 
 type Props = {
   projectId: string;
@@ -31,22 +37,29 @@ type Props = {
 export const AddUserToProjectScreen = ({projectId}: Props) => {
   const {t} = useTranslation();
 
-  const projectName = useSelector(
-    state => state.project.projects[projectId]?.name,
-  );
+  const project = useSelector(selectProject(projectId));
 
   // FYI: There was previously a mechanism to enter emails individually, but set roles at the same time.
   // This was replaced, but we could refer back to `userRecord` in previous versions if we ever end up
   // wanting to add multiple users at the same time.
 
+  const handleMissingProject = useNavToBottomTabsAndShowSyncError();
+  const requirements = useMemoizedRequirements([
+    {data: project, doIfMissing: handleMissingProject},
+  ]);
+
   return (
-    <ScreenScaffold AppBar={<AppBar title={projectName} />}>
-      <ScreenContentSection title={t('projects.add_user.heading')}>
-        <Text variant="body1">{t('projects.add_user.help_text')}</Text>
-        <Box mt="md">
-          <AddTeamMemberForm projectId={projectId} />
-        </Box>
-      </ScreenContentSection>
-    </ScreenScaffold>
+    <ScreenDataRequirements requirements={requirements}>
+      {() => (
+        <ScreenScaffold AppBar={<AppBar title={project.name} />}>
+          <ScreenContentSection title={t('projects.add_user.heading')}>
+            <Text variant="body1">{t('projects.add_user.help_text')}</Text>
+            <Box mt="md">
+              <AddTeamMemberForm projectId={projectId} />
+            </Box>
+          </ScreenContentSection>
+        </ScreenScaffold>
+      )}
+    </ScreenDataRequirements>
   );
 };

dev-client/src/screens/CreateSiteScreen/CreateSiteScreen.tsx

@@ -22,7 +22,6 @@ import {Coords} from 'terraso-client-shared/types';
 
 import {ScreenCloseButton} from 'terraso-mobile-client/components/buttons/icons/appBar/ScreenCloseButton';
 import {addSite} from 'terraso-mobile-client/model/site/siteGlobalReducer';
-import {fetchSitesForProject} from 'terraso-mobile-client/model/site/siteSlice';
 import {AppBar} from 'terraso-mobile-client/navigation/components/AppBar';
 import {CreateSiteView} from 'terraso-mobile-client/screens/CreateSiteScreen/components/CreateSiteView';
 import {ScreenScaffold} from 'terraso-mobile-client/screens/ScreenScaffold';
@@ -32,9 +31,6 @@ type Props =
   | {
       coords: Coords;
     }
-  | {
-      projectId: string;
-    }
   | {
       elevation: number;
     }
@@ -52,9 +48,6 @@ export const CreateSiteScreen = (props: Props = {}) => {
         console.error(result.payload.parsedErrors);
         return;
       }
-      if (input.projectId) {
-        dispatch(fetchSitesForProject(input.projectId));
-      }
       return result.payload;
     },
     [dispatch],
@@ -66,7 +59,6 @@ export const CreateSiteScreen = (props: Props = {}) => {
       AppBar={<AppBar LeftButton={<ScreenCloseButton />} />}>
       <CreateSiteView
         createSiteCallback={createSiteCallback}
-        defaultProjectId={'projectId' in props ? props.projectId : undefined}
         sitePin={'coords' in props ? props.coords : undefined}
         elevation={'elevation' in props ? props.elevation : undefined}
       />