Change calculation of successful upload status stats (#2869)

* Show upload status also if we have no status but an error

E.g. if all uploads fail.

* Change number of successful upploads

Now to use number of attempts made minus number of failed uploads.

* Add unit tests

* Refactor state of successful uploads into store

src/components/MyObservations/SimpleUploadBanner.js

@@ -92,7 +92,7 @@ const SimpleUploadBanner = ( {
 
   return (
     <View className="py-2">
-      {status.text !== "" && renderUploadStatusText( )}
+      {( status.text !== "" || !!error ) && renderUploadStatusText( )}
       <UploadProgressBar progress={progress} />
     </View>
   );

src/components/MyObservations/SimpleUploadBannerContainer.js

@@ -38,7 +38,6 @@ const SimpleUploadBannerContainer = ( {
   const currentDeleteCount = useStore( state => state.currentDeleteCount );
   const deleteError = useStore( state => state.deleteError );
   const uploadMultiError = useStore( state => state.multiError );
-  const uploadErrorsByUuid = useStore( state => state.errorsByUuid );
   const initialNumObservationsInQueue = useStore( state => state.initialNumObservationsInQueue );
   const totalToolbarProgress = useStore( state => state.totalToolbarProgress );
   const uploadStatus = useStore( state => state.uploadStatus );
@@ -47,6 +46,8 @@ const SimpleUploadBannerContainer = ( {
 
   const stopAllUploads = useStore( state => state.stopAllUploads );
   const numUploadsAttempted = useStore( state => state.numUploadsAttempted );
+  const totalUploadErrors = useStore( state => state.getTotalUploadErrors() );
+  const numUploadedWithoutErrors = useStore( state => state.getNumUploadedWithoutErrors() );
 
   // Note that initialNumObservationsInQueue is the number of obs being uploaded in
   // the current upload session, so it might be 1 if a single obs is
@@ -69,7 +70,6 @@ const SimpleUploadBannerContainer = ( {
   const pendingUpload = uploadStatus === UPLOAD_PENDING && numUploadableObservations > 0;
   const uploadInProgress = uploadStatus === UPLOAD_IN_PROGRESS && numUploadsAttempted > 0;
   const uploadsComplete = uploadStatus === UPLOAD_COMPLETE && initialNumObservationsInQueue > 0;
-  const totalUploadErrors = Object.keys( uploadErrorsByUuid ).length;
 
   const setDeletionsProgress = ( ) => {
     // TODO: we should emit deletions progress like we do for uploads for an accurate progress
@@ -130,8 +130,8 @@ const SimpleUploadBannerContainer = ( {
       return status;
     }
 
-    if ( uploadsComplete ) {
-      status.text = t( "X-observations-uploaded", { count: numUploadsAttempted } );
+    if ( uploadsComplete && numUploadedWithoutErrors > 0 ) {
+      status.text = t( "X-observations-uploaded", { count: numUploadedWithoutErrors } );
       return status;
     }
 
@@ -140,7 +140,7 @@ const SimpleUploadBannerContainer = ( {
     currentDeleteCount,
     deletionsComplete,
     initialNumDeletionsInQueue,
-    numUploadsAttempted,
+    numUploadedWithoutErrors,
     numUploadableObservations,
     pendingUpload,
     manualSyncInProgress,

src/stores/createUploadObservationsSlice.ts

@@ -279,7 +279,9 @@ const createUploadObservationsSlice: StateCreator<UploadObservationsSlice> = ( s
         ? UPLOAD_COMPLETE
         : UPLOAD_IN_PROGRESS
     } );
-  } )
+  } ),
+  getTotalUploadErrors: () => Object.keys( get().errorsByUuid ).length,
+  getNumUploadedWithoutErrors: () => get().numUploadsAttempted - get().getTotalUploadErrors()
 } );
 
 export default createUploadObservationsSlice;

tests/unit/components/MyObservations/SimpleUploadBannerContainer.test.js

@@ -146,6 +146,71 @@ describe( "SimpleUploadBannerContainer", () => {
     expect( statusText ).toBeVisible( );
   } );
 
+  it( "displays 1 upload completed and 4 failed", () => {
+    useStore.setState( {
+      layout: {
+        isDefaultMode: false
+      },
+      numUploadsAttempted: 5,
+      uploadStatus: UPLOAD_COMPLETE,
+      syncingStatus: SYNC_PENDING,
+      initialNumObservationsInQueue: 5,
+      errorsByUuid: {
+        1: true, 2: true, 3: true, 4: true
+      }
+    } );
+    renderComponent( <SimpleUploadBannerContainer currentUser={mockUser} /> );
+
+    const successText = screen.getByText( /1 observation uploaded/ );
+    const errorText = screen.getByText( /4 uploads failed/ );
+    expect( successText ).toBeVisible( );
+    expect( errorText ).toBeVisible( );
+  } );
+
+  it( "displays only error when all 5 uploads failed", () => {
+    useStore.setState( {
+      layout: {
+        isDefaultMode: false
+      },
+      uploadStatus: UPLOAD_COMPLETE,
+      syncingStatus: SYNC_PENDING,
+      initialNumObservationsInQueue: 5,
+      numUploadsAttempted: 5,
+      errorsByUuid: {
+        1: true,
+        2: true,
+        3: true,
+        4: true,
+        5: true
+      }
+    } );
+    renderComponent( <SimpleUploadBannerContainer currentUser={mockUser} /> );
+
+    const errorText = screen.getByText( /5 uploads failed/ );
+    expect( errorText ).toBeVisible();
+    const successText = screen.queryByText( /1 observation uploaded/ );
+    expect( successText ).toBeFalsy();
+  } );
+
+  it( "displays 4 uploads completed and 1 failed", () => {
+    useStore.setState( {
+      layout: {
+        isDefaultMode: false
+      },
+      uploadStatus: UPLOAD_COMPLETE,
+      syncingStatus: SYNC_PENDING,
+      initialNumObservationsInQueue: 5,
+      numUploadsAttempted: 5,
+      errorsByUuid: { 1: true }
+    } );
+    renderComponent( <SimpleUploadBannerContainer currentUser={mockUser} /> );
+
+    const successText = screen.getByText( /4 observations uploaded/ );
+    const errorText = screen.getByText( /1 upload failed/ );
+    expect( successText ).toBeVisible();
+    expect( errorText ).toBeVisible();
+  } );
+
   // 20240611 amanda - removing this test for now, since I believe the new intended UI
   // is that the user will only ever see "Syncing..." followed by
   // "1 observation deleted" UI after deleting a local observation. feel free to reinstate this