Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix listing other users’ annotations #7563

Merged
merged 2 commits into from
Jan 17, 2024
Merged

Fix listing other users’ annotations #7563

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
051af32
Fix listing other users’ annotations
fm3 Jan 17, 2024
894f1a5
changelog
fm3 Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.unreleased.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Fixed that last dimension value in ND dataset was not loaded. [#7535](https://github.com/scalableminds/webknossos/pull/7535)
- Fixed the initialization of the mapping list for agglomerate views if json mappings are present. [#7537](https://github.com/scalableminds/webknossos/pull/7537)
- Fixed a bug where uploading ND volume annotations would lead to errors due to parsing of the chunk paths. [#7547](https://github.com/scalableminds/webknossos/pull/7547)
- Fixed a bug where listing the annotations of other users would result in empty lists even if there are annotations and you should be allowed to see them. [#7563](https://github.com/scalableminds/webknossos/pull/7563)

### Removed
- Removed several unused frontend libraries. [#7521](https://github.com/scalableminds/webknossos/pull/7521)
Expand Down
4 changes: 3 additions & 1 deletion app/controllers/AnnotationController.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -550,9 +550,11 @@ class AnnotationController @Inject()(
annotationInfos <- annotationDAO.findAllListableExplorationals(
isFinished,
None,
isForOwnDashboard = true,
AnnotationType.Explorational,
limit.getOrElse(annotationService.DefaultAnnotationListLimit),
pageNumber.getOrElse(0))
pageNumber.getOrElse(0)
)
annotationCount <- Fox.runIf(includeTotalCount.getOrElse(false))(
annotationDAO.countAllListableExplorationals(isFinished)) ?~> "annotation.countReadable.failed"
annotationInfosJsons = annotationInfos.map(annotationService.writeCompactInfo)
Expand Down
5 changes: 4 additions & 1 deletion app/controllers/UserController.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ class UserController @Inject()(userService: UserService,
annotations <- annotationDAO.findAllListableExplorationals(
isFinished,
Some(request.identity._id),
isForOwnDashboard = true,
AnnotationType.Explorational,
limit.getOrElse(annotationService.DefaultAnnotationListLimit),
pageNumber.getOrElse(0)
Expand Down Expand Up @@ -172,9 +173,11 @@ class UserController @Inject()(userService: UserService,
annotations <- annotationDAO.findAllListableExplorationals(
isFinished,
Some(userIdValidated),
isForOwnDashboard = false,
AnnotationType.Explorational,
limit.getOrElse(annotationService.DefaultAnnotationListLimit),
pageNumber.getOrElse(0))
pageNumber.getOrElse(0)
)
annotationCount <- Fox.runIf(includeTotalCount.getOrElse(false))(
annotationDAO.countAllFor(userIdValidated, isFinished, AnnotationType.Explorational))
jsonList = annotations.map(annotationService.writeCompactInfo)
Expand Down
20 changes: 13 additions & 7 deletions app/models/annotation/Annotation.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -248,14 +248,17 @@ class AnnotationDAO @Inject()(sqlClient: SqlClient, annotationLayerDAO: Annotati
accessQueryFromAccessQWithPrefix(listAccessQ, q"")(ctx)

override protected def readAccessQ(requestingUserId: ObjectId): SqlToken =
readAccessQWithPrefix(requestingUserId, q"")

protected def readAccessQWithPrefix(requestingUserId: ObjectId, prefix: SqlToken): SqlToken =
q"""(
visibility = ${AnnotationVisibility.Public}
or (visibility = ${AnnotationVisibility.Internal}
and (select _organization from webknossos.teams where webknossos.teams._id = _team)
${prefix}visibility = ${AnnotationVisibility.Public}
or (${prefix}visibility = ${AnnotationVisibility.Internal}
and (select _organization from webknossos.teams where webknossos.teams._id = ${prefix}_team)
in (select _organization from webknossos.users_ where _id = $requestingUserId))
or _team in (select _team from webknossos.user_team_roles where _user = $requestingUserId and isTeamManager)
or _user = $requestingUserId
or (select _organization from webknossos.teams where webknossos.teams._id = _team)
or ${prefix}_team in (select _team from webknossos.user_team_roles where _user = $requestingUserId and isTeamManager)
or ${prefix}_user = $requestingUserId
or (select _organization from webknossos.teams where webknossos.teams._id = ${prefix}_team)
in (select _organization from webknossos.users_ where _id = $requestingUserId and isAdmin)
)"""

Expand Down Expand Up @@ -304,11 +307,14 @@ class AnnotationDAO @Inject()(sqlClient: SqlClient, annotationLayerDAO: Annotati
def findAllListableExplorationals(
isFinished: Option[Boolean],
forUser: Option[ObjectId],
// In dashboard, list only own + explicitly shared annotations. When listing those of another user, list all of their annotations the viewer is allowed to see
isForOwnDashboard: Boolean,
Comment on lines +310 to +311
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// In dashboard, list only own + explicitly shared annotations. When listing those of another user, list all of their annotations the viewer is allowed to see
isForOwnDashboard: Boolean,
// In dashboard, list only own + explicitly shared annotations. When listing those of another user, list all of their annotations the viewer is allowed to see
isForOwnDashboard: Boolean,

I find the naming here to be too closely linked with the intent and not with what actually happens. Also surprising behavior when the function is called findAllListable and the parameter gives no hint on that the result may change. Maybe more like filterOwnedAndExplicitlyShared

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, wrote #7565 to track that

typ: AnnotationType,
limit: Int,
pageNumber: Int = 0)(implicit ctx: DBAccessContext): Fox[List[AnnotationCompactInfo]] =
for {
accessQuery <- accessQueryFromAccessQWithPrefix(listAccessQ, q"a.")
accessQuery <- if (isForOwnDashboard) accessQueryFromAccessQWithPrefix(listAccessQ, q"a.")
else accessQueryFromAccessQWithPrefix(readAccessQWithPrefix, q"a.")
stateQuery = getStateQuery(isFinished)
userQuery = forUser.map(u => q"a._user = $u").getOrElse(q"true")
typQuery = q"a.typ = $typ"
Expand Down
9 changes: 0 additions & 9 deletions frontend/javascripts/admin/admin_rest_api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -553,15 +553,6 @@ export function deletePrivateLink(linkId: string): Promise<{
}

// ### Annotations
export function getAnnotationInfos(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I assuming correctly, that this route does not exist anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The backend still provides this route for old clients, but the frontend doesn’t use it anymore (was replaced by getReadableAnnotations)

isFinished: boolean,
pageNumber: number = 0,
): Promise<Array<APIAnnotationInfo>> {
return Request.receiveJSON(
`/api/user/annotations?isFinished=${isFinished.toString()}&pageNumber=${pageNumber}`,
);
}

export function getCompactAnnotationsForUser(
userId: string,
isFinished: boolean,
Expand Down