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

Inconsistency between information displayed on data page and download file #2481

Closed
4 of 8 tasks
lbeaufort opened this issue Oct 29, 2018 · 4 comments · Fixed by #2488
Closed
4 of 8 tasks

Inconsistency between information displayed on data page and download file #2481

lbeaufort opened this issue Oct 29, 2018 · 4 comments · Fixed by #2488
Assignees
Milestone

Comments

@lbeaufort
Copy link
Member

lbeaufort commented Oct 29, 2018

@PaulClark2 commented on Sun Sep 02 2018

Reported by: fecgov/FEC#7693

What we are after: The data page should display all the rows that are included in the download file.

Why we are after it: The data page should display all request data to users. The inconsistency between the data page and the download file undermines user confidence.

Current state:
0ae61d62-548c-45fe-a96f-325f90521b7a

c6db719c-2a66-4b74-92a0-861b75d16965

Business completion criteria:

  • understand and explain the cause of the discrepancy
  • make the data page row count and the download row count the same (technical completion criteria)

Technical completion criteria:

  • Get rid of sort_hide_null=true and change to =false and see if it's fixed < we know this will work
  • Modify datatables to sort nulls as the last items in the list when sorting descending and the top when sorting ascending
  • Check, and if necessary, correct across all datatables for consistency: Candidates, committees, filings, reports....etc. Let's keep these consistent across the app - there shouldn't be any change to receipts/disbursements because the only fields we can sort on rarely have null data, but if we added more sort fields it could manifest there.

@patphongs commented on Tue Sep 04 2018

This is related to #3104


@PaulClark2 commented on Wed Sep 05 2018

It might be related to #3104. I’d like someone to confirm this is the same issue.

The symptoms that prompted the feedback issue seem to be different. The number of rows in the data table don’t match the number of rows in the download file.. (The estimate is also wrong.) We are not displaying all the results to the user in the data table. The only way a user can see all the results is to download the file.

What’s the correct answer to this user’s query, two or seven? If it’s seven like the download implies, why aren’t we displaying seven rows in the data table.


@patphongs commented on Wed Sep 05 2018

@PaulClark2 The devs talked about this yesterday and it is related to #3104. The count estimates seem to be inaccurate across the board and this is another example of inaccurate counts. I'm going to close this as a duplicate and have already linked this issue to the counts issue ticket.


@PaulClark2 commented on Wed Sep 05 2018

@patphongs this isn’t really about the counts. The data table displays 2 rows the download contains 7 rows. Which answer is correct?


@patphongs commented on Wed Sep 05 2018

@PaulClark2 I see what you're saying, I'll ask the team about this.


@patphongs commented on Wed Sep 05 2018

@PaulClark2 Did some additional investigation and what is happening is that there is an additional flag for sort_hide_null=true in the API request (See below). That means that in the datatable, any records with null values will not be returned, hence why you only see 2 rows.

I believe this flag is turned on when we want to save on performance especially on receipt and disbursement queries. But for the downloads, it retrieves null records as well and returns that in the CSV. I guess the question is, do we want the download to also hide null values?

2 records (datatable call):
https://api.open.fec.gov/v1/candidates/?api_key=5yyI90SU3Xb73TVlv4wrEhQxYcCwMWCywQiGdYbJ&sort_hide_null=true&q=jeff+brown&sort=-first_file_date&per_page=30&page=1

7 records:
https://api.open.fec.gov/v1/candidates/?api_key=5yyI90SU3Xb73TVlv4wrEhQxYcCwMWCywQiGdYbJ&q=jeff+brown&sort=-first_file_date&per_page=30&page=1


@AmyKort commented on Fri Sep 14 2018

Let's explore the possibility that was raised today of providing users the option to pick a data set that contains null committees and another option that excludes those results.


@patphongs commented on Tue Sep 18 2018

Some ideas for this that was discussed. @AmyKort @PaulClark2 can you please help make a decision about what the business logic should be and we can proceed from there?

  • Creating a toggle so that users can switch show_hide_null
  • Keep the business logic the way it is now and leaving nulls inside the downloads
  • Remove nulls from downloads

@lbeaufort commented on Fri Oct 05 2018

@PaulClark2 I confirmed that the issue we discussed with the filings data table count being off stems from the same issue, so I'm not going to enter a separate bug.

We need to:

  1. Determine if hiding nulls in the sort column is the desired default behavior on the front end (this doesn't seem ideal but I can look into it more)
  2. Modify the API to adjust the counts downward if we're hiding results that have a null value in the sort column

@patphongs commented on Mon Oct 29 2018

We need some business logic clarification:

What is sort_hide_null

sort_hide_null toggles rows on and off that have sort column that is null. By default the sort_hide_null=true and therefore will hide any null rows from the sorted column. If we turn this off and have sort_hide_null=false it will return all data.

Possible problems:

  • If we set sort_hide_null=false by default, we will get null rows returned for the sorted column.

Business expert questions @PaulClark2 @jwchumley @JonellaCulmer:

  • We need to be consistent with our exports. Currently our exports will return null data. The front-end view will not. Do we want to turn on null results for the front end datatable view?
  • Will null data that is returned be confusing and less filterable? Because it's difficult to go through hundreds and possibly thousands of rows of sorted null data on the front end?

Possible technical concerns:

  • Need to investigate if this will effect performance because there may be a lot of null data. Database experts should query the data to see how many null rows are returned for sorted columns.

@lbeaufort commented on Mon Oct 29 2018

@patphongs in this example, the front-end view should return 7 rows, not 2.


@PaulClark2 commented on Mon Oct 29 2018

Agreed, the front end should show seven rows.

@lbeaufort
Copy link
Member Author

@patphongs
Copy link
Member

patphongs commented Nov 1, 2018

Now I realize that there's an AJAX call that happens every time there's a sort. Because of this, we'll need a corresponding API change so that the sortable column or sort option always returns nulls last, see issue fecgov/openFEC#3470.

For now, the CMS fix (PR #2488) for this issue will simply be to display all data, including nulls by default.

@lbeaufort
Copy link
Member Author

Re-opening to track the follow-up task here: #2491

@lbeaufort
Copy link
Member Author

I confirmed on dev that this example now works properly and returns 7 rows in the data table as well as 7 rows in the download file.
https://fec-dev-proxy.app.cloud.gov/data/candidates/?q=jeff+brown&q=jeffrey+vincent+brown
screen shot 2018-11-08 at 12 59 07 pm

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants