diff --git a/modules/backend/src/main/scala/docspell/backend/ops/OQueryBookmarks.scala b/modules/backend/src/main/scala/docspell/backend/ops/OQueryBookmarks.scala index 72ec9de8..8b8976ce 100644 --- a/modules/backend/src/main/scala/docspell/backend/ops/OQueryBookmarks.scala +++ b/modules/backend/src/main/scala/docspell/backend/ops/OQueryBookmarks.scala @@ -57,14 +57,11 @@ object OQueryBookmarks { _.map(r => Bookmark(r.id, r.name, r.label, r.query, r.isPersonal, r.created)) ) - def create(account: AccountId, b: NewBookmark): F[AddResult] = - store - .transact(for { - r <- RQueryBookmark.createNew(account, b.name, b.label, b.query, b.personal) - n <- RQueryBookmark.insert(r) - } yield n) - .attempt - .map(AddResult.fromUpdate) + def create(account: AccountId, b: NewBookmark): F[AddResult] = { + val record = + RQueryBookmark.createNew(account, b.name, b.label, b.query, b.personal) + store.transact(RQueryBookmark.insertIfNotExists(account, record)) + } def update(account: AccountId, id: Ident, b: NewBookmark): F[UpdateResult] = UpdateResult.fromUpdate( diff --git a/modules/store/src/main/resources/db/migration/h2/V1.31.0__query_bookmark.sql b/modules/store/src/main/resources/db/migration/h2/V1.31.0__query_bookmark.sql index f90c8e67..c98efc01 100644 --- a/modules/store/src/main/resources/db/migration/h2/V1.31.0__query_bookmark.sql +++ b/modules/store/src/main/resources/db/migration/h2/V1.31.0__query_bookmark.sql @@ -6,7 +6,8 @@ CREATE TABLE "query_bookmark" ( "cid" varchar(254) not null, "query" varchar(2000) not null, "created" timestamp, + "__user_id" varchar(254) not null, foreign key ("user_id") references "user_"("uid") on delete cascade, foreign key ("cid") references "collective"("cid") on delete cascade, - unique("cid", "user_id", "name") + unique("cid", "__user_id", "name") ) diff --git a/modules/store/src/main/resources/db/migration/mariadb/V1.31.0__query_bookmark.sql b/modules/store/src/main/resources/db/migration/mariadb/V1.31.0__query_bookmark.sql index de37bcab..73b83810 100644 --- a/modules/store/src/main/resources/db/migration/mariadb/V1.31.0__query_bookmark.sql +++ b/modules/store/src/main/resources/db/migration/mariadb/V1.31.0__query_bookmark.sql @@ -6,7 +6,8 @@ CREATE TABLE `query_bookmark` ( `cid` varchar(254) not null, `query` varchar(2000) not null, `created` timestamp, + `__user_id` varchar(254) not null, foreign key (`user_id`) references `user_`(`uid`) on delete cascade, foreign key (`cid`) references `collective`(`cid`) on delete cascade, - unique(`cid`, `user_id`, `name`) + unique(`cid`, `__user_id`, `name`) ) diff --git a/modules/store/src/main/resources/db/migration/postgresql/V1.31.0__query_bookmark.sql b/modules/store/src/main/resources/db/migration/postgresql/V1.31.0__query_bookmark.sql index f90c8e67..c98efc01 100644 --- a/modules/store/src/main/resources/db/migration/postgresql/V1.31.0__query_bookmark.sql +++ b/modules/store/src/main/resources/db/migration/postgresql/V1.31.0__query_bookmark.sql @@ -6,7 +6,8 @@ CREATE TABLE "query_bookmark" ( "cid" varchar(254) not null, "query" varchar(2000) not null, "created" timestamp, + "__user_id" varchar(254) not null, foreign key ("user_id") references "user_"("uid") on delete cascade, foreign key ("cid") references "collective"("cid") on delete cascade, - unique("cid", "user_id", "name") + unique("cid", "__user_id", "name") ) diff --git a/modules/store/src/main/scala/docspell/store/records/RQueryBookmark.scala b/modules/store/src/main/scala/docspell/store/records/RQueryBookmark.scala index 5dbf34a5..a89cd49b 100644 --- a/modules/store/src/main/scala/docspell/store/records/RQueryBookmark.scala +++ b/modules/store/src/main/scala/docspell/store/records/RQueryBookmark.scala @@ -7,7 +7,7 @@ package docspell.store.records import cats.data.NonEmptyList -import cats.syntax.option._ +import cats.syntax.all._ import docspell.common._ import docspell.query.ItemQuery @@ -16,6 +16,7 @@ import docspell.store.qb._ import doobie._ import doobie.implicits._ +import docspell.store.AddResult final case class RQueryBookmark( id: Ident, @@ -48,6 +49,8 @@ object RQueryBookmark { val query = Column[ItemQuery]("query", this) val created = Column[Timestamp]("created", this) + val internUserId = Column[String]("__user_id", this) + val all: NonEmptyList[Column[_]] = NonEmptyList.of(id, name, label, userId, cid, query, created) } @@ -76,12 +79,14 @@ object RQueryBookmark { curTime ) - def insert(r: RQueryBookmark): ConnectionIO[Int] = + def insert(r: RQueryBookmark): ConnectionIO[Int] = { + val userIdDummy = r.userId.getOrElse(Ident.unsafe("-")) DML.insert( T, - T.all, - sql"${r.id},${r.name},${r.label},${r.userId},${r.cid},${r.query},${r.created}" + T.all.append(T.internUserId), + sql"${r.id},${r.name},${r.label},${r.userId},${r.cid},${r.query},${r.created},$userIdDummy" ) + } def update(r: RQueryBookmark): ConnectionIO[Int] = DML.update( @@ -97,6 +102,42 @@ object RQueryBookmark { def deleteById(cid: Ident, id: Ident): ConnectionIO[Int] = DML.delete(T, T.id === id && T.cid === cid) + def nameExists(account: AccountId, name: String): ConnectionIO[Boolean] = { + val user = RUser.as("u") + val bm = RQueryBookmark.as("bm") + + val users = Select( + user.uid.s, + from(user), + user.cid === account.collective && user.login === account.user + ) + Select( + select(count(bm.id)), + from(bm), + bm.name === name && bm.cid === account.collective && (bm.userId.isNull || bm.userId + .in(users)) + ).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 + def insertIfNotExists( + account: AccountId, + r: ConnectionIO[RQueryBookmark] + ): ConnectionIO[AddResult] = + for { + bm <- r + res <- + nameExists(account, bm.name).flatMap { + case true => + AddResult + .entityExists(s"A bookmark '${bm.name}' already exists.") + .pure[ConnectionIO] + case false => insert(bm).attempt.map(AddResult.fromUpdate) + } + } yield res + def allForUser(account: AccountId): ConnectionIO[Vector[RQueryBookmark]] = { val user = RUser.as("u") val bm = RQueryBookmark.as("bm") diff --git a/modules/webapp/src/main/elm/Comp/BookmarkQueryForm.elm b/modules/webapp/src/main/elm/Comp/BookmarkQueryForm.elm index e0251cc5..1cf6d8ea 100644 --- a/modules/webapp/src/main/elm/Comp/BookmarkQueryForm.elm +++ b/modules/webapp/src/main/elm/Comp/BookmarkQueryForm.elm @@ -76,9 +76,11 @@ initWith bm = isValid : Model -> Bool isValid model = - Comp.PowerSearchInput.isValid model.queryModel - && model.name - /= Nothing + List.all identity + [ Comp.PowerSearchInput.isValid model.queryModel + , model.name /= Nothing + , not model.nameExists + ] get : Model -> Maybe BookmarkedQuery @@ -116,14 +118,6 @@ update flags msg model = nameCheck1 name = Api.bookmarkNameExists flags name NameExistsResp - nameCheck2 = - case model.name of - Just n -> - Api.bookmarkNameExists flags n NameExistsResp - - Nothing -> - Cmd.none - throttleSub = Throttle.ifNeeded (Time.every 150 (\_ -> UpdateThrottle)) @@ -141,11 +135,7 @@ update flags msg model = ) SetPersonal flag -> - let - ( newThrottle, cmd ) = - Throttle.try nameCheck2 model.nameExistsThrottle - in - ( { model | isPersonal = flag, nameExistsThrottle = newThrottle }, cmd, throttleSub ) + ( { model | isPersonal = flag }, Cmd.none, Sub.none ) QueryMsg lm -> let @@ -218,7 +208,7 @@ view texts model = ] [] , span - [ class S.infoMessagePlain + [ class S.warnMessagePlain , class "font-medium text-sm" , classList [ ( "invisible", not model.nameExists ) ] ] diff --git a/modules/webapp/src/main/elm/Messages/Comp/BookmarkQueryForm.elm b/modules/webapp/src/main/elm/Messages/Comp/BookmarkQueryForm.elm index 34639a5d..e5d3605f 100644 --- a/modules/webapp/src/main/elm/Messages/Comp/BookmarkQueryForm.elm +++ b/modules/webapp/src/main/elm/Messages/Comp/BookmarkQueryForm.elm @@ -33,7 +33,7 @@ gb = , userLocationText = "The bookmarked query is just for you" , collectiveLocation = "Collective scope" , collectiveLocationText = "The bookmarked query can be used and edited by all users" - , nameExistsWarning = "A bookmark with this name exists, it is overwritten on save!" + , nameExistsWarning = "A bookmark with this name exists!" } @@ -45,5 +45,5 @@ de = , userLocationText = "Der Bookmark ist nur für dich" , collectiveLocation = "Kollektiv-Bookmark" , collectiveLocationText = "Der Bookmark kann von allen Benutzer verwendet werden" - , nameExistsWarning = "Der Bookmark existiert bereits. Er wird beim Speichern überschrieben." + , nameExistsWarning = "Der Bookmark existiert bereits!" }