Skip to content

Allow filtering on Array fields.#53

Merged
gmaclennan merged 1 commit intorewritefrom
filter-array-values
Dec 6, 2016
Merged

Allow filtering on Array fields.#53
gmaclennan merged 1 commit intorewritefrom
filter-array-values

Conversation

@gmaclennan
Copy link
Member

This is a little tricky because Mapbox filters do not work on arrays. Instead we flatten the array into:

arrayField.0: arrayValue0, arrayField.1: arrayValue1, arrayField.2: arrayValue2

Then pass the same filter to each of these and group with an ‘any’ filter.

Things not working quite right, or untested:

  • Arrays that contain non-string fields
  • The Popup component does not render the title or subtitle correctly if these are array fields. Currently the Popup gets its props from the Mapbox component, which has the flattened data, and the Popup is rendered in a separate context without access to the redux store, so we can't access the selectors.
  • Some of the code for array values is a little fragile and edge-cases might break things.
  • ODK Collect tends to return space-separated fields rather than array fields, so we need to detect and transform those.

This is a little tricky because Mapbox filters do not work on arrays. Instead we flatten the array into:

arrayField.0: arrayValue0, arrayField.1: arrayValue1, arrayField.2: arrayValue2

Then pass the same filter to each of these and group with an ‘any’ filter.
@gmaclennan gmaclennan mentioned this pull request Dec 6, 2016
p[v] = typeof p[v] === 'undefined' ? 1 : p[v] + 1
v = Array.isArray(v) ? v : [v]
v.forEach(function (w) {
p[w] = typeof p[w] === 'undefined' ? 1 : p[w] + 1
Copy link

@mojodna mojodna Dec 6, 2016

Choose a reason for hiding this comment

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

My eye snagged on this. Is this the same as:

v.forEach(w => {
  p[w] = (p[w] || 0) + 1
})

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes it would be, although I try to avoid this pattern when I remember to, because you can get bugs when you have falsey values (e.g. p[w] = 0), although in this case we would be fine, I think the more explicit code is better.

@mojodna
Copy link

mojodna commented Dec 6, 2016

LGTM on a cursory reading.

@gmaclennan gmaclennan merged commit 8244a24 into rewrite Dec 6, 2016
@gmaclennan gmaclennan deleted the filter-array-values branch December 9, 2016 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants