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

fixed issue #130, so people can select from browse by agency multiple time #131

Merged
merged 2 commits into from
Feb 27, 2019

Conversation

DanielJDufour
Copy link
Contributor

A similar PR may already be submitted!
Please search among the Pull request before creating one.

Thanks for submitting a pull request! Please provide enough information so that others can review your pull request:

For more information, see the CONTRIBUTING guide.

Summary

This PR fixes/implements the following bugs/features

  • Don't accumulate federal agency filters when you select browse by agency multiple times

Explain the motivation for making this change. What existing problem does the pull request solve?
Issue #130

Test plan (required)

Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes UI.
Manually tested and also added to pre-deploy testing checklist

Code formatting

Closing issues

Fixes #130

…tiple times and browse projects will only filter by that agency selected each time
@DanielJDufour DanielJDufour requested a review from a team December 21, 2018 18:29
@DanielJDufour DanielJDufour changed the title fixed issue #130, so people can select from browse by agency multiple time WIP: fixed issue #130, so people can select from browse by agency multiple time Dec 21, 2018
@DanielJDufour
Copy link
Contributor Author

Don't review this yet. Just discovered a bug when I select All in Browse by Agency

@DanielJDufour DanielJDufour removed the request for review from a team December 21, 2018 18:33
@DanielJDufour
Copy link
Contributor Author

Ready for review now

@DanielJDufour DanielJDufour requested a review from a team December 21, 2018 18:43
@DanielJDufour DanielJDufour changed the title WIP: fixed issue #130, so people can select from browse by agency multiple time fixed issue #130, so people can select from browse by agency multiple time Dec 21, 2018
@@ -25,21 +25,21 @@ const mapDispatchToProps = dispatch => {
const value = normalize(event.target.value)
if (value !== 'browse by agency') {
let url
Copy link
Contributor

Choose a reason for hiding this comment

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

let url = `/browse-projects?agencies=${value}&page=1&size=10&sort=data_quality`

size: 10,
sort: 'data_quality',
filters: []
}
if (value === 'all') {
url = '/browse-projects?page=1&size=10&sort=data_quality'
Copy link
Contributor

Choose a reason for hiding this comment

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

url = url.split(`agencies=${value}&`).join('')

@ftonato
Copy link
Contributor

ftonato commented Dec 26, 2018

Hello @DanielJDufour,

Your changes that enough, but I suggest simple updates (you can see above).
If you believe is interesting, accept, otherwise just accept this PR ;)

@saracope saracope merged commit 298fdcc into master Feb 27, 2019
@saracope saracope deleted the 130ClearAgencyFilter branch February 27, 2019 18:08
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.

Results from previous agency search do not clear upon search for new agency
3 participants