From 09b2a0e79a5b2ef7068899f7d6badb6965de0ad6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Przemys=C5=82aw=20Kaminski?= <pk@intrepidus.pl>
Date: Fri, 8 Sep 2023 11:09:21 +0200
Subject: [PATCH] [ngrams] fixes to ngrams state synchronization

The issue was this:

When user clicks around the ngrams table, only local state is
updated (`ngramsLocalPatch`). After he clicks 'Sychronize', this is
uploaded to server and `ngramsVersion` number is bumped up. What
happened is that `ngramsValidPatch` inside internal state was updated
and so the patch was sychronized locally, without refreshing the
table. However, this resulted in errors later, when user changed the
same term, because then, when user changes view to 'Stop terms', the
table is refreshed with the newly applied terms and `ngramsValidPatch`
doesn't make sense anymore.

The solution is to set `ngramsValidPatch` to `mempty` and, after state
is updated, refresh the current table.

It could be argued whether it's the most optimal thing to do and why
don't we keep things "offline". Well, clicking on 'Sychronize'
requires the client to be online so why not sync the table already at
this time?

I guess this makes things bit simpler and I think it renders
`ngramsValidPatch` unnecessary. So our state could be
simplified (patches is already a complex beast).
---
 src/Gargantext/Components/NgramsTable.purs          | 13 ++++++++++---
 .../Components/NgramsTable/SyncResetButton.purs     |  5 +++--
 src/Gargantext/Core/NgramsTable/Functions.purs      |  6 ++++--
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/src/Gargantext/Components/NgramsTable.purs b/src/Gargantext/Components/NgramsTable.purs
index 4e19006f..524988fb 100644
--- a/src/Gargantext/Components/NgramsTable.purs
+++ b/src/Gargantext/Components/NgramsTable.purs
@@ -824,6 +824,12 @@ mainNgramsTableCpt = here.component "mainNgramsTable" cpt
       -- onNgramsClickRef <- R.useRef Nothing
       -- onSaveRef   <- R.useRef Nothing
       state <- T.useBox initialState
+
+      -- https://gitlab.iscpif.fr/gargantext/purescript-gargantext/issues/594
+      -- Refreshing view when state.ngramsVersion changes.
+      ngramsVersion <- T.useFocused (_.ngramsVersion) (\a b -> b { ngramsVersion = a}) state
+      ngramsVersion' <- T.useLive T.unequal ngramsVersion
+
       -- ngramsLocalPatch <- T.useFocused (_.ngramsLocalPatch) (\a b -> b { ngramsLocalPatch = a }) state
 
       -- nodeId <- T.useFocused (_.nodeId) (\a b -> b { nodeId = a }) path
@@ -841,11 +847,11 @@ mainNgramsTableCpt = here.component "mainNgramsTable" cpt
       case cacheState' of
         NT.CacheOn  -> pure $ R.fragment
           [ loadedNgramsTableHeader { searchQuery, params }
-          , mainNgramsTableCacheOn (Record.merge props { state })
+          , mainNgramsTableCacheOn (Record.merge props { key: show ngramsVersion', state })
           ]
         NT.CacheOff -> pure $ R.fragment
-          [loadedNgramsTableHeader { searchQuery, params}
-          , mainNgramsTableCacheOff (Record.merge props { state })
+          [ loadedNgramsTableHeader { searchQuery, params }
+          , mainNgramsTableCacheOff (Record.merge props { key: show ngramsVersion', state })
           ]
 
 
@@ -1015,6 +1021,7 @@ ngramsTreeEditRealCpt = here.component "ngramsTreeEditReal" cpt where
 
 type MainNgramsTableCacheProps =
   ( state :: T.Box State
+  , key   :: String
   | MainNgramsTableProps )
 
 mainNgramsTableCacheOn :: R2.Leaf MainNgramsTableCacheProps
diff --git a/src/Gargantext/Components/NgramsTable/SyncResetButton.purs b/src/Gargantext/Components/NgramsTable/SyncResetButton.purs
index 57937f2a..27e23993 100644
--- a/src/Gargantext/Components/NgramsTable/SyncResetButton.purs
+++ b/src/Gargantext/Components/NgramsTable/SyncResetButton.purs
@@ -50,11 +50,12 @@ syncResetButtonsCpt = here.component "syncResetButtons" cpt
         synchronizeClick _ = delay unit $ \_ -> do
           T.write_ true synchronizing
           performAction $ Synchronize { afterSync: newAfterSync }
-          performAction ResetPatches
 
         newAfterSync x = do
           afterSync x
-          liftEffect $ T.write_ false synchronizing
+          liftEffect $ do
+            T.write_ false synchronizing
+            performAction ResetPatches
 
       pure $
 
diff --git a/src/Gargantext/Core/NgramsTable/Functions.purs b/src/Gargantext/Core/NgramsTable/Functions.purs
index 009a3783..b4f22188 100644
--- a/src/Gargantext/Core/NgramsTable/Functions.purs
+++ b/src/Gargantext/Core/NgramsTable/Functions.purs
@@ -436,8 +436,9 @@ syncPatches props state callback = do
               s {
                   ngramsLocalPatch = fromNgramsPatches mempty
                 , ngramsStagePatch = fromNgramsPatches mempty
-                , ngramsValidPatch = fromNgramsPatches newPatch <> ngramsLocalPatch <> s.ngramsValidPatch
-                              -- First the already valid patch, then the local patch, then the newly received newPatch.
+                , ngramsValidPatch = fromNgramsPatches mempty
+                -- , ngramsValidPatch = fromNgramsPatches newPatch <> ngramsLocalPatch <> s.ngramsValidPatch
+                -- First the already valid patch, then the local patch, then the newly received newPatch.
                 , ngramsVersion    = newVersion
                 }) state
             here.log2 "[syncPatches] ngramsVersion" newVersion
@@ -525,6 +526,7 @@ coreDispatch _ state (CommitPatch pt) =
 coreDispatch _ state ResetPatches =
   T.modify_ (_ { ngramsLocalPatch = mempty :: NgramsTablePatch
                , ngramsSelection  = mempty :: Set NgramsTerm
+               -- , ngramsValidPatch = mempty :: NgramsTablePatch
                }) state
 
 isSingleNgramsTerm :: NgramsTerm -> Boolean
-- 
2.21.0