fix(avs): fix #225 by skipping firm updates entirely if AVS FirmInfo is unchanged for previously seen values for AVS User to be updated

Note: noticed while working on #225

CHANGELOG.md

@@ -2,6 +2,43 @@
 
 All notable changes to this project will be documented in this file. See [standard-version](https://github.com/conventional-changelog/standard-version) for commit guidelines.
 
+## [27.4.79](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/compare/v27.4.78...v27.4.79) (2024-09-10)
+
+
+### Bug Fixes
+
+* **notifications:** fix [#180](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/issues/180) qualification expiry notification are sent only once ([74f7633](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/74f7633837870448f7cab1013719f42ab49941fe))
+* **supervision:** fix [#181](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/issues/181) by unifying deletion of supervision ([6a070a6](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/6a070a67756bd4ef4b9b5efc176f34c7ed183f1a))
+
+## [27.4.78](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/compare/v27.4.77...v27.4.78) (2024-09-05)
+
+
+### Bug Fixes
+
+* **avs:** acs auto synch had inverted success/failure ([4f7855b](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/4f7855b9ee7133c5ee7e2ca63d63e5d9f060d62f))
+* **avs:** fix [#124](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/issues/124) avs auto synch filter working ([2a27a1e](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/2a27a1efa673a4245a7e8667bd30c79ac1891b9c))
+* **avs:** fix [#178](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/issues/178) by deleting old superiors for individual users ([ade27e6](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/ade27e647913ffe4432b41d585b3e00d1c68d4a0))
+* **avs:** typo in superior remark, towards [#178](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/issues/178) ([3c5edb1](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/3c5edb1b970c8c154d9957837007815b29e23964))
+* **mail:** fix [#179](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/issues/179) by adding download links for PDF attachments ([620e3e4](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/620e3e470080831826ccc960dd876e7bb4fcea03))
+
+## [27.4.77](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/compare/v27.4.76...v27.4.77) (2024-09-02)
+
+
+### Bug Fixes
+
+* **avs:** attempt LDAP upsert before creating avs users ([cfe2318](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/cfe2318f81c951a7f7310e8bcd9ec25d79417587))
+* **avs:** company superiors are now irregular supervisors and old ones are deleted ([7e5c256](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/7e5c256b4c15a15f7218dd7c1490d5e7add4b1c1))
+* **avs:** fix [#124](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/issues/124) implement automatic avs driving licence synchronisation ([cc5da9a](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/cc5da9a2a9bfc8a29f6fe19260bd6dc5412ad4a1))
+* **avs:** switch company did not always increase priority ([8ec2875](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/8ec2875590718f28c3bab8c10141065e11f1405c))
+* **build:** minor linter fix ([be5e609](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/be5e609b1fe879428784d78fa62a559d0764a85a))
+* **firm:** fix [#174](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/issues/174) by adding address search filter to all company view ([40dadd5](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/40dadd58762156005b5889b93a56ffdc044b4460))
+* **firm:** fix [#175](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/issues/175) by separating superiors in firm tables and selections ([8397c46](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/8397c468a04af42ba3baee2f84a0051adbc74374))
+* **ldap:** no more timeout for ldap synch all button ([f946e99](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/f946e99da3bc37514a4e3621438ac133cdc16732))
+* **linter:** minor bug in exam-correct.hs ([8bc3663](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/8bc3663ee2e4ded19091ebe350de82cd693093fc))
+* **mail:** display html emails no longer distorts page ([b0972bb](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/b0972bb154f453edd545fb4f658d9f5ff79966eb)), closes [#2](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/issues/2)
+* **model:** flip erroneous boolean SQL default for CompanyPostalAddress ([b7e5b8f](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/b7e5b8f111b5115d816d984c6ef2f12edfcef5bb))
+* **user:** fix pagination and count for supervision tables ([9c82558](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/commit/9c82558d71a032dad27e892c489c7004d091e088))
+
 ## [27.4.76](https://gitlab2.rz.ifi.lmu.de/uni2work/uni2work/compare/v27.4.75...v27.4.76) (2024-08-08)
 
 

nix/docker/version.json

@@ -1,3 +1,3 @@
 {
-  "version": "27.4.76"
+  "version": "27.4.79"
 }

package-lock.json

@@ -1,6 +1,6 @@
 {
   "name": "uni2work",
-  "version": "27.4.76",
+  "version": "27.4.79",
   "lockfileVersion": 1,
   "requires": true,
   "dependencies": {

package.json

@@ -1,6 +1,6 @@
 {
   "name": "uni2work",
-  "version": "27.4.76",
+  "version": "27.4.79",
   "description": "",
   "keywords": [],
   "author": "",

package.yaml

@@ -1,5 +1,5 @@
 name: uniworx
-version: 27.4.76
+version: 27.4.79
 dependencies:
   - base
   - yesod

src/Audit.hs

@@ -1,7 +1,9 @@
--- SPDX-FileCopyrightText: 2023 Gregor Kleen <gregor.kleen@ifi.lmu.de>,Steffen Jost <s.jost@fraport.de>
+-- SPDX-FileCopyrightText: 2023-24 Gregor Kleen <gregor.kleen@ifi.lmu.de>,Steffen Jost <s.jost@fraport.de>
 --
 -- SPDX-License-Identifier: AGPL-3.0-or-later
 
+{-# LANGUAGE TypeApplications #-}
+
 module Audit
   ( module Audit.Types
   , AuditException(..)
@@ -17,6 +19,8 @@ import Import.NoModel
 import Settings
 import Model
 import Database.Persist.Sql
+import qualified Database.Esqueleto.Experimental as E -- needs TypeApplications Lang-Pragma
+import qualified Database.Esqueleto.Utils        as E
 import Audit.Types
 
 import qualified Data.Text          as Text
@@ -129,7 +133,7 @@ logInterface :: ( AuthId (HandlerSite m) ~ Key User
       -> Text       -- ^ Any additional information
       -> ReaderT (YesodPersistBackend (HandlerSite m)) m ()
 -- ^ Log a transaction using information available from `HandlerT`, also calls `audit`
-logInterface interfaceLogInterface interfaceLogSubtype interfaceLogSuccess interfaceLogRows interfaceLogInfo = do 
+logInterface interfaceLogInterface interfaceLogSubtype interfaceLogSuccess interfaceLogRows interfaceLogInfo = do
   interfaceLogWrite <- (methodGet /=) . Wai.requestMethod . reqWaiRequest <$> getRequest
   logInterface' interfaceLogInterface interfaceLogSubtype interfaceLogWrite interfaceLogSuccess interfaceLogRows interfaceLogInfo
 
@@ -173,20 +177,25 @@ logInterface' (Text.strip -> interfaceLogInterface) (Text.strip -> interfaceLogS
 
 reportAdminProblem ::  ( IsSqlBackend (YesodPersistBackend (HandlerSite m))
                   , SqlBackendCanWrite (YesodPersistBackend (HandlerSite m))
-                  , MonadHandler m                 
+                  , MonadHandler m
                   -- , HasCallStack
                   )
-      => AdminProblem -- ^ Problem to record 
+      => AdminProblem -- ^ Problem to record
       -> ReaderT (YesodPersistBackend (HandlerSite m)) m ()
--- ^ Log a problem that needs interventions by admins 
+-- ^ Log a problem that needs interventions by admins, provided this problem has not already been reported and is still unsolved
 --
 --   - `problemLogTime` is now
 --   - `problemSolver` is Nothing, we do not record the person who caused it
-reportAdminProblem problem@(toJSON -> problemLogInfo) = do
-  problemLogTime <- liftIO getCurrentTime
+reportAdminProblem problem = do
   let problemLogSolved = Nothing
       problemLogSolver = Nothing
-  insert_ ProblemLog{..}
+      problemLogInfo   = toJSON problem
+  problemLogTime <- liftIO getCurrentTime
+  isKnown <- E.selectExists $ do
+    pl <- E.from $ E.table @ProblemLog
+    E.where_ $ E.isNothing (pl E.^. ProblemLogSolved)
+        E.&&. E.val problemLogInfo E.==. pl E.^. ProblemLogInfo
+  unless isKnown $ insert_ ProblemLog{..}
   $logWarnS "Problem" $ Text.filter (/= '\n') $ tshow problem -- <> " - " <> pack (prettyCallStack callStack)
 
 

src/Handler/Utils/Avs.hs

@@ -329,6 +329,8 @@ updateAvsUserByADC newAvsDataContact@(AvsDataContact apid newAvsPersonInfo newAv
       let usrId = userAvsUser usravs
       usr <- MaybeT $ get usrId
       lift $ do -- maybeT no longer needed from here onwards
+        uuid :: CryptoUUIDUser <- encrypt usrId
+        $logInfoS "AVS" [st|updateAvsUserByADC: #{tshow uuid}|]
         let oldAvsPersonInfo  = userAvsLastPersonInfo usravs   -- Nothing is ok here
             oldAvsFirmInfo    = userAvsLastFirmInfo   usravs   -- Nothing is ok here
             oldAvsCardNo      = userAvsLastCardNo     usravs & fmap Just
@@ -380,72 +382,73 @@ updateAvsUserByADC newAvsDataContact@(AvsDataContact apid newAvsPersonInfo newAv
                       , UserAvsLastCardNo     =. newAvsCardNo
                       ]
 
-        -- update company association & supervision
-        newCompanyEnt@Entity{entityKey=newCompanyId} <- upsertAvsCompany newAvsFirmInfo oldAvsFirmInfo
-        oldCompanyEnt <- getAvsCompany `traverseJoin` oldAvsFirmInfo
-        primaryCompanyId <- userCompanyCompany <<$>> getUserPrimaryCompany usrId
-        let oldCompanyId = entityKey <$> oldCompanyEnt
-            -- oldCompanyMb = entityVal <$> oldCompanyEnt
-            -- pst_up  = if
-            --           -- | isNothing oldCompanyMb || oldCompanyId == primaryCompanyId                                              -- refactor could replace next 4 lines
-            --           --   -> mkUpdate'      usr newCompany oldCompanyMb $ CheckUpdate UserPrefersPostal _companyPrefersPostal  -- always update if company association is fresh (case should not occur in practice though)
-            --           | isNothing oldCompanyMb
-            --             -> mkUpdateDirect usr newCompany              $ CheckUpdate UserPrefersPostal _companyPrefersPostal  -- always update if company association is fresh (case should not occur in practice though)
-            --           | oldCompanyId == primaryCompanyId -- && isJust oldCompanyId -- is ensured by previous line
-            --             -> mkUpdate       usr newCompany oldCompanyMb $ CheckUpdate UserPrefersPostal _companyPrefersPostal  -- possibly change postal preference
-            --           | otherwise
-            --             -> Nothing
-            superReasonComDef = tshow SupervisorReasonCompanyDefault
-            newUserComp = UserCompany usrId newCompanyId False False 1 True Nothing -- default value for new company insertion, if no update can be done
+        usr_up2 <- guardMonoidM (oldAvsFirmInfo /= Just newAvsFirmInfo) $ do
+          -- update company association & supervision
+          newCompanyEnt@Entity{entityKey=newCompanyId} <- upsertAvsCompany newAvsFirmInfo oldAvsFirmInfo
+          upsertCompanySuperior newCompanyEnt newAvsFirmInfo oldAvsFirmInfo usrId   -- ensure firmInfo superior is supervisor for this user
+          oldCompanyEnt <- getAvsCompany `traverseJoin` oldAvsFirmInfo
+          primaryCompanyId <- userCompanyCompany <<$>> getUserPrimaryCompany usrId
+          let oldCompanyId = entityKey <$> oldCompanyEnt
+              -- oldCompanyMb = entityVal <$> oldCompanyEnt
+              -- pst_up  = if
+              --           -- | isNothing oldCompanyMb || oldCompanyId == primaryCompanyId                                              -- refactor could replace next 4 lines
+              --           --   -> mkUpdate'      usr newCompany oldCompanyMb $ CheckUpdate UserPrefersPostal _companyPrefersPostal  -- always update if company association is fresh (case should not occur in practice though)
+              --           | isNothing oldCompanyMb
+              --             -> mkUpdateDirect usr newCompany              $ CheckUpdate UserPrefersPostal _companyPrefersPostal  -- always update if company association is fresh (case should not occur in practice though)
+              --           | oldCompanyId == primaryCompanyId -- && isJust oldCompanyId -- is ensured by previous line
+              --             -> mkUpdate       usr newCompany oldCompanyMb $ CheckUpdate UserPrefersPostal _companyPrefersPostal  -- possibly change postal preference
+              --           | otherwise
+              --             -> Nothing
+              superReasonComDef = tshow SupervisorReasonCompanyDefault
+              newUserComp = UserCompany usrId newCompanyId False False 1 True Nothing -- default value for new company insertion, if no update can be done
 
-        usr_up2 <- case oldAvsFirmInfo of
-          _ | Just newCompanyId == oldCompanyId                                               -- company unchanged entirely
-              -> return mempty                                                                --  => do nothing
-          (Just oafi) |  isJust    (view _avsFirmPostAddressSimple  oafi)
-                      && ((==) `on` view _avsFirmPostAddressSimple) oafi newAvsFirmInfo       -- non-empty company address unchanged OR
-                      || isJust    (view _avsFirmPrimaryEmail       oafi)
-                      && ((==) `on` view _avsFirmPrimaryEmail)      oafi newAvsFirmInfo       -- non-empty company primary email unchanged
-              -> do                                                                           --  => just update user company association, keeping supervision privileges
-                  case oldCompanyId of
-                    Nothing -> void $ insertUnique newUserComp -- it's ok if this already exists
-                    Just ocid -> do
-                      void $ upsertBySafe (UniqueUserCompany usrId ocid) newUserComp (_userCompanyCompany .~ newCompanyId)  -- keep default supervisor settings
-                      void $ updateWhere [ UserSupervisorSupervisor ==. usrId                                               -- update company-related supervisions
-                                         , UserSupervisorCompany ==. Just ocid                                              -- to new company, regardless of
-                                         , UserSupervisorReason ==. Just superReasonComDef]                                 -- user
-                                         [ UserSupervisorCompany =. Just newCompanyId]
-                  return mempty
-          _ | Just newCompanyId == primaryCompanyId                                           -- old primaryCompany is now also AVS-company
-              -> do
-                  whenIsJust oldCompanyId $ \oldCid -> do
-                    deleteBy $ UniqueUserCompany usrId oldCid
-                    deleteWhere $ (UserSupervisorUser ==. usrId):(UserSupervisorCompany ==. oldCompanyId):(UserSupervisorReason ~=. superReasonComDef)
-                  return mempty
-          _                                                                                   -- company changed completely
-              -> do
-                  (pst_up, problems) <- switchAvsUserCompany False False usrId newCompanyId
-                  mapM_ reportAdminProblem problems
-                  -- Following line does not type, hence additional parameter needed
-                  -- return [ u | u@Update{updateField=f} <- pst_up, f /= UserPostAddress ] -- already computed in frm_up above, duplicate update must be prevented (version above accounts for legacy updates)
-                  return pst_up
-                  -- SPECIALISED CODE, PROBABLY DEPRECATED
-                  -- switch user company, keeping old priority
-                  -- (getBy . UniqueUserCompany usrId) `traverseJoin` oldCompanyId >>= \case
-                  --   Nothing ->
-                  --     void $ insertUnique newUserComp
-                  --   Just Entity{entityKey=ucidOld, entityVal=UserCompany{userCompanyCompany, userCompanySupervisor, userCompanySupervisorReroute, userCompanyPriority}} -> do
-                  --     when userCompanySupervisor $ reportAdminProblem $ AdminProblemSupervisorNewCompany usrId userCompanyCompany newCompanyId userCompanySupervisorReroute
-                  --     delete ucidOld
-                  --     void $ insertUnique newUserComp{userCompanyPriority} -- keep priority, if insert succeeds
-                  -- -- adjust supervison
-                  -- let oldCompDefSuperFltr = mconcat [UserSupervisorCompany ~~. oldCompanyId, UserSupervisorReason ~=. superReasonComDef]
-                  -- deleteWhere $          (UserSupervisorSupervisor ==. usrId) : oldCompDefSuperFltr
-                  -- oldAPs <- deleteWhereCount $ (UserSupervisorUser ==. usrId) : oldCompDefSuperFltr
-                  -- addDefaultSupervisors' newCompanyId $ singleton usrId
-                  -- newAPs <- count $ (UserSupervisorUser ==. usrId) : (UserSupervisorCompany ==. Just newCompanyId) : (UserSupervisorReason ~=. superReasonComDef)
-                  -- when (oldAPs > 0 && newAPs <= 0) $ reportAdminProblem $ AdminProblemNewlyUnsupervised usrId oldCompanyId newCompanyId
-                  -- return pst_up
-        upsertCompanySuperior newCompanyEnt newAvsFirmInfo oldAvsFirmInfo [usrId]   -- ensure firmInfo superior is supervisor for this user
+          case oldAvsFirmInfo of
+            _ | Just newCompanyId == oldCompanyId                                               -- company unchanged entirely
+                -> return mempty                                                                --  => do nothing
+            (Just oafi) |  isJust    (view _avsFirmPostAddressSimple  oafi)
+                        && ((==) `on` view _avsFirmPostAddressSimple) oafi newAvsFirmInfo       -- non-empty company address unchanged OR
+                        || isJust    (view _avsFirmPrimaryEmail       oafi)
+                        && ((==) `on` view _avsFirmPrimaryEmail)      oafi newAvsFirmInfo       -- non-empty company primary email unchanged
+                -> do                                                                           --  => just update user company association, keeping supervision privileges
+                    case oldCompanyId of
+                      Nothing -> void $ insertUnique newUserComp -- it's ok if this already exists
+                      Just ocid -> do
+                        void $ upsertBySafe (UniqueUserCompany usrId ocid) newUserComp (_userCompanyCompany .~ newCompanyId)  -- keep default supervisor settings
+                        void $ updateWhere [ UserSupervisorSupervisor ==. usrId                                               -- update company-related supervisions
+                                          , UserSupervisorCompany ==. Just ocid                                              -- to new company, regardless of
+                                          , UserSupervisorReason ==. Just superReasonComDef]                                 -- user
+                                          [ UserSupervisorCompany =. Just newCompanyId]
+                    return mempty
+            _ | Just newCompanyId == primaryCompanyId                                           -- old primaryCompany is now also AVS-company
+                -> do
+                    whenIsJust oldCompanyId $ \oldCid -> do
+                      deleteBy $ UniqueUserCompany usrId oldCid
+                      deleteWhere $ (UserSupervisorUser ==. usrId):(UserSupervisorCompany ==. oldCompanyId):(UserSupervisorReason ~=. superReasonComDef)
+                    return mempty
+            _                                                                                   -- company changed completely
+                -> do
+                    (pst_up, problems) <- switchAvsUserCompany False False usrId newCompanyId
+                    mapM_ reportAdminProblem problems
+                    -- Following line does not type, hence additional parameter needed
+                    -- return [ u | u@Update{updateField=f} <- pst_up, f /= UserPostAddress ] -- already computed in frm_up above, duplicate update must be prevented (version above accounts for legacy updates)
+                    return pst_up
+                    -- SPECIALISED CODE, PROBABLY DEPRECATED
+                    -- switch user company, keeping old priority
+                    -- (getBy . UniqueUserCompany usrId) `traverseJoin` oldCompanyId >>= \case
+                    --   Nothing ->
+                    --     void $ insertUnique newUserComp
+                    --   Just Entity{entityKey=ucidOld, entityVal=UserCompany{userCompanyCompany, userCompanySupervisor, userCompanySupervisorReroute, userCompanyPriority}} -> do
+                    --     when userCompanySupervisor $ reportAdminProblem $ AdminProblemSupervisorNewCompany usrId userCompanyCompany newCompanyId userCompanySupervisorReroute
+                    --     delete ucidOld
+                    --     void $ insertUnique newUserComp{userCompanyPriority} -- keep priority, if insert succeeds
+                    -- -- adjust supervison
+                    -- let oldCompDefSuperFltr = mconcat [UserSupervisorCompany ~~. oldCompanyId, UserSupervisorReason ~=. superReasonComDef]
+                    -- deleteWhere $          (UserSupervisorSupervisor ==. usrId) : oldCompDefSuperFltr
+                    -- oldAPs <- deleteWhereCount $ (UserSupervisorUser ==. usrId) : oldCompDefSuperFltr
+                    -- addDefaultSupervisors' newCompanyId $ singleton usrId
+                    -- newAPs <- count $ (UserSupervisorUser ==. usrId) : (UserSupervisorCompany ==. Just newCompanyId) : (UserSupervisorReason ~=. superReasonComDef)
+                    -- when (oldAPs > 0 && newAPs <= 0) $ reportAdminProblem $ AdminProblemNewlyUnsupervised usrId oldCompanyId newCompanyId
+                    -- return pst_up
         update usrId usr_up2                                         -- update user by company switch first (due to possible conflicts with usr_up2)
         update usrId usr_up1                                         -- update user eventually
         update uaId  avs_ups                                         -- update stored avsinfo for future updates
@@ -585,16 +588,18 @@ getAvsCompany afi =
 
 -- | insert a company from AVS firm info or update an existing one based on previous values
 upsertAvsCompany :: AvsFirmInfo -> Maybe AvsFirmInfo -> DB (Entity Company)
+-- upsertAvsCompany newAvsFirmInfo (Just oldAvsFirmInfo)
+--   | newAvsFirmInfo == oldAvsFirmInfo = maybeM (upsertAvsCompany newAvsFirmInfo Nothing) pure $ getAvsCompany newAvsFirmInfo  -- firmInfo unchanged, shortcircuit; SHORTCIRCUIT no longer needed, checked at call-site due to result not being wrapped in Maybe
 upsertAvsCompany newAvsFirmInfo mbOldAvsFirmInfo = do
     mbFirmEnt <- getAvsCompany newAvsFirmInfo --  primarily by AvsId, then Shorthand, then name
-    $logInfoS "AVS" [st|upsertAvsCompany: old #{tshow mbFirmEnt} new #{tshow newAvsFirmInfo}|]
-    case (mbFirmEnt, mbOldAvsFirmInfo) of
-      (Nothing, _) -> do                                                                                  -- insert new company, neither AvsId nor Shorthand exist in DB
+    $logInfoS "AVS" [st|upsertAvsCompany: old #{tshow mbOldAvsFirmInfo} new #{tshow newAvsFirmInfo} ent-new #{tshow mbFirmEnt}|]
+    case mbFirmEnt of
+      Nothing -> do                                                                                  -- insert new company, neither AvsId nor Shorthand exist in DB
         afn <- if 0 < newAvsFirmInfo ^. _avsFirmFirmNo
                 then return $ newAvsFirmInfo ^. _avsFirmFirmNo
                 else maybe (-1) (pred . companyAvsId . entityVal) <$> selectMaybe [CompanyAvsId <. 0] [Asc CompanyAvsId]
         let upd  = flip updateRecord newAvsFirmInfo
-            dmy = Company                                                                                 -- mostly dummy, values are actually prodcued through firmInfo2company below for consistency
+            dmy = Company                                                                            -- mostly dummy, values are actually produced through firmInfo2company below for consistency
                   { companyName           = newAvsFirmInfo ^. _avsFirmFirm          . from _CI
                   , companyShorthand      = newAvsFirmInfo ^. _avsFirmAbbreviation  . from _CI
                   , companyAvsId          = afn
@@ -606,11 +611,12 @@ upsertAvsCompany newAvsFirmInfo mbOldAvsFirmInfo = do
         $logInfoS "AVS" $ "Insert new company: " <> tshow cmp
         newCmp <- insertEntity cmp
         reportAdminProblem $ AdminProblemNewCompany $ entityKey newCmp
-        $logInfoS "AVS" "Insert new company completed."
         return newCmp
 
-      (Just Entity{entityKey=firmid, entityVal=firm}, oldAvsFirmInfo) -> do                               -- possibly update existing company, if isJust oldAvsFirmInfo and changed occurred
-        let cmp_ups = mapMaybe  (mkUpdate'            firm newAvsFirmInfo oldAvsFirmInfo) firmInfo2company
+      (Just Entity{entityKey=firmid, entityVal=firm}) -> do                                          -- possibly update existing company, if isJust oldAvsFirmInfo and identical AvsFirmNo and changes occurred
+        let oldHasSameFirmNo = Just (newAvsFirmInfo ^. _avsFirmFirmNo) == (mbOldAvsFirmInfo ^? _Just . _avsFirmFirmNo)
+            oldAvsFirmInfo = guardOnM oldHasSameFirmNo mbOldAvsFirmInfo
+            cmp_ups = mapMaybe  (mkUpdate'            firm newAvsFirmInfo oldAvsFirmInfo) firmInfo2company
             key_ups =            mkUpdate'            firm newAvsFirmInfo oldAvsFirmInfo  firmInfo2key
         uniq_ups    <-           mkUpdateCheckUnique' firm newAvsFirmInfo oldAvsFirmInfo  firmInfo2companyNo
         $logInfoS "AVS" [st|Update company #{companyShorthand firm}: #{tshow (length cmp_ups)}, #{tshow (length key_ups)}, #{tshow (length uniq_ups)} for #{tshow oldAvsFirmInfo}|]
@@ -629,7 +635,7 @@ upsertAvsCompany newAvsFirmInfo mbOldAvsFirmInfo = do
                | otherwise -> $logInfoS "AVS" $ "Update company shorthand failed for " <> ciOriginal cmp_key <> " and " <> ciOriginal alt_key
             maybeM (return res_cmp) return $ getBy uniq_cmp
           _otherwise -> return res_cmp
-        $logInfoS "AVS" "Update company completed."
+        $logInfoS "AVS" [st|Update company #{companyShorthand firm} completed.|]
         return res_cmp2
   where
     firmInfo2key =
@@ -645,8 +651,8 @@ upsertAvsCompany newAvsFirmInfo mbOldAvsFirmInfo = do
 
 
 -- | adjust superiors, assumes that CompanyUser exists for all usrs for given company; does not work otherwise
-upsertCompanySuperior :: Entity Company -> AvsFirmInfo -> Maybe AvsFirmInfo -> [UserId] -> DB () -- may return superior (Maybe UserId), but currently not needed
-upsertCompanySuperior Entity{entityKey=cid, entityVal=Company{}} newAfi oldAfi usrs =
+upsertCompanySuperior :: Entity Company -> AvsFirmInfo -> Maybe AvsFirmInfo -> UserId -> DB () -- may return superior (Maybe UserId), but currently not needed
+upsertCompanySuperior Entity{entityKey=cid, entityVal=Company{}} newAfi oldAfi usrId =
   let reasonSuperior = Just $ tshow SupervisorReasonAvsSuperior
       getInsertUid eml = altM (guessUserByEmail $ stripCI eml) (catchAll2log' $ Just . entityKey <$> ldapLookupAndUpsert eml)
       newAvsNo    = newAfi ^.         _avsFirmFirmNo
@@ -655,22 +661,26 @@ upsertCompanySuperior Entity{entityKey=cid, entityVal=Company{}} newAfi oldAfi u
       mbOldEmail  = oldAfi ^? _Just . _avsFirmEMailSuperior . _Just
       getSupId    =  getInsertUid                `traverseJoin` mbSupEmail
       getOldId    = (guessUserByEmail . stripCI) `traverseJoin` mbOldEmail
-      unchangedCompany  = oldAvsNo   == Just newAvsNo
-      changedSuperior   = mbSupEmail /= mbOldEmail     -- beware, both could be Nothing
+      getSupervision :: Maybe UserId -> DB (Maybe (Entity UserSupervisor))
+      getSupervision = traverseJoin (getBy . flip UniqueUserSupervisor usrId)
+      unchangedCompany = oldAvsNo   == Just newAvsNo
+      changedSuperior  = mbSupEmail /= mbOldEmail     -- beware we only have AvsFirmInfo for one user; also both could be Nothing
       --   1. not unchangedCompany: do not delete, but ensure that superior supervision is set, since it could be a just a single user company change
       --   2. unchangedCompany && not changedSuperior: superior must already been set, short-circuit
       --   3. unchangedCompany && changedSuperior: update superior for all users
   in unless (unchangedCompany && not changedSuperior) $ do      -- do nothing if (unchangedCompany && not changedSuperior).
-    mbSupId <- getSupId
+    mbSupId  <- getSupId
+    mbUsrSup <- getSupervision mbSupId
     -- delete old superiors, if any
     when (unchangedCompany && changedSuperior) $
       deleteWhere $ mcons ((UserSupervisorSupervisor !=.) <$> mbSupId)
                           [ UserSupervisorCompany ==. Just cid, UserSupervisorReason ==. reasonSuperior ]
     unless unchangedCompany $
-      deleteWhere [ UserSupervisorReason ==. reasonSuperior, UserSupervisorUser <-. usrs ]
+      deleteWhere [ UserSupervisorReason ==. reasonSuperior, UserSupervisorUser ==. usrId ]
     -- ensure superior supervision
-    case mbSupId of
-      Just supId -> do
+    case (mbSupId, mbUsrSup) of
+      (_          , Just _) -> return () -- supId is already supervisor for uid for any reason
+      (Just supId, Nothing) -> do
         -- ensure association between company and superior at equal-to-top priority
         prio <- getCompanyUserMaxPrio supId
         void $ insertUnique (UserCompany supId cid False False prio True reasonSuperior) -- superior is not a supervisor, do not change existing user company associations
@@ -702,7 +712,7 @@ upsertCompanySuperior Entity{entityKey=cid, entityVal=Company{}} newAfi oldAfi u
         when (unchangedCompany && changedSuperior) $ do
           oldSupId <- getOldId
           reportAdminProblem $ AdminProblemCompanySuperiorChange supId cid oldSupId
-      Nothing ->
+      (Nothing, Nothing) ->
         when (unchangedCompany && changedSuperior) $ do
           oldSupId <- getOldId
           reportAdminProblem $ AdminProblemCompanySuperiorNotFound mbSupEmail cid oldSupId

src/Jobs/Handler/SynchroniseAvs.hs

@@ -122,11 +122,11 @@ dispatchJobSynchroniseAvsQueue = JobHandlerException $ do
       -- E.truncateTable $ AvsSync (error "truncateTable: AvsSyncUser not needed") now Nothing
       -- return jobs
     let (unlinked, linked) = foldl' discernJob mempty jobs
-    $logInfoS "SynchronisAvs" [st|AVS synch performing for #{length linked} AVS linked users and #{length unlinked} unlinked users|]
+    $logInfoS "SynchronisAvs" [st|AVS synch start for #{length linked} AVS linked users and #{length unlinked} unlinked users|]
     void $ updateAvsUserByIds  linked
     void $ linktoAvsUserByUIDs unlinked
     runDB $ deleteWhere [AvsSyncUser <-. (E.unValue . fst3 <$> jobs)]
-    $logInfoS "SynchronisAvs" [st|AVS synch performed for #{length linked} AVS linked users and #{length unlinked} unlinked users|]
+    $logInfoS "SynchronisAvs" [st|AVS synch end for #{length linked} AVS linked users and #{length unlinked} unlinked users|]
     -- we do not reschedule failed synchs here in order to avoid a loop
   where
     discernJob accs (         _ , E.Value (Just api), E.Value True  ) = accs & over _2 (Set.insert api)