fix: groups would restore properly, but be incorrect at the next session

Initially we were assigning the group from the Sidebar each time a tab
is created. The issue is that to avoid creating the tab in Aff (see
onTabCreated method) we're splitting the action of creating a tab and
assigning it a group in two different messages. Now the problem comes
with the asynchronicity of each action:
- We're first creating a tab with no group assigned.
- We're then sending a message to update the group of each tab.
The missing piece in between is that when the Sidebar received the tab
creation event, it didn't have the tab's group assignation and was
assigning a default one. It would then receive a message to switch the
group, but a message to the background was already sent to update the
group of the tab to the initial one. So while the state of the current
session was correct, at the next session the restored tabs would lose
their group.

src/Background.purs

@@ -101,50 +101,53 @@ onTabCreated stateRef tab = do
   -- opening a session on top of an already existing session. If the user
   -- starts creating groups, opening tab, and then restore a session, then it
   -- will probably break.
-  launchAff_ $
-     (getTabValue tid "groupId" :: Aff (Maybe GroupId)) >>= \gid' ->
-       for_ gid' \gid -> retrieveGroups wid >>= \groups' -> do
-          -- First initialize the groups, then assign the tab. Otherwise the
-          -- tab could be assigned to a non existing group.
-          case groups' of 
-               [] -> pure unit
-               groups -> liftEffect $ GS.sendToTabPort tab state $ BgInitializeGroups groups
-
-          liftEffect $ GS.sendToTabPort tab state $ BgAssignTabToGroup tid gid
+  launchAff_ do
+    groups' <- retrieveGroups wid
+    -- First initialize the groups, then assign the tab. Otherwise the
+    -- tab could be assigned to a non existing group.
+    case groups' of 
+        [] -> pure unit
+        groups -> liftEffect do
+           log $ "[bg] groups found for window " <> (show wid)
+           GS.sendToTabPort tab state $ BgInitializeGroups groups
+
+    gid <-(getTabValue tid "groupId" :: Aff (Maybe GroupId))
+    liftEffect $ log $ "[bg] gid maybe found for tab " <> (show tid) <> ": " <> (show gid)
+    liftEffect $ GS.sendToTabPort tab state $ BgAssignTabToGroup tid gid
 
 onTabUpdated :: StateRef -> TabId -> OnUpdated.ChangeInfo -> Tab -> Effect Unit
 onTabUpdated stateRef tid cinfo tab = do
-  log $ "bg: updated tab " <> show tid
+  log $ "[bg] updated tab " <> show tid
   state <- Ref.modify (GS.updateTab tab) stateRef
   GS.sendToTabPort tab state $ BgTabUpdated tid cinfo tab
 
 onTabMoved :: StateRef -> TabId -> OnMoved.MoveInfo -> Effect Unit
 onTabMoved ref tid minfo = do
-  log $ "bg: moved tab " <> show tid
+  log $ "[bg] moved tab " <> show tid
   state <- Ref.modify (GS.moveTab minfo.fromIndex minfo.toIndex minfo.windowId) ref
   GS.sendToWindowPort minfo.windowId state $ BgTabMoved tid minfo.fromIndex minfo.toIndex
 
 onTabActived :: StateRef -> OnActivated.ActiveInfo -> Effect Unit
 onTabActived stateRef (OnActivated.ActiveInfo aInfo) = do
-  log $ "bg: activated tab " <> show aInfo.tabId
+  log $ "[bg] activated tab " <> show aInfo.tabId
   state <- Ref.modify (GS.activateTab aInfo.windowId aInfo.previousTabId aInfo.tabId) stateRef
   GS.sendToWindowPort aInfo.windowId state $ BgTabActivated aInfo.previousTabId aInfo.tabId
 
 onTabDeleted :: StateRef -> TabId -> OnRemoved.RemoveInfo -> Effect Unit
 onTabDeleted stateRef tabId info = do 
-  log $ "bg: deleted tab " <> show tabId
+  log $ "[bg] deleted tab " <> show tabId
   state <- Ref.modify (GS.deleteTab info.windowId tabId) stateRef
   GS.sendToWindowPort info.windowId state $ BgTabDeleted tabId
 
 onTabDetached :: StateRef -> TabId -> OnDetached.DetachInfo -> Effect Unit
 onTabDetached stateRef tabId info = do
-  log $ "bg: detached tab " <> show tabId
+  log $ "[bg] detached tab " <> show tabId
   state <- Ref.modify (GS.detachTab info.oldWindowId tabId) stateRef
   GS.sendToWindowPort info.oldWindowId state $ BgTabDetached tabId
 
 onTabAttached :: StateRef -> TabId -> OnAttached.AttachInfo -> Effect Unit
 onTabAttached stateRef tid info = do
-  log $ "bg: attached tab " <> show tid
+  log $ "[bg] attached tab " <> show tid
   state <- Ref.modify (GS.attachTab info.newWindowId tid info.newPosition) stateRef
   case GS.tabFromWinIdAndTabId info.newWindowId tid state of
      Just newTab -> GS.sendToWindowPort info.newWindowId state $ BgTabAttached newTab
@@ -225,7 +228,7 @@ onNewWindowId port stateRef listenerRef winId = do
           in
               tabs # traverse \tab@(Tab t) -> (getTabValue t.id "groupId" :: Aff (Maybe GroupId)) >>= 
                 case _ of 
-                     Nothing -> setTabValue t.id "groupId" defaultGroup *> pure (TabWithGroup tab defaultGroup)
+                     Nothing -> pure (TabWithGroup tab defaultGroup)
                      Just gid -> pure $ TabWithGroup tab gid
 
 

src/Model/BackgroundEvent.purs

@@ -15,7 +15,23 @@ data BackgroundEvent
   = BgInitialTabList (Array GroupData) (Array TabWithGroup)
   | BgInitializeGroups (Array GroupData)
   | BgTabCreated Tab
-  | BgAssignTabToGroup TabId GroupId
+
+  -- Initially we were assigning the group from the Sidebar each time a tab is created. The issue is
+  -- that to avoid creating the tab in Aff (see onTabCreated method) we're splitting the action of
+  -- creating a tab and assigning it a group in two different messages. Now the problem comes with
+  -- the asynchronicity of each action:
+  -- - We're first creating a tab with no group assigned.
+  -- - We're then sending a message to update the group of each tab.
+  -- The missing piece in between is that when the Sidebar received the tab creation event, it
+  -- didn't have the tab's group assignation and was assigning a default one. It would then receive
+  -- a message to switch the group, but a message to the background was already sent to update the
+  -- group of the tab to the initial one. So while the state of the current session was correct, at
+  -- the next session the restored tabs would lose their group.
+  --
+  -- Hence this solution: 
+  -- | GroupId of Nothing means we ask the Sidebar to assign the group (by using SbChangeTabGroup).
+  -- | Just GroupId means we tell the Sidebar the group of the tab (i.e. we're restoring a tab).
+  | BgAssignTabToGroup TabId (Maybe GroupId)
   | BgTabDeleted TabId
   | BgTabUpdated TabId ChangeInfo Tab
   | BgTabMoved TabId Int Int

src/Sidebar/Components/Bar.purs

@@ -72,7 +72,7 @@ data Query a
   = TabsQuery (Tabs.Query a)
   | InitialTabsWithGroup (Array GroupData) (Array TabWithGroup) a
   | InitializeGroups (Array GroupData) a
-  | AssignTabToGroup TabId GroupId a
+  | AssignTabToGroup TabId (Maybe GroupId) a
   | GroupDeleted GroupId (Maybe TabId) a
 
 initialGroup :: M.Map GroupId Group
@@ -295,7 +295,16 @@ handleQuery = case _ of
 
       pure (Just a)
 
-   AssignTabToGroup tid gid a -> do 
+   -- Given Nothing, we assign the group ourselves (i.e. the tab had no group to start with)
+   AssignTabToGroup tid Nothing a -> do 
+      { tabsToGroup } <- H.get
+      let groupId = M.lookup tid tabsToGroup
+      for_ groupId \gid -> H.raise $ SbChangeTabGroup tid (Just gid)
+      pure (Just a)
+
+   -- Given an existing group for the tab, we modify our state to reflect it. No need to update the
+   -- background since the information already comes for there.
+   AssignTabToGroup tid (Just gid) a -> do 
       oldS <- H.get
 
       for_ (M.lookup tid oldS.tabsToGroup) \prevGid -> do
@@ -309,9 +318,6 @@ handleQuery = case _ of
         tab <- join <$> (H.query _tabs prevGid $ H.request $ Tabs.TabDeleted tid)
 
         let newTabIndex = getGroupPositionOfTab tid gid s.groupTabsPositions
-        
-        liftEffect $ log $ "[sb] new tab index: " <> (show newTabIndex)
-        liftEffect $ unsafeLog s.groupTabsPositions
 
         case Tuple tab newTabIndex of
              Tuple (Just (Tab tab')) (Just newTabIndex') -> 
@@ -339,13 +345,6 @@ handleQuery = case _ of
           in
              s { groups = newGroups, tabsToGroup = M.fromFoldable tabIdGroup, groupTabsPositions = tabIdGroup }
 
-       -- Update the browser state to re-assign correctly all the tabs
-       let 
-           (groupsTupled :: Array (Tuple TabId GroupId)) = M.toUnfoldableUnordered s.tabsToGroup
-           setGroups = groupsTupled <#>
-              (\(Tuple tid gid) -> H.raise $ SbChangeTabGroup tid (Just gid)) 
-       void $ sequence setGroups
-
        -- Initialize each child tabs component with its tabs
        let 
             tabsGroups = tabs <#> \(TabWithGroup tab@(Tab t) _) -> Tuple tab $ fromMaybe s.currentGroup (M.lookup t.id s.tabsToGroup)
@@ -402,7 +401,6 @@ handleTabsQuery = case _ of
          }
 
        void $ tellChild tabGroupId $ Tabs.TabCreated newTab
-       H.raise $ SbChangeTabGroup tab.id (Just tabGroupId)
        pure $ Just a