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

rework pagination on review related lists #1097

Closed
wants to merge 3 commits into from
Closed

rework pagination on review related lists #1097

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
68e574c
rework pagination on review related lists
CollinBeczak Jan 2, 2024
bc07d27
refactor filters to pass tests
CollinBeczak Jan 2, 2024
7d3f912
simplify filters
CollinBeczak Jan 2, 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
15 changes: 12 additions & 3 deletions app/org/maproulette/framework/controller/TaskReviewController.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -604,15 +604,21 @@ class TaskReviewController @Inject() (
/**
* Returns a list of challenges that have reviews/review requests.
*
* @param reviewTasksType The type of reviews (1: To Be Reviewed, 2: User's reviewed Tasks, 3: All reviewed by users 4: meta review tasks)
* @param tStatus The task statuses to include
* @param excludeOtherReviewers Whether tasks completed by other reviewers should be included
* @param reviewTasksType The type of reviews (1: To Be Reviewed, 2: User's reviewed Tasks, 3: All reviewed by users 4: meta review tasks)
* @param tStatus The task statuses to include
* @param excludeOtherReviewers Whether tasks completed by other reviewers should be included
* @param challengeSearchQuery Query string for filtering challenges
* @param projectSearchQuery Query string for filtering projects
* @param limit Number of items per page
* @param page Page number
* @return JSON challenge list
*/
def listChallenges(
reviewTasksType: Int,
tStatus: String,
excludeOtherReviewers: Boolean = false,
challengeSearchQuery: String = "",
projectSearchQuery: String = "",
limit: Int,
page: Int
): Action[AnyContent] =
Expand All @@ -623,11 +629,14 @@ class TaskReviewController @Inject() (
case _ => None
}

// Filter challenges based on search query
val challenges = this.serviceManager.challengeListing.withReviewList(
reviewTasksType,
user,
taskStatus,
excludeOtherReviewers,
challengeSearchQuery,
projectSearchQuery,
Paging(limit, page)
)

Expand Down
193 changes: 116 additions & 77 deletions app/org/maproulette/framework/service/ChallengeListingService.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,100 +26,139 @@ class ChallengeListingService @Inject() (
def retrieve(parentId: Long): Option[ChallengeListing] =
this.query(Query.simple(List(BaseParameter(Challenge.FIELD_PARENT_ID, parentId)))).headOption

def emptyFilterGroup: FilterGroup = FilterGroup(List.empty)

/**
* Returns a list of challenges that have reviews/review requests.
*
* @param reviewTasksType The type of reviews (1: To Be Reviewed, 2: User's reviewed Tasks, 3: All reviewed by users)
* @param user The user making request (for challenge permission visibility)
* @param taskStatus The task statuses to include
* @param reviewTasksType The type of reviews (1: To Be Reviewed, 2: User's reviewed Tasks, 3: All reviewed by users)
* @param user The user making request (for challenge permission visibility)
* @param taskStatus The task statuses to include
* @param excludeOtherReviewers Whether tasks completed by other reviewers should be included
* @param challengeSearchQuery Search query for filtering challenges
* @param projectSearchQuery Search query for filtering projects
* @param paging Paging information
* @return A list of children listing objects
*/
def withReviewList(
reviewTasksType: Int,
user: User,
taskStatus: Option[List[Int]] = None,
excludeOtherReviewers: Boolean = false,
challengeSearchQuery: String = "",
projectSearchQuery: String = "",
paging: Paging = Paging()
): List[ChallengeListing] = {
val challengeNameParameter =
if (challengeSearchQuery.nonEmpty) {
Some(
BaseParameter(
Challenge.FIELD_NAME,
SQLUtils.search(challengeSearchQuery),
Operator.ILIKE,
table = Some(Challenge.TABLE)
)
)
} else None

val projectNameParameter =
if (projectSearchQuery.nonEmpty) {
Some(
BaseParameter(
Project.FIELD_DISPLAY_NAME,
SQLUtils.search(projectSearchQuery),
Operator.ILIKE,
table = Some(Project.TABLE)
)
)
} else None

val parameters = List(challengeNameParameter, projectNameParameter).flatten
val filterGroup =
if (parameters.nonEmpty) FilterGroup(parameters)
else emptyFilterGroup

val taskReviewFilterGroup = FilterGroup(
List(
// Has a task review
BaseParameter(
TaskReview.FIELD_ID,
"",
Operator.NULL,
negate = true,
table = Some(TaskReview.TABLE)
),
// Exclude unnecessary review status
BaseParameter(
"review_status",
Task.REVIEW_STATUS_UNNECESSARY,
Operator.EQ,
negate = true,
table = Some(TaskReview.TABLE)
),
// Task Status in list if given a list of task statuses
FilterParameter.conditional(
"status",
taskStatus.getOrElse(List.empty),
Operator.IN,
includeOnlyIfTrue = taskStatus.getOrElse(List.empty).nonEmpty,
table = Some("tasks")
),
// review_requested_by != user.id unless a super user
// to be reviewed tasks (review type = 1)
FilterParameter.conditional(
TaskReview.FIELD_REVIEW_REQUESTED_BY,
user.id,
negate = true,
includeOnlyIfTrue = (reviewTasksType == 1) && !permission.isSuperUser(user),
table = Some(TaskReview.TABLE)
),
// reviewed_by == user.id for 'tasks reviewed by me' (review type = 3)
FilterParameter.conditional(
TaskReview.FIELD_REVIEWED_BY,
user.id,
includeOnlyIfTrue = reviewTasksType == 2,
table = Some(TaskReview.TABLE)
),
// reviewed_by == user.id for 'my reviewed tasks' (review type = 2)
FilterParameter.conditional(
TaskReview.FIELD_REVIEW_REQUESTED_BY,
user.id,
includeOnlyIfTrue = reviewTasksType == 3,
table = Some(TaskReview.TABLE)
),
// review status = requested or disputed if reviewTasksType = 1
FilterParameter.conditional(
TaskReview.FIELD_REVIEW_STATUS,
List(Task.REVIEW_STATUS_REQUESTED, Task.REVIEW_STATUS_DISPUTED),
Operator.IN,
includeOnlyIfTrue = reviewTasksType == 1,
table = Some(TaskReview.TABLE)
)
)
)

val excludeOtherReviewersFilterGroup = FilterGroup(
List(
BaseParameter(
TaskReview.FIELD_REVIEWED_BY,
"",
Operator.NULL,
table = Some(TaskReview.TABLE)
),
BaseParameter(TaskReview.FIELD_REVIEWED_BY, user.id, table = Some(TaskReview.TABLE))
),
OR(),
excludeOtherReviewers && reviewTasksType == 1
)

val filter =
Filter(
List(
FilterGroup(
List(
// Has a task review
BaseParameter(
TaskReview.FIELD_ID,
"",
Operator.NULL,
negate = true,
table = Some(TaskReview.TABLE)
),
// Exclude unnecessary review status
BaseParameter(
"review_status",
Task.REVIEW_STATUS_UNNECESSARY,
Operator.EQ,
negate = true,
table = Some(TaskReview.TABLE)
),
// Task Status in list if given a list of task statuses
FilterParameter.conditional(
"status",
taskStatus.getOrElse(List.empty),
Operator.IN,
includeOnlyIfTrue = taskStatus.nonEmpty,
table = Some("tasks")
),
// review_requested_by != user.id unless a super user
// to be reviewed tasks (review type = 1)
FilterParameter.conditional(
TaskReview.FIELD_REVIEW_REQUESTED_BY,
user.id,
negate = true,
includeOnlyIfTrue = (reviewTasksType == 1) && !permission.isSuperUser(user),
table = Some(TaskReview.TABLE)
),
// reviewed_by == user.id for 'tasks reviewed by me' (review type = 3)
FilterParameter.conditional(
TaskReview.FIELD_REVIEWED_BY,
user.id,
includeOnlyIfTrue = reviewTasksType == 2,
table = Some(TaskReview.TABLE)
),
// reviewed_by == user.id for 'my reviewed tasks' (review type = 2)
FilterParameter.conditional(
TaskReview.FIELD_REVIEW_REQUESTED_BY,
user.id,
includeOnlyIfTrue = reviewTasksType == 3,
table = Some(TaskReview.TABLE)
),
// review status = requested or disputed if reviewTasksType = 1
FilterParameter.conditional(
TaskReview.FIELD_REVIEW_STATUS,
List(Task.REVIEW_STATUS_REQUESTED, Task.REVIEW_STATUS_DISPUTED),
Operator.IN,
includeOnlyIfTrue = reviewTasksType == 1,
table = Some(TaskReview.TABLE)
)
)
),
// Check project/challenge visiblity
filterGroup,
taskReviewFilterGroup,
challengeService.challengeVisibilityFilter(user),
// reviewed_by is empty or user.id if excludeOtherReviewers
FilterGroup(
List(
BaseParameter(
TaskReview.FIELD_REVIEWED_BY,
"",
Operator.NULL,
table = Some(TaskReview.TABLE)
),
BaseParameter(TaskReview.FIELD_REVIEWED_BY, user.id, table = Some(TaskReview.TABLE))
),
OR(),
excludeOtherReviewers && reviewTasksType == 1
)
excludeOtherReviewersFilterGroup
)
)
this.query(
Expand Down
8 changes: 7 additions & 1 deletion conf/v2_route/challenge.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -539,14 +539,20 @@ POST /challenges/bulkArchive @org.maproulette.controllers
# - name: excludeOtherReviewers
# in: query
# description: Exclude reviews by completed by other reviewers (default true)
# - name: challengeSearchQuery
# in: query
# description: Filter for challenge names that include this value
# - name: projectSearchQuery
# in: query
# description: Filter for project names that include this value
# - name: limit
# in: query
# description: Limit the number of results returned in the response. Default value is 10.
# - name: page
# in: query
# description: Used in conjunction with the limit parameter to page through X number of responses. Default value is 0, ie. first page.
###
GET /review/challenges @org.maproulette.framework.controller.TaskReviewController.listChallenges(reviewTasksType:Int, tStatus:String ?= "", excludeOtherReviewers:Boolean ?= false, limit:Int ?= 10, page:Int ?= 0)
GET /review/challenges @org.maproulette.framework.controller.TaskReviewController.listChallenges(reviewTasksType:Int, tStatus:String ?= "", excludeOtherReviewers:Boolean ?= false, challengeSearchQuery ?= "", projectSearchQuery ?= "", limit:Int ?= 10, page:Int ?= 0)
###
# tags: [ Challenge ]
# summary: List all the Challenges Tasks.
Expand Down
6 changes: 3 additions & 3 deletions test/org/maproulette/framework/service/ChallengeListingServiceSpec.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ class ChallengeListingServiceSpec(implicit val application: Application) extends
}

"fetch challenges with reviews" taggedAs (ChallengeListingTag) in {
val challenges = this.service.withReviewList(1, User.superUser)
challenges.size mustEqual 1
}
val challenges = this.service.withReviewList(1, User.superUser)
challenges.size mustEqual 1
}
}

override implicit val projectTestName: String = "ChallengeListingSpecProject"
Expand Down
Loading