Skip to content

Commit

Permalink
add global column to challenges and implement filtering for the column (
Browse files Browse the repository at this point in the history
#1136)

* add global column to challenges and implement filtering for the column

* fix descriptions and add default to column

* ressolve test error

* move isGlobal update to scheduled action

* scalafmt

* fix build errors

* add sql back
  • Loading branch information
CollinBeczak authored Jan 2, 2025
1 parent 7801e6f commit 51c893c
Show file tree
Hide file tree
Showing 13 changed files with 92 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,7 @@ class ChallengeController @Inject() (
jsonBody = Utils.insertIntoJson(jsonBody, "owner", user.osmProfile.id, true)(LongWrites)
jsonBody = Utils.insertIntoJson(jsonBody, "enabled", true)(BooleanWrites)
jsonBody = Utils.insertIntoJson(jsonBody, "deleted", false)(BooleanWrites)
jsonBody = Utils.insertIntoJson(jsonBody, "isGlobal", false)(BooleanWrites)
jsonBody =
Utils.insertIntoJson(jsonBody, "challengeType", Actions.ITEM_TYPE_CHALLENGE)(IntWrites)
jsonBody = Utils.insertIntoJson(jsonBody, "difficulty", Challenge.DIFFICULTY_NORMAL)(IntWrites)
Expand Down
24 changes: 24 additions & 0 deletions app/org/maproulette/framework/mixins/SearchParametersMixin.scala
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ trait SearchParametersMixin {
var filterList = List(
this.filterLocation(params),
this.filterBounding(params),
this.filterChallengeGlobal(params),
this.filterTaskStatus(params),
this.filterTaskId(params),
this.filterTaskFeatureId(params),
Expand Down Expand Up @@ -717,6 +718,29 @@ trait SearchParametersMixin {
}
}

/**
* Filters by c.is_global. Will only include if
* challengeParams.filterGlobal value is true
*/
def filterChallengeGlobal(params: SearchParameters): FilterGroup = {
params.challengeParams.filterGlobal match {
case Some(v) if v =>
FilterGroup(
List(
FilterParameter.conditional(
"is_global",
value = "false",
Operator.EQ,
useValueDirectly = true,
includeOnlyIfTrue = true,
table = Some("c")
)
)
)
case _ => FilterGroup(List())
}
}

/**
* Filters by c.requires_local
*/
Expand Down
2 changes: 2 additions & 0 deletions app/org/maproulette/framework/model/Challenge.scala
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,7 @@ case class Challenge(
override val modified: DateTime,
override val description: Option[String] = None,
deleted: Boolean = false,
isGlobal: Boolean = false,
infoLink: Option[String] = None,
general: ChallengeGeneral,
creation: ChallengeCreation,
Expand Down Expand Up @@ -329,6 +330,7 @@ object Challenge extends CommonField {
DateTime.now(),
None,
false,
false,
None,
ChallengeGeneral(-1, -1, ""),
ChallengeCreation(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ object ChallengeRepository {
get[Option[String]]("locationJSON") ~
get[Option[String]]("boundingJSON") ~
get[Boolean]("deleted") ~
get[Boolean]("is_global") ~
get[Option[List[Long]]]("virtual_parent_ids") ~
get[Boolean]("challenges.is_archived") ~
get[Int]("challenges.review_setting") ~
Expand All @@ -273,7 +274,7 @@ object ChallengeRepository {
mediumPriorityRule ~ lowPriorityRule ~ defaultZoom ~ minZoom ~ maxZoom ~ defaultBasemap ~ defaultBasemapId ~
customBasemap ~ updateTasks ~ exportableProperties ~ osmIdProperty ~ taskBundleIdProperty ~ preferredTags ~ preferredReviewTags ~
limitTags ~ limitReviewTags ~ taskStyles ~ lastTaskRefresh ~ dataOriginDate ~ requiresLocal ~ location ~ bounding ~
deleted ~ virtualParents ~ isArchived ~ reviewSetting ~ datasetUrl ~ taskWidgetLayout ~ systemArchivedAt =>
deleted ~ isGlobal ~ virtualParents ~ isArchived ~ reviewSetting ~ datasetUrl ~ taskWidgetLayout ~ systemArchivedAt =>
val hpr = highPriorityRule match {
case Some(c) if StringUtils.isEmpty(c) || StringUtils.equals(c, "{}") => None
case r => r
Expand All @@ -294,6 +295,7 @@ object ChallengeRepository {
modified,
description,
deleted,
isGlobal,
infoLink,
ChallengeGeneral(
ownerId,
Expand Down
13 changes: 13 additions & 0 deletions app/org/maproulette/jobs/SchedulerActor.scala
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,19 @@ class SchedulerActor @Inject() (
last_updated = NOW()
WHERE id = ${id};"""
SQL(query).executeUpdate()

// Update is_global based on bounding box
val updateGlobalQuery =
s"""UPDATE challenges
SET is_global = (
CASE
WHEN (ST_XMax(bounding)::numeric - ST_XMin(bounding)::numeric) > 180 THEN TRUE
WHEN (ST_YMax(bounding)::numeric - ST_YMin(bounding)::numeric) > 90 THEN TRUE
ELSE FALSE
END
);"""

SQL(updateGlobalQuery).executeUpdate()
}
// The above query will not update the cache, so remove the id from the cache in case it is there
logger.debug(s"Flushing challenge cache of challenge with id $id")
Expand Down
19 changes: 14 additions & 5 deletions app/org/maproulette/models/dal/ChallengeDAL.scala
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ class ChallengeDAL @Inject() (
get[Option[String]]("boundingJSON") ~
get[Boolean]("challenges.requires_local") ~
get[Boolean]("deleted") ~
get[Boolean]("is_global") ~
get[Boolean]("challenges.is_archived") ~
get[Int]("challenges.review_setting") ~
get[Option[String]]("challenges.dataset_url") ~
Expand All @@ -133,7 +134,7 @@ class ChallengeDAL @Inject() (
minZoom ~ maxZoom ~ defaultBasemap ~ defaultBasemapId ~ customBasemap ~ updateTasks ~
exportableProperties ~ osmIdProperty ~ taskBundleIdProperty ~ preferredTags ~ preferredReviewTags ~
limitTags ~ limitReviewTags ~ taskStyles ~ lastTaskRefresh ~ dataOriginDate ~ location ~ bounding ~
requiresLocal ~ deleted ~ isArchived ~ reviewSetting ~ datasetUrl ~ taskWidgetLayout ~ completionPercentage ~ tasksRemaining =>
requiresLocal ~ deleted ~ isGlobal ~ isArchived ~ reviewSetting ~ datasetUrl ~ taskWidgetLayout ~ completionPercentage ~ tasksRemaining =>
val hpr = highPriorityRule match {
case Some(c) if StringUtils.isEmpty(c) || StringUtils.equals(c, "{}") => None
case r => r
Expand All @@ -154,6 +155,7 @@ class ChallengeDAL @Inject() (
modified,
description,
deleted,
isGlobal,
infoLink,
ChallengeGeneral(
ownerId,
Expand Down Expand Up @@ -257,6 +259,7 @@ class ChallengeDAL @Inject() (
get[Option[String]]("boundingJSON") ~
get[Boolean]("challenges.requires_local") ~
get[Boolean]("deleted") ~
get[Boolean]("is_global") ~
get[Option[List[Long]]]("virtual_parent_ids") ~
get[Option[List[String]]]("presets") ~
get[Boolean]("challenges.is_archived") ~
Expand All @@ -273,7 +276,7 @@ class ChallengeDAL @Inject() (
lowPriorityRule ~ defaultZoom ~ minZoom ~ maxZoom ~ defaultBasemap ~ defaultBasemapId ~
customBasemap ~ updateTasks ~ exportableProperties ~ osmIdProperty ~ taskBundleIdProperty ~ preferredTags ~
preferredReviewTags ~ limitTags ~ limitReviewTags ~ taskStyles ~ lastTaskRefresh ~
dataOriginDate ~ location ~ bounding ~ requiresLocal ~ deleted ~ virtualParents ~
dataOriginDate ~ location ~ bounding ~ requiresLocal ~ deleted ~ isGlobal ~ virtualParents ~
presets ~ isArchived ~ reviewSetting ~ datasetUrl ~ taskWidgetLayout ~ systemArchivedAt ~ completionPercentage ~ tasksRemaining =>
val hpr = highPriorityRule match {
case Some(c) if StringUtils.isEmpty(c) || StringUtils.equals(c, "{}") => None
Expand All @@ -295,6 +298,7 @@ class ChallengeDAL @Inject() (
modified,
description,
deleted,
isGlobal,
infoLink,
ChallengeGeneral(
ownerId,
Expand Down Expand Up @@ -479,15 +483,15 @@ class ChallengeDAL @Inject() (
this.cacheManager.withOptionCaching { () =>
val insertedChallenge =
this.withMRTransaction { implicit c =>
SQL"""INSERT INTO challenges (name, owner_id, parent_id, difficulty, description, info_link, blurb,
SQL"""INSERT INTO challenges (name, owner_id, parent_id, difficulty, description, is_global, info_link, blurb,
instruction, enabled, featured, checkin_comment, checkin_source,
overpass_ql, remote_geo_json, overpass_target_type, status, status_message, default_priority, high_priority_rule,
medium_priority_rule, low_priority_rule, default_zoom, min_zoom, max_zoom,
default_basemap, default_basemap_id, custom_basemap, updatetasks, exportable_properties,
osm_id_property, task_bundle_id_property, last_task_refresh, data_origin_date, preferred_tags, preferred_review_tags,
limit_tags, limit_review_tags, task_styles, requires_local, is_archived, review_setting, dataset_url, task_widget_layout)
VALUES (${challenge.name}, ${challenge.general.owner}, ${challenge.general.parent}, ${challenge.general.difficulty},
${challenge.description}, ${challenge.infoLink}, ${challenge.general.blurb}, ${challenge.general.instruction},
${challenge.description}, ${challenge.isGlobal}, ${challenge.infoLink}, ${challenge.general.blurb}, ${challenge.general.instruction},
${challenge.general.enabled}, ${challenge.general.featured},
${challenge.general.checkinComment}, ${challenge.general.checkinSource}, ${challenge.creation.overpassQL}, ${challenge.creation.remoteGeoJson},
${challenge.creation.overpassTargetType}, ${challenge.status},
Expand Down Expand Up @@ -584,6 +588,7 @@ class ChallengeDAL @Inject() (
val parentId = (updates \ "parentId").asOpt[Long].getOrElse(cachedItem.general.parent)
val difficulty =
(updates \ "difficulty").asOpt[Int].getOrElse(cachedItem.general.difficulty)
val isGlobal = (updates \ "isGlobal").asOpt[Boolean].getOrElse(cachedItem.isGlobal)
val description =
(updates \ "description").asOpt[String].getOrElse(cachedItem.description.getOrElse(""))
val infoLink =
Expand Down Expand Up @@ -700,7 +705,7 @@ class ChallengeDAL @Inject() (
.getOrElse(cachedItem.extra.presets.getOrElse(null))

val updatedChallenge =
SQL"""UPDATE challenges SET name = $name, owner_id = $ownerId, parent_id = $parentId, difficulty = $difficulty,
SQL"""UPDATE challenges SET name = $name, owner_id = $ownerId, parent_id = $parentId, difficulty = $difficulty, is_global = $isGlobal,
description = $description, info_link = $infoLink, blurb = $blurb, instruction = $instruction,
enabled = $enabled, featured = $featured, checkin_comment = $checkinComment, checkin_source = $checkinSource, overpass_ql = $overpassQL,
remote_geo_json = $remoteGeoJson, overpass_target_type = $overpassTargetType, status = $status, status_message = $statusMessage, default_priority = $defaultPriority,
Expand Down Expand Up @@ -1851,6 +1856,10 @@ class ChallengeDAL @Inject() (
this.appendInWhereClause(whereClause, s"c.is_archived = false")
}

if (searchParameters.challengeParams.filterGlobal == true) {
this.appendInWhereClause(whereClause, s"c.is_global = false")
}

searchParameters.challengeParams.requiresLocal match {
case Some(SearchParameters.CHALLENGE_REQUIRES_LOCAL_EXCLUDE) =>
this.appendInWhereClause(whereClause, s"c.requires_local = false")
Expand Down
2 changes: 2 additions & 0 deletions app/org/maproulette/models/utils/ChallengeFormatters.scala
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ trait ChallengeWrites extends DefaultWrites {
(JsPath \ "modified").write[DateTime] and
(JsPath \ "description").writeNullable[String] and
(JsPath \ "deleted").write[Boolean] and
(JsPath \ "isGlobal").write[Boolean] and
(JsPath \ "infoLink").writeNullable[String] and
JsPath.write[ChallengeGeneral] and
JsPath.write[ChallengeCreation] and
Expand Down Expand Up @@ -110,6 +111,7 @@ trait ChallengeReads extends DefaultReads {
((JsPath \ "modified").read[DateTime] or Reads.pure(DateTime.now())) and
(JsPath \ "description").readNullable[String] and
(JsPath \ "deleted").read[Boolean] and
(JsPath \ "isGlobal").read[Boolean] and
(JsPath \ "infoLink").readNullable[String] and
JsPath.read[ChallengeGeneral] and
JsPath.read[ChallengeCreation] and
Expand Down
1 change: 1 addition & 0 deletions app/org/maproulette/provider/KeepRightProvider.scala
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@ class KeepRightProvider @Inject() (
DateTime.now(),
None,
false,
false,
None,
ChallengeGeneral(User.superUser.id, rootProjectId, ""),
ChallengeCreation(),
Expand Down
7 changes: 5 additions & 2 deletions app/org/maproulette/session/SearchParameters.scala
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ case class SearchChallengeParameters(
challengeDifficulty: Option[Int] = None,
challengeStatus: Option[List[Int]] = None,
requiresLocal: Option[Int] = Some(SearchParameters.CHALLENGE_REQUIRES_LOCAL_EXCLUDE),
archived: Option[Boolean] = None
archived: Option[Boolean] = None,
filterGlobal: Option[Boolean] = None
)

case class SearchReviewParameters(
Expand Down Expand Up @@ -439,7 +440,9 @@ object SearchParameters {
//requiresLocal
this.getIntParameter(request.getQueryString("cLocal"), Some(params.challengeParams.requiresLocal.getOrElse(SearchParameters.CHALLENGE_REQUIRES_LOCAL_EXCLUDE))),
//includeArchived
this.getBooleanParameter(request.getQueryString("ca"), params.challengeParams.archived)
this.getBooleanParameter(request.getQueryString("ca"), params.challengeParams.archived),
//filterGlobal
this.getBooleanParameter(request.getQueryString("fg"), params.challengeParams.filterGlobal)
),
new SearchTaskParameters(
//taskTags
Expand Down
22 changes: 22 additions & 0 deletions conf/evolutions/default/98.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
# --!Ups
-- Add a new column 'is_global' to the 'challenges' table with a default value
ALTER TABLE IF EXISTS challenges
ADD COLUMN IF NOT EXISTS is_global BOOLEAN NOT NULL DEFAULT FALSE;

COMMENT ON COLUMN challenges.is_global IS
'The challenges.is_global represents if a challenge is classified as global, currently a challenge is classified as global if it is wider than 180 degrees (half the map width) or taller than 90 degrees (half the map height).';

-- Update 'is_global' column based on bounding box dimensions
UPDATE challenges
SET is_global = (
CASE
WHEN (ST_XMax(bounding)::numeric - ST_XMin(bounding)::numeric) > 180 THEN TRUE
WHEN (ST_YMax(bounding)::numeric - ST_YMin(bounding)::numeric) > 90 THEN TRUE
ELSE FALSE
END
);

# --!Downs
-- Drop the 'is_global' column
ALTER TABLE IF EXISTS challenges
DROP COLUMN is_global;
3 changes: 2 additions & 1 deletion docs/github_example.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ RESPONSE {
"description": "Project for Github Example.",
"groups": [],
"enabled": true,
"deleted": false
"deleted": false,
"isGlobal": false
}
```
***NOTE:** The owner id will match the ID of the user used based on the APIKey.<br/>
Expand Down
2 changes: 2 additions & 0 deletions test/org/maproulette/provider/ChallengeProviderSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ class ChallengeProviderSpec extends PlaySpec with MockitoSugar {
DateTime.now(),
None,
false,
false,
None,
ChallengeGeneral(101, 1, ""),
ChallengeCreation(),
Expand All @@ -30,6 +31,7 @@ class ChallengeProviderSpec extends PlaySpec with MockitoSugar {
DateTime.now(),
None,
false,
false,
None,
ChallengeGeneral(101, 1, ""),
ChallengeCreation(),
Expand Down
1 change: 1 addition & 0 deletions test/org/maproulette/utils/TestSpec.scala
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ trait TestSpec extends PlaySpec with MockitoSugar {
DateTime.now(),
None,
false,
false,
None,
ChallengeGeneral(101, 1, ""),
ChallengeCreation(),
Expand Down

0 comments on commit 51c893c

Please sign in to comment.