Skip to content
This repository has been archived by the owner on Dec 23, 2017. It is now read-only.

Feature/fix receipts link #867

Merged
merged 5 commits into from
Oct 22, 2015

Conversation

jmcarp
Copy link
Contributor

@jmcarp jmcarp commented Oct 20, 2015

Adds a few improvements related to https://github.com/18F/FEC/issues/72. It turned out that the receipts link on the committee detail page included a "cycle" parameter, which isn't one of the filters for the browse receipts page. This caused an immediate URL change upon loading the browse page.

  • Fix link to receipts per cycle from the committee detail page
  • Don't change URL based on irrelevant query parameters

cc @noahmanger

Note: looking at this code reminds me that it's kind of hairy in places and definitely in need of tests. I'm going to take a crack at refactoring the filter logic later this week--planning to add the missing tests there.

When comparing the current URL to the requested parameters, optionally
ignore a list of irrelevant keys. This allows us to avoid automatic URL
changes if the user includes irrelevant query parameters, like "?foo=bar",
in the URL.

Related to https://github.com/18F/FEC/issues/72.
@noahmanger
Copy link
Contributor

Only thing I noticed is the individual transaction links from the aggregates on election pages still have the cycle param.

@jmcarp
Copy link
Contributor Author

jmcarp commented Oct 21, 2015

Good catch @noahmanger. If I'm reading correctly, the URLs on the election page should be fixed in #876, so this patch should be good to go.

* Use built-in currency formatter from `Intl`.
* Pass correctly formatted numbers as query params.
* Remove unnecessary `cycle` param from aggregate links.
var column = meta.settings.aoColumns[meta.col].data;
return _.extend({
committee_id: (context.candidates[row.candidate_id] || {}).committee_ids,
cycle: context.election.cycle
committee_id: (context.candidates[row.candidate_id] || {}).committee_ids

Choose a reason for hiding this comment

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

Identifier 'committee_id' is not in camel case.
Identifier 'candidate_id' is not in camel case.
Identifier 'committee_ids' is not in camel case.

@jmcarp
Copy link
Contributor Author

jmcarp commented Oct 22, 2015

Patch updated @noahmanger.

Getting this behavior to work required some tweaks to the
jquery.inputmask configuration. This may get simpler once we move away
from input masking for our filters.

h/t @noahmanger
@jmcarp
Copy link
Contributor Author

jmcarp commented Oct 22, 2015

Patch updated.

noahmanger pushed a commit that referenced this pull request Oct 22, 2015
@noahmanger noahmanger merged commit 6f47886 into fecgov:develop Oct 22, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants