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.

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

Clean up code for user compact list #7808

Merged
merged 8 commits into from
May 21, 2024
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released
- Non-admin or -manager users can no longer start long-running jobs that create datasets. This includes annotation materialization and AI inferrals. [#7753](https://github.com/scalableminds/webknossos/pull/7753)
- In the time tracking view, all annotations and tasks can be shown for each user by expanding the table. The individual time spans spent with a task or annotating an explorative annotation can be accessed via CSV export. The detail view including a chart for the individual spans has been removed. [#7733](https://github.com/scalableminds/webknossos/pull/7733)
- Slightly refactored the `<FixExpandleTable/>`component to use columns as props. [#7772](https://github.com/scalableminds/webknossos/pull/7772)
- The config value `datastore.localFolderWhitelist` can now be set for each datastore individually. [#7800](https://github.com/scalableminds/webknossos/pull/7800)
fm3 marked this conversation as resolved.
Show resolved Hide resolved

### Fixed
- Fixed a bug where a toast that was reopened had a flickering effect during the reopening animation. [#7793](https://github.com/scalableminds/webknossos/pull/7793)
Expand Down
9 changes: 2 additions & 7 deletions app/controllers/UserController.scala
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,8 @@ class UserController @Inject()(userService: UserService,
isAdmin: Option[Boolean]
): Action[AnyContent] = sil.SecuredAction.async { implicit request =>
for {
(users, userCompactInfos) <- userDAO.findAllCompactWithFilters(isEditable,
isTeamManagerOrAdmin,
isAdmin,
request.identity)
zipped = users.zip(userCompactInfos)
js <- Fox.serialCombined(zipped.sortBy(_._1.lastName.toLowerCase))(u =>
userService.publicWritesCompact(u._1, u._2))
userCompactInfos <- userDAO.findAllCompactWithFilters(isEditable, isTeamManagerOrAdmin, isAdmin, request.identity)
js <- Fox.serialCombined(userCompactInfos.sortBy(_.lastName.toLowerCase))(userService.publicWritesCompact)
} yield Ok(Json.toJson(js))
}

Expand Down
74 changes: 24 additions & 50 deletions app/models/user/User.scala
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import slick.lifted.Rep
import utils.sql.{SQLDAO, SimpleSQLDAO, SqlClient, SqlToken}
import utils.ObjectId

import java.sql.Timestamp
import scala.concurrent.ExecutionContext

object User {
Expand Down Expand Up @@ -63,11 +62,11 @@ case class User(
}

case class UserCompactInfo(
_id: String,
_multiUserId: String,
_id: ObjectId,
_multiUserId: ObjectId,
email: String,
firstname: String,
lastname: String,
firstName: String,
lastName: String,
userConfiguration: String,
isAdmin: Boolean,
isOrganizationOwner: Boolean,
Expand All @@ -78,40 +77,17 @@ case class UserCompactInfo(
teamManagersAsArrayLiteral: String,
experienceValuesAsArrayLiteral: String,
experienceDomainsAsArrayLiteral: String,
lastActivity: Timestamp,
organization_id: String,
organization_name: String,
lastActivity: Instant,
organizationId: ObjectId,
organizationName: String,
novelUserExperienceInfos: String,
selectedTheme: String,
created: Timestamp,
created: Instant,
lastTaskTypeId: Option[String],
isSuperUser: Boolean,
isEmailVerified: Boolean,
isEditable: Boolean
) {
def toUser(implicit ec: ExecutionContext): Fox[User] =
for {
userConfiguration <- Fox.box2Fox(parseAndValidateJson[JsObject](userConfiguration))
} yield {
User(
ObjectId(_id),
ObjectId(_multiUserId),
ObjectId(organization_id),
firstname,
lastname,
Instant.fromSql(lastActivity),
userConfiguration,
LoginInfo(User.default_login_provider_id, _id),
isAdmin,
isOrganizationOwner,
isDatasetManager,
isDeactivated,
isUnlisted = false,
Instant.fromSql(created),
lastTaskTypeId.map(ObjectId(_))
)
}
}
)

class UserDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
extends SQLDAO[User, UsersRow, Users](sqlClient) {
Expand Down Expand Up @@ -222,23 +198,20 @@ class UserDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
"""

// Necessary since a tuple can only have 22 elements
implicit def GetResultUserCompactInfo(implicit e0: GetResult[String],
e1: GetResult[java.sql.Timestamp],
e2: GetResult[Boolean],
e3: GetResult[Option[String]]): GetResult[UserCompactInfo] = GetResult { prs =>
fm3 marked this conversation as resolved.
Show resolved Hide resolved
implicit def GetResultUserCompactInfo: GetResult[UserCompactInfo] = GetResult { prs =>
import prs._
// format: off
UserCompactInfo(<<[String],<<[String],<<[String],<<[String],<<[String],<<[String],<<[Boolean],<<[Boolean],
<<[Boolean],<<[Boolean],<<[String],<<[String],<<[String],<<[String], <<[String],<<[java.sql.Timestamp],<<[String],
<<[String],<<[String],<<[String],<<[java.sql.Timestamp],<<?[String],<<[Boolean],<<[Boolean],<<[Boolean]
UserCompactInfo(<<[ObjectId],<<[ObjectId],<<[String],<<[String],<<[String],<<[String],<<[Boolean],<<[Boolean],
<<[Boolean],<<[Boolean],<<[String],<<[String],<<[String],<<[String], <<[String],<<[Instant],<<[ObjectId],
<<[String],<<[String],<<[String],<<[Instant],<<?[String],<<[Boolean],<<[Boolean],<<[Boolean]
)
// format: on
}
def findAllCompactWithFilters(
isEditable: Option[Boolean],
isTeamManagerOrAdmin: Option[Boolean],
isAdmin: Option[Boolean],
requestingUser: User)(implicit ctx: DBAccessContext): Fox[(List[User], List[UserCompactInfo])] =

def findAllCompactWithFilters(isEditable: Option[Boolean],
isTeamManagerOrAdmin: Option[Boolean],
isAdmin: Option[Boolean],
requestingUser: User)(implicit ctx: DBAccessContext): Fox[List[UserCompactInfo]] =
for {
selectionPredicates <- buildSelectionPredicates(isEditable,
isTeamManagerOrAdmin,
Expand All @@ -257,7 +230,7 @@ class UserDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
OR COUNT(autr.team_ids) = 0
AS iseditable
"""
r <- run(q"""
rows <- run(q"""
WITH
-- agg_experiences and agg_user_team_roles are extracted to avoid left-join fanout.
agg_experiences AS (
Expand Down Expand Up @@ -318,9 +291,7 @@ class UserDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)
m._id, m.email, m.novelUserExperienceinfos, m.selectedTheme, m.isSuperUser, m.isEmailVerified,
autr.team_ids, autr.team_names, autr.team_managers, aux.experience_values, aux.experience_domains
""".as[UserCompactInfo])
users <- Fox.combined(r.toList.map(_.toUser))
compactInfos = r.toList
} yield (users, compactInfos)
} yield rows.toList

// NOTE: This will not return admins. They have “access to all teams”. Consider fetching those too when you use this
def findAllByTeams(teamIds: List[ObjectId])(implicit ctx: DBAccessContext): Fox[List[User]] =
Expand Down Expand Up @@ -471,7 +442,10 @@ class UserDAO @Inject()(sqlClient: SqlClient)(implicit ec: ExecutionContext)

def updateLastActivity(userId: ObjectId, lastActivity: Instant = Instant.now)(
implicit ctx: DBAccessContext): Fox[Unit] =
updateTimestampCol(userId, _.lastactivity, lastActivity)
for {
_ <- assertUpdateAccess(userId)
_ <- run(q"UPDATE webknossos.users SET lastActivity = $lastActivity WHERE _id = $userId".asUpdate)
} yield ()

def updateUserConfiguration(userId: ObjectId, userConfiguration: JsObject)(implicit ctx: DBAccessContext): Fox[Unit] =
for {
Expand Down
30 changes: 12 additions & 18 deletions app/models/user/UserService.scala
Original file line number Diff line number Diff line change
Expand Up @@ -73,12 +73,6 @@ class UserService @Inject()(conf: WkConf,
case None => userDAO.findFirstByMultiUser(multiUser._id)
}

def findOneByEmailAndOrganization(email: String, organizationId: ObjectId)(implicit ctx: DBAccessContext): Fox[User] =
for {
multiUser <- multiUserDAO.findOneByEmail(email)
user <- userDAO.findOneByOrgaAndMultiUser(organizationId, multiUser._id)
} yield user

def assertNotInOrgaYet(multiUserId: ObjectId, organizationId: ObjectId): Fox[Unit] =
for {
userBox <- userDAO.findOneByOrgaAndMultiUser(organizationId, multiUserId)(GlobalAccessContext).futureBox
Expand Down Expand Up @@ -371,7 +365,7 @@ class UserService @Inject()(conf: WkConf,
}
}

def publicWritesCompact(user: User, userCompactInfo: UserCompactInfo): Fox[JsObject] =
def publicWritesCompact(userCompactInfo: UserCompactInfo): Fox[JsObject] =
for {
_ <- Fox.successful(())
teamsJson = parseArrayLiteral(userCompactInfo.teamIdsAsArrayLiteral).indices.map(
Expand All @@ -390,24 +384,24 @@ class UserService @Inject()(conf: WkConf,
novelUserExperienceInfos <- Json.parse(userCompactInfo.novelUserExperienceInfos).validate[JsObject]
} yield {
Json.obj(
"id" -> user._id.toString,
"id" -> userCompactInfo._id,
"email" -> userCompactInfo.email,
"firstName" -> user.firstName,
"lastName" -> user.lastName,
"isAdmin" -> user.isAdmin,
"isOrganizationOwner" -> user.isOrganizationOwner,
"isDatasetManager" -> user.isDatasetManager,
"isActive" -> !user.isDeactivated,
"firstName" -> userCompactInfo.firstName,
"lastName" -> userCompactInfo.lastName,
"isAdmin" -> userCompactInfo.isAdmin,
"isOrganizationOwner" -> userCompactInfo.isOrganizationOwner,
"isDatasetManager" -> userCompactInfo.isDatasetManager,
"isActive" -> !userCompactInfo.isDeactivated,
"teams" -> teamsJson,
"experiences" -> experienceJson,
"lastActivity" -> user.lastActivity,
"lastActivity" -> userCompactInfo.lastActivity,
"isAnonymous" -> false,
"isEditable" -> userCompactInfo.isEditable,
"organization" -> userCompactInfo.organization_name,
"organization" -> userCompactInfo.organizationName,
"novelUserExperienceInfos" -> novelUserExperienceInfos,
"selectedTheme" -> userCompactInfo.selectedTheme,
"created" -> user.created,
"lastTaskTypeId" -> user.lastTaskTypeId.map(_.toString),
"created" -> userCompactInfo.created,
"lastTaskTypeId" -> userCompactInfo.lastTaskTypeId,
fm3 marked this conversation as resolved.
Show resolved Hide resolved
"isSuperUser" -> userCompactInfo.isSuperUser,
"isEmailVerified" -> userCompactInfo.isEmailVerified,
)
Expand Down
9 changes: 0 additions & 9 deletions app/utils/sql/SQLDAO.scala
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,4 @@ abstract class SQLDAO[C, R, X <: AbstractTable[R]] @Inject()(sqlClient: SqlClien
} yield ()
}

protected def updateTimestampCol(id: ObjectId, column: X => Rep[java.sql.Timestamp], newValue: Instant)(
implicit ctx: DBAccessContext): Fox[Unit] = {
val query = for { row <- collection if notdel(row) && idColumn(row) === id.id } yield column(row)
for {
_ <- assertUpdateAccess(id)
_ <- run(query.update(newValue.toSql))
} yield ()
}

}