Merge pull request #1292 from eikek/fix/update-bookmark

Fix personal/non-personal when updating bookmarks

modules/backend/src/main/scala/docspell/backend/ops/OQueryBookmarks.scala

@@ -6,6 +6,7 @@
 
 package docspell.backend.ops
 
+import cats.data.OptionT
 import cats.effect._
 import cats.implicits._
 
@@ -14,7 +15,7 @@ import docspell.query.ItemQuery
 import docspell.store.AddResult
 import docspell.store.Store
 import docspell.store.UpdateResult
-import docspell.store.records.RQueryBookmark
+import docspell.store.records._
 
 trait OQueryBookmarks[F[_]] {
 
@@ -73,7 +74,14 @@ object OQueryBookmarks {
 
       def update(account: AccountId, id: Ident, b: NewBookmark): F[UpdateResult] =
         UpdateResult.fromUpdate(
-          store.transact(RQueryBookmark.update(convert.toRecord(account, id, b)))
+          store.transact {
+            (for {
+              userId <- OptionT(RUser.findIdByAccount(account))
+              n <- OptionT.liftF(
+                RQueryBookmark.update(convert.toRecord(account, id, userId, b))
+              )
+            } yield n).getOrElse(0)
+          }
         )
 
       def delete(account: AccountId, bookmark: Ident): F[Unit] =
@@ -85,12 +93,17 @@ object OQueryBookmarks {
     def toModel(r: RQueryBookmark): Bookmark =
       Bookmark(r.id, r.name, r.label, r.query, r.isPersonal, r.created)
 
-    def toRecord(account: AccountId, id: Ident, b: NewBookmark): RQueryBookmark =
+    def toRecord(
+        account: AccountId,
+        id: Ident,
+        userId: Ident,
+        b: NewBookmark
+    ): RQueryBookmark =
       RQueryBookmark(
         id,
         b.name,
         b.label,
-        None, // userId and some other values are not used
+        if (b.personal) userId.some else None,
         account.collective,
         b.query,
         Timestamp.Epoch

modules/store/src/main/scala/docspell/store/records/RQueryBookmark.scala

@@ -95,7 +95,8 @@ object RQueryBookmark {
       DML.set(
         T.name.setTo(r.name),
         T.label.setTo(r.label),
-        T.query.setTo(r.query)
+        T.query.setTo(r.query),
+        T.userId.setTo(r.userId)
       )
     )
 
@@ -119,9 +120,12 @@ object RQueryBookmark {
     ).build.query[Int].unique.map(_ > 0)
   }
 
-  // impl note: store.add doesn't work, because it checks for duplicate
-  // after trying to insert the check is necessary because a name
-  // should be unique across personal *and* collective bookmarks
+  // impl note: store.add doesn't work, because it checks for
+  // duplicate after trying to insert. the check is necessary because
+  // a name should be unique across personal *and* collective
+  // bookmarks, which is not covered by db indexes. This is now
+  // checked before (and therefore subject to race conditions, but is
+  // neglected here)
   def insertIfNotExists(
       account: AccountId,
       r: ConnectionIO[RQueryBookmark]