-
Notifications
You must be signed in to change notification settings - Fork 0
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
#329: Add selector to EQIP and CSP total page #339
Conversation
@pengyin-shan I really like the column pagination to avoid the really long scroll bar. One issue though, I noticed if you select 4 or 5 practices where it adds a second page and then you go to page 2 and then select "all practices", which clears out the table, it says I'm on page 2 of 1. See screenshot: |
Thanks for catching this @navarroc ! The bug occurred because the table's page wasn't resetting when switching to "All Practices". I've pushed a fix that resets both the column page and data page when the practice selection changes. |
The column reset issue is fixed, but I noticd another issue. I see an issue with the exported data for EQIP. It doesn't match what is displayed in the table. For example, I selected practice 101, 110 and 128. California in the tables shows 15146 for Agricultural Energy Management Plan - Written Benefits, but in the exported file it says 287,352. |
Thank you @navarroc , this behavior was caused by a column mismatch, and I have replaced it with a more stable method to handle this. The downloaded file from the dev site should now reflect this change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Tested and verified issues I noted were fixed. I don't see any other issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It works well and looks good!
This happened because I assumed all codes were numerical. To confirm, should the codes be alphabetical? Will any special characters appear in the codes? Could you provide an example of an alphabetical code that is non-zero so I can test? It shouldn’t take too much time to add this feature. |
…acters for EQIP and CSP selector
Copying from Slack for future reference: This is an example with special characters: https://policydesignlab-dev.ncsa.illinois.edu/pdl/titles/title-ii/programs/csp/state-distribution?practice_code=%221%20-%20Irrigated%20Cropland%22. |
@sandeep-ps (cc: @navarroc @ylyangtw), I have updated the code so that the selector now works with practice names containing alphanumeric characters and special symbols. These changes apply to both the EQIP and CSP pages, but it seems that only the CSP page is currently testable. Please test multiple practices with different name formats on the CSP page to ensure they all work correctly. Let me know if you still notice the issue. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a known issue due to how a practice name and code are ingested in the database. It will be addressed through a backend fix. The rest looks good to me. Approving.
As a note: the The backend will be worked to handle this case. |
This PR addresses issues #329 and #331.
Changes Made
How to Test
Click the "Export this table to csv" button and you should be able to see the full table. Verify the table data.
Repeart step 1 to 4 for the CSP: https://policydesignlab-dev.ncsa.illinois.edu/csp
Play with IRA tables at https://policydesignlab-dev.ncsa.illinois.edu/ira to verify the column pagniation feature.