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

Fix to Filter not being proprely handled when naviagating back to Home Page #1205

Merged
merged 12 commits into from Aug 22, 2017

Conversation

jpwentz
Copy link
Contributor

@jpwentz jpwentz commented Aug 11, 2017

No description provided.

@jpwentz
Copy link
Contributor Author

jpwentz commented Aug 22, 2017

These are insanely particular code checks . It can still be merged.

Copy link
Contributor

@jeremiak jeremiak left a comment

Choose a reason for hiding this comment

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

This looks pretty great! I fixed the linting errors and added some tests for the new utility file

if (f.crime && !isValidCrime(f.crime)) delete f.crime
if (f.place && !lookupUsa(f.place, f.placeType)) delete f.place

const f = filterValidator({ ...filters })
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's combine this with the line below so that we don't have to declare the variable

@@ -1,15 +1,9 @@
import { FILTER_RESET, FILTERS_UPDATE } from './constants'
import offenses from '../util/offenses'
import lookupUsa from '../util/usa'
import filterValidator from '../util/filter'
Copy link
Contributor

Choose a reason for hiding this comment

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

Love pulling this out into a util

@@ -0,0 +1,63 @@
/* eslint no-undef: 0 */
Copy link
Contributor

Choose a reason for hiding this comment

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

I added a test file for the new util, FYI

@jeremiak jeremiak merged commit 6acf6af into master Aug 22, 2017
@jeremiak jeremiak deleted the jw-summary-range-fix branch August 22, 2017 22:12
@jeremiak jeremiak mentioned this pull request Aug 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants