Repository: osmlab/maproulette3
Pull request #1206 (2020-04-14 21:05:13)

Adjust paging params for project API endpoints
* Pass page instead of calculated offset to project APIs as that is what
is now expected by the server as of v4

* Update search framework to accept props that can be passed along to
search functions

* Honor active project filters when additional pages of results are
retrieved by ProjectListWidget


Patch:
diff --git a/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/Messages.js b/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/Messages.js
index c567648e..dfc0d688 100644
--- a/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/Messages.js
+++ b/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/Messages.js
@@ -358,21 +358,6 @@ will not be able to make sense of it.
     defaultMessage: "Default priority level for tasks in this challenge",
   },
 
-  highPriorityRulesLabel: {
-    id: 'Admin.EditChallenge.form.highPriorityRules.label',
-    defaultMessage: "High Priority Rules",
-  },
-
-  mediumPriorityRulesLabel: {
-    id: 'Admin.EditChallenge.form.mediumPriorityRules.label',
-    defaultMessage: "Medium Priority Rules",
-  },
-
-  lowPriorityRulesLabel: {
-    id: 'Admin.EditChallenge.form.lowPriorityRules.label',
-    defaultMessage: "Low Priority Rules",
-  },
-
   step4Label: {
     id: 'Admin.EditChallenge.form.step4.label',
     defaultMessage: "Extra",
diff --git a/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/PriorityRuleGroup.js b/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/PriorityRuleGroup.js
index 9972cec2..a70f641e 100644
--- a/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/PriorityRuleGroup.js
+++ b/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/PriorityRuleGroup.js
@@ -2,8 +2,6 @@ import _isEmpty from 'lodash/isEmpty'
 import _map from 'lodash/map'
 import _isArray from 'lodash/isArray'
 import _compact from 'lodash/compact'
-import _flatten from 'lodash/flatten'
-import _groupBy from 'lodash/groupBy'
 import _trim from 'lodash/trim'
 
 /**
@@ -22,50 +20,27 @@ export const preparePriorityRuleGroupForForm = (ruleObject, isNested=false) => {
   if (!_isEmpty(ruleObject)) {
     if (_isArray(ruleObject.rules)) {
       preparedGroup.ruleGroup.condition = ruleObject.condition
-      preparedGroup.ruleGroup.rules = combineRulesForForm(
-        _map(ruleObject.rules, rule => preparePriorityRuleForForm(rule))
-      )
+      preparedGroup.ruleGroup.rules = _map(ruleObject.rules, rule => {
+        if (_isArray(rule.rules)) {
+          return preparePriorityRuleGroupForForm(rule, true)
+        }
+
+        // Key and Value are represented as a single `value` field, dot
+        // separated.
+        let [key, ...value] = rule.value.split('.')
+        return {
+          key,
+          valueType: rule.type,
+          operator: rule.operator,
+          value: _trim(value.join('.')),
+        }
+      })
     }
   }
 
   return preparedGroup
 }
 
-/**
- * Format a single priority rule from the server to the representation
- * expected by the edit form
- */
-export const preparePriorityRuleForForm = rule => {
-  if (_isArray(rule.rules)) {
-    return preparePriorityRuleGroupForForm(rule, true)
-  }
-
-  // Key and Value are represented as a single `value` field, dot separated
-  let [key, ...value] = rule.value.split('.')
-  return {
-    key,
-    valueType: rule.type,
-    operator: rule.operator,
-    value: _trim(value.join('.')),
-  }
-}
-
-/**
- * Combine multiple rules from the same group that match against the same
- * property and operator into a single rule with comma-separated values
- */
-export const combineRulesForForm = rules => {
-  const rulesByProperty = _groupBy(rules, rule => `${rule.key}:::${rule.operator}`)
-  return _map(rulesByProperty, groupedRules =>
-    groupedRules.length === 1 ?
-    groupedRules[0] :
-    Object.assign({}, groupedRules[0], {
-      // Quote any strings with literal commas prior to combining, and then join together
-      value: _map(groupedRules, rule => /,/.test(rule.value) ? `"${rule.value}"` : rule.value).join(','),
-    })
-  )
-}
-
 /**
  * Prepares a group of priority rules for saving, converting it to the
  * (stringified) JSON representation expected by the server.
@@ -74,80 +49,44 @@ export const preparePriorityRuleGroupForSaving = ruleGroup => {
   return JSON.stringify(normalizeRuleGroupForSaving(ruleGroup))
 }
 
-/**
- * Format a group of rules for the server
- */
 export const normalizeRuleGroupForSaving = ruleGroup => {
   const normalizedGroup = {}
 
-  const rules = _compact(_flatten(_map(
-    ruleGroup.rules, rule => normalizeRuleForSaving(rule)
-  )))
-
-  if (rules.length > 0) {
-    normalizedGroup.condition = ruleGroup.condition
-    normalizedGroup.rules = rules
-  }
-
-  return normalizedGroup
-}
-
-/**
- * Format a single rule for the server. Note that this can return an array of
- * multiple rules if comma-separated values were used and the rule needs to be
- * split
- */
-export const normalizeRuleForSaving = (rule, allowCSV=true) => {
-  if (rule.valueType === "nested rule") {
-    return normalizeRuleGroupForSaving(rule.ruleGroup)
-  }
-
-  if (_isEmpty(rule.key)) {
-    return null
-  }
+  const rules = _compact(_map(ruleGroup.rules, rule => {
+    if (rule.valueType === "nested rule") {
+      return normalizeRuleGroupForSaving(rule.ruleGroup)
+    }
 
-  // If there are multiple, comma-separated values, split into separate rules
-  // (ignoring commas in quoted strings)
-  if (allowCSV && /,/.test(rule.value)) {
-    return _flatten(csvStringToArray(rule.value)).map(value =>
-      normalizeRuleForSaving(Object.assign({}, rule, {value}), false)
-    )
-  }
+    if (_isEmpty(rule.key)) {
+      return null
+    }
 
-  // Due to react-jsonschema-form bug #768, the default operator values
-  // don't get populated if the user doesn't change their selection, so we
-  // set the operator to the default here if needed
-  if (!rule.operator && rule.valueType) {
-    if (rule.valueType === "string") {
-      rule.operator = "equal" // default string operator
+    // Due to react-jsonschema-form bug #768, the default operator values
+    // don't get populated if the user doesn't change their selection, so we
+    // set the operator to the default here if needed
+    if (!rule.operator && rule.valueType) {
+      if (rule.valueType === "string") {
+        rule.operator = "equal" // default string operator
+      }
+      else {
+        rule.operator = "==" // default numeric operator
+      }
     }
-    else {
-      rule.operator = "==" // default numeric operator
+
+    // The server expects the Key and Value to be represented as a single
+    // `value` string, dot-separated. To work-around an MR2 bug, we set
+    // empty values to a single space for now.
+    return {
+      value: `${rule.key}.${_isEmpty(rule.value) ? ' ' : rule.value}`,
+      type: rule.valueType,
+      operator: rule.operator,
     }
-  }
+  }))
 
-  // The server expects the Key and Value to be represented as a single
-  // `value` string, dot-separated. To work-around an MR2 bug, we set
-  // empty values to a single space for now.
-  return {
-    value: `${rule.key}.${_isEmpty(rule.value) ? ' ' : rule.value}`,
-    type: rule.valueType,
-    operator: rule.operator,
+  if (rules.length > 0) {
+    normalizedGroup.condition = ruleGroup.condition
+    normalizedGroup.rules = rules
   }
-}
 
-/**
- * Parse CSV, handling quoted strings and escaped characters
- * From https://gist.github.com/Jezternz/c8e9fafc2c114e079829974e3764db75
- */
-const csvStringToArray = strData => {
-  const objPattern = new RegExp(("(\\,|\\r?\\n|\\r|^)(?:\"([^\"]*(?:\"\"[^\"]*)*)\"|([^\\,\\r\\n]*))"),"gi")
-  let arrMatches = null, arrData = [[]]
-  while ((arrMatches = objPattern.exec(strData))){
-      if (arrMatches[1].length && arrMatches[1] !== ",")arrData.push([])
-      arrData[arrData.length - 1].push(arrMatches[2] ?
-          arrMatches[2].replace(new RegExp( "\"\"", "g" ), "\"") :
-          arrMatches[3])
-  }
-  return arrData
+  return normalizedGroup
 }
diff --git a/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/Step3Schema.js b/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/Step3Schema.js
index 07447aca..cd1ae09f 100644
--- a/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/Step3Schema.js
+++ b/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/Step3Schema.js
@@ -129,21 +129,21 @@ export const jsSchema = intl => {
         default: TaskPriority.high,
       },
       highPriorityRules: {
-        title: intl.formatMessage(messages.highPriorityRulesLabel),
+        title: "High Priority Rules",
         type: "object",
         properties: {
           ruleGroup: { "$ref": "#/definitions/priorityRuleGroup" },
         },
       },
       mediumPriorityRules: {
-        title: intl.formatMessage(messages.mediumPriorityRulesLabel),
+        title: "Medium Priority Rules",
         type: "object",
         properties: {
           ruleGroup: { "$ref": "#/definitions/priorityRuleGroup" },
         },
       },
       lowPriorityRules: {
-        title: intl.formatMessage(messages.lowPriorityRulesLabel),
+        title: "Low Priority Rules",
         type: "object",
         properties: {
           ruleGroup: { "$ref": "#/definitions/priorityRuleGroup" },
diff --git a/src/components/AdminPane/Manage/Widgets/ProjectListWidget/ProjectListWidget.js b/src/components/AdminPane/Manage/Widgets/ProjectListWidget/ProjectListWidget.js
index 8ac49617..799afaec 100644
--- a/src/components/AdminPane/Manage/Widgets/ProjectListWidget/ProjectListWidget.js
+++ b/src/components/AdminPane/Manage/Widgets/ProjectListWidget/ProjectListWidget.js
@@ -140,13 +140,19 @@ const Widget =
       'resultProjects'
     ),
     'adminProjectList',
-    queryCriteria => {
+    (queryCriteria, resultsPerPage, props) => {
       // We only fetch all managed projects if we are not doing a query.
       if (queryCriteria.query) {
         return null
       }
-      return fetchManageableProjects(_get(queryCriteria, 'page.currentPage'),
-                                     _get(queryCriteria, 'page.resultsPerPage'))
+
+      const filters = _get(props, 'currentConfiguration.filters.projectFilters', {})
+      return fetchManageableProjects(
+        _get(queryCriteria, 'page.currentPage'),
+        _get(queryCriteria, 'page.resultsPerPage'),
+        filters.owner,
+        filters.visible
+       )
     },
   )
 
diff --git a/src/components/HOCs/WithSearch/WithSearch.js b/src/components/HOCs/WithSearch/WithSearch.js
index 0b42ed5a..1a580c72 100644
--- a/src/components/HOCs/WithSearch/WithSearch.js
+++ b/src/components/HOCs/WithSearch/WithSearch.js
@@ -42,7 +42,7 @@ const WithSearch = (WrappedComponent, searchGroup, searchFunction) => {
   // Debounce the search function so the server doesn't get hammered as a user
   // types in a query string
   const debouncedSearch = searchFunction ?
-    _debounce(props => props.performSearch(props.searchCriteria, searchFunction),
+    _debounce(props => props.performSearch(props.searchCriteria, searchFunction, props),
               1000, {leading: false}) : null
 
   return WithUserLocation(
@@ -119,8 +119,8 @@ export const mapStateToProps = (state, searchGroup) => {
 }
 
 export const mapDispatchToProps = (dispatch, ownProps, searchGroup) => ({
-  performSearch: (query, searchFunction) => {
-    return dispatch(performSearch(searchGroup, query, searchFunction))
+  performSearch: (query, searchFunction, props) => {
+    return dispatch(performSearch(searchGroup, query, searchFunction, props))
   },
 
   setSearch: (query, searchName) => {
diff --git a/src/components/TopUserChallenges/TopUserChallengesWidget.js b/src/components/TopUserChallenges/TopUserChallengesWidget.js
index cfe789d3..7c3f67cb 100644
--- a/src/components/TopUserChallenges/TopUserChallengesWidget.js
+++ b/src/components/TopUserChallenges/TopUserChallengesWidget.js
@@ -27,31 +27,17 @@ const descriptor = {
 }
 
 export class TopUserChallengesWidget extends Component {
-  updateChallenges = monthsPast => {
-    this.props.fetchTopChallenges(
-      this.props.user.id,
-      subMonths(new Date(), monthsPast)
-    )
-  }
-
-  currentMonthsPast = () => {
-    return this.props.widgetConfiguration.monthsPast || 1
-  }
-
   setMonthsPast = monthsPast => {
     if (this.props.widgetConfiguration.monthsPast !== monthsPast) {
       this.props.updateWidgetConfiguration({monthsPast})
-      this.updateChallenges(monthsPast)
-    }
-  }
-
-  componentDidMount() {
-    if (this.props.user) {
-      this.updateChallenges(this.currentMonthsPast())
+      this.props.fetchTopChallenges(this.props.user.id,
+                                    subMonths(new Date(), monthsPast))
     }
   }
 
   render() {
+    const monthsPast = this.props.widgetConfiguration.monthsPast || 1
+
     return (
       <QuickWidget
         {...this.props}
@@ -64,7 +50,7 @@ export class TopUserChallengesWidget extends Component {
               this.props.lightMode ? "mr-button--green" : "mr-button--green-lighter"
             )}
             pastMonthsOptions={[1, 3, 6, 12]}
-            currentMonthsPast={this.currentMonthsPast()}
+            currentMonthsPast={monthsPast}
             selectDuration={this.setMonthsPast}
           />
         }
diff --git a/src/lang/en-US.json b/src/lang/en-US.json
index cb1e53b0..9ce4ab1a 100644
--- a/src/lang/en-US.json
+++ b/src/lang/en-US.json
@@ -95,9 +95,6 @@
   "Admin.EditChallenge.form.step3.description": "The priority of tasks can be defined as High, Medium and Low. All high priority tasks will be offered to users first when working through a challenge, followed by medium and finally low priority tasks. Each task's priority is assigned automatically based on the rules you specify below, each of which is evaluated against the task's feature properties (OSM tags if you are using an Overpass query, otherwise whatever properties you've chosen to include in your GeoJSON). Tasks that don't pass any rules will be assigned the default priority.",
   "Admin.EditChallenge.form.defaultPriority.label": "Default Priority",
   "Admin.EditChallenge.form.defaultPriority.description": "Default priority level for tasks in this challenge",
-  "Admin.EditChallenge.form.highPriorityRules.label": "High Priority Rules",
-  "Admin.EditChallenge.form.mediumPriorityRules.label": "Medium Priority Rules",
-  "Admin.EditChallenge.form.lowPriorityRules.label": "Low Priority Rules",
   "Admin.EditChallenge.form.step4.label": "Extra",
   "Admin.EditChallenge.form.step4.description": "Extra information that can be optionally set to give a better mapping experience specific to the requirements of the challenge",
   "Admin.EditChallenge.form.updateTasks.label": "Remove Stale Tasks",
@@ -1116,9 +1113,6 @@
   "Challenge.status.deletingTasks": "Deleting Tasks",
   "Challenge.type.challenge": "Challenge",
   "Challenge.type.survey": "Survey",
-  "Challenge.cooperativeType.none": "None",
-  "Challenge.cooperativeType.tags": "Tag Fix",
-  "Challenge.cooperativeType.changeFile": "Cooperative",
   "Editor.none.label": "None",
   "Editor.id.label": "Edit in iD (web editor)",
   "Editor.josm.label": "Edit in JOSM",
diff --git a/src/services/Project/Project.js b/src/services/Project/Project.js
index a5c2c6c8..3a5836bb 100644
--- a/src/services/Project/Project.js
+++ b/src/services/Project/Project.js
@@ -82,7 +82,7 @@ export const fetchManageableProjects = function(page = null, limit = RESULTS_PER
     return new Endpoint(
       api.projects.managed, {
         schema: [ projectSchema() ],
-        params: {limit: limit, page: (pageToFetch * limit), onlyOwned, onlyEnabled}
+        params: {limit: limit, page: pageToFetch, onlyOwned, onlyEnabled}
       }
     ).execute().then(normalizedResults => {
       dispatch(receiveProjects(normalizedResults.entities))
@@ -174,7 +174,7 @@ export const searchProjects = function(searchCriteria, limit=RESULTS_PER_PAGE) {
         params: {
           q: `%${query}%`,
           onlyEnabled: onlyEnabled ? 'true' : 'false',
-          page: page * limit,
+          page,
           limit,
         }
     }).execute().then(normalizedResults => {
diff --git a/src/services/Search/Search.js b/src/services/Search/Search.js
index 313f2389..f4ab4adb 100644
--- a/src/services/Search/Search.js
+++ b/src/services/Search/Search.js
@@ -403,7 +403,7 @@ export const receivedResults = function(searchName, fetchId) {
 }
 
 // async action creators
-export const performSearch = function(searchName, query, asyncSearchAction) {
+export const performSearch = function(searchName, query, asyncSearchAction, props) {
   return function(dispatch) {
     const fetchId = _uniqueId()
     if (!query || query.length < 2) {
@@ -411,7 +411,7 @@ export const performSearch = function(searchName, query, asyncSearchAction) {
     }
 
     const resultsPerPage = _get(query, 'page.resultsPerPage')
-    const actionToDo = asyncSearchAction(query, resultsPerPage)
+    const actionToDo = asyncSearchAction(query, resultsPerPage, props)
 
     if (actionToDo) {
       dispatch(fetchingResults(searchName, fetchId))


Test patch:
diff --git a/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/PriorityRuleGroup.test.js b/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/PriorityRuleGroup.test.js
index 3bdf529b..e908c0e2 100644
--- a/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/PriorityRuleGroup.test.js
+++ b/src/components/AdminPane/Manage/ManageChallenges/EditChallenge/PriorityRuleGroup.test.js
@@ -8,7 +8,6 @@ describe("preparePriorityRuleGroupForSaving", () => {
   let emptyOperatorFormRule = null
   let eqOperatorFormRule = null
   let lteOperatorFormRule = null
-  let csvFormRule = null
 
   beforeEach(() => {
     equalsOperatorFormRule = {
@@ -45,13 +44,6 @@ describe("preparePriorityRuleGroupForSaving", () => {
       value: "456",
     }
 
-    csvFormRule = {
-      key: "firstTag",
-      valueType: "string",
-      operator: "equals",
-      value: "foo,baz",
-    }
-
     basicFormGroup = {
       condition: 'AND',
       rules: [
@@ -126,36 +118,11 @@ describe("preparePriorityRuleGroupForSaving", () => {
 
     expect(result.rules[2].operator).toEqual("==")
   })
-
-  test("comma-separated values are split into separate rules", () => {
-    basicFormGroup.rules = [csvFormRule]
-
-    const result =
-      JSON.parse(preparePriorityRuleGroupForSaving(basicFormGroup))
-
-    expect(result.rules.length).toBe(2)
-    expect(result.rules[0].value).toEqual("firstTag.foo")
-    expect(result.rules[1].value).toEqual("firstTag.baz")
-  })
-
-  test("commas are ignored in quoted strings when splitting comma-separated values", () => {
-    csvFormRule.value = '"foo,baz"'
-    basicFormGroup.rules = [csvFormRule]
-
-    const result =
-      JSON.parse(preparePriorityRuleGroupForSaving(basicFormGroup))
-
-    expect(result.rules.length).toBe(1)
-    expect(result.rules[0].value).toEqual("firstTag.foo,baz")
-  })
 })
 
 describe("preparePriorityRuleGroupForForm", () => {
   let basicSavedGroup = null
   let equalsOperatorSavedRule = null
-  let anotherFirstTagEqualsSavedRule = null
-  let notEqualsFirstTagSavedRule = null
-  let literalCommaFirstTagSavedRule = null
   let containsOperatorSavedRule = null
   let emptyOperatorSavedRule = null
   let eqOperatorSavedRule = null
@@ -168,24 +135,6 @@ describe("preparePriorityRuleGroupForForm", () => {
       value: "firstTag.foo",
     }
 
-    anotherFirstTagEqualsSavedRule = {
-      type: "string",
-      operator: "equals",
-      value: "firstTag.baz",
-    }
-
-    notEqualsFirstTagSavedRule = {
-      type: "string",
-      operator: "notEquals",
-      value: "firstTag.baz",
-    }
-
-    literalCommaFirstTagSavedRule = {
-      type: "string",
-      operator: "equals",
-      value: "firstTag.literal,comma",
-    }
-
     containsOperatorSavedRule = {
       type: "string",
       operator: "contains",
@@ -253,34 +202,4 @@ describe("preparePriorityRuleGroupForForm", () => {
 
     expect(result).toMatchSnapshot()
   })
-
-  test("multiple rules for same property are combined with comma-separated values", () => {
-    basicSavedGroup.rules.push(anotherFirstTagEqualsSavedRule)
-
-    const result = preparePriorityRuleGroupForForm(basicSavedGroup).ruleGroup
-
-    expect(result.rules.length).toBe(basicSavedGroup.rules.length - 1)
-    expect(result.rules[0].key).toEqual("firstTag")
-    expect(result.rules[0].value).toEqual("foo,baz")
-  })
-
-  test("literal commas appearing in values are quoted", () => {
-    basicSavedGroup.rules.push(literalCommaFirstTagSavedRule)
-
-    const result = preparePriorityRuleGroupForForm(basicSavedGroup).ruleGroup
-
-    expect(result.rules.length).toBe(basicSavedGroup.rules.length - 1)
-    expect(result.rules[0].key).toEqual("firstTag")
-    expect(result.rules[0].value).toEqual('foo,"literal,comma"')
-  })
-
-  test("multiple rules for same property with different operators aren't combined", () => {
-    basicSavedGroup.rules.push(notEqualsFirstTagSavedRule)
-
-    const result = preparePriorityRuleGroupForForm(basicSavedGroup).ruleGroup
-
-    expect(result.rules.length).toBe(basicSavedGroup.rules.length)
-    expect(result.rules[0].key).toEqual("firstTag")
-    expect(result.rules[0].value).toEqual("foo")
-  })
 })
diff --git a/src/components/HOCs/WithSearch/WithSearch.test.js b/src/components/HOCs/WithSearch/WithSearch.test.js
index 3996bcfa..32640c9e 100644
--- a/src/components/HOCs/WithSearch/WithSearch.test.js
+++ b/src/components/HOCs/WithSearch/WithSearch.test.js
@@ -197,10 +197,11 @@ test("searchQueries.searchGroup is passed through to the wrapped component", ()
 test("mapDispatchToProps maps function performSearch", () => {
   const dispatch = jest.fn(() => Promise.resolve())
   const mappedProps = mapDispatchToProps(dispatch, {}, "searchGroup")
+  const someProps = {foo: "foo"}
 
-  mappedProps.performSearch("query", "searchProjects")
+  mappedProps.performSearch("query", "searchProjects", someProps)
   expect(dispatch).toBeCalled()
-  expect(performSearch).toBeCalledWith("searchGroup", "query", "searchProjects")
+  expect(performSearch).toBeCalledWith("searchGroup", "query", "searchProjects", someProps)
   expect(mappedProps).toMatchSnapshot()
 })