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

#329: Add selector to EQIP and CSP total page #339

Merged
merged 14 commits into from
Nov 25, 2024

Conversation

pengyin-shan
Copy link
Collaborator

@pengyin-shan pengyin-shan commented Nov 18, 2024

This PR addresses issues #329 and #331.

Changes Made

  1. Selector Operation Added: The Selector operation has been added to both the EQIP and CSP pages.
  2. Column Pagination Implemented: Column pagination has been created for their tables to prevent overcrowding when users add many practices. This ensures the table does not expand excessively, preserving the overall layout.
  3. IRA Updates: The selector tables in IRA have been updated to incorporate this new feature.

How to Test

  1. Navigate to the EQIP page on the development site: https://policydesignlab-dev.ncsa.illinois.edu/eqip. You should see the default status reflect "All Practices" for both the map and the table:
Screenshot 2024-11-21 at 4 16 33 PM
  1. Select random practices. Test with more than three practices to observe column pagination. YThe number displayed in the map area should change based on your selections:
Screenshot 2024-11-21 at 4 18 04 PM
  1. Check that the table at the bottom of the page reflects your selections.Column pagination should function correctly when more than six columns are displayed:
Screenshot 2024-11-21 at 4 18 53 PM Screenshot 2024-11-21 at 4 19 32 PM
  1. Click the "Export this table to csv" button and you should be able to see the full table. Verify the table data.

  2. Repeart step 1 to 4 for the CSP: https://policydesignlab-dev.ncsa.illinois.edu/csp

  3. Play with IRA tables at https://policydesignlab-dev.ncsa.illinois.edu/ira to verify the column pagniation feature.

@pengyin-shan pengyin-shan changed the title [WIP] Update eqip selector to fit the new api #329: Add selector to EQIP and CSP total page Nov 21, 2024
@pengyin-shan pengyin-shan added this to the 1.4.0 milestone Nov 21, 2024
@pengyin-shan pengyin-shan marked this pull request as ready for review November 21, 2024 22:44
@navarroc
Copy link
Contributor

@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:

column_pagination_bug

@pengyin-shan
Copy link
Collaborator Author

@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:

column_pagination_bug

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.

@navarroc
Copy link
Contributor

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.

@pengyin-shan
Copy link
Collaborator Author

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.

Copy link
Contributor

@navarroc navarroc left a 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.

@sandeep-ps
Copy link
Member

sandeep-ps commented Nov 25, 2024

There may be a bug. I selected three practice codes for CSP, but all requests are using practice code 666. Please see the screenshot for more details. This is in the deployed development instance. It looks like this happens when selecting non-numeric practices.

image

Copy link
Contributor

@ylyangtw ylyangtw left a 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!

@pengyin-shan
Copy link
Collaborator Author

There may be a bug. I selected three practice codes for CSP, but all requests are using practice code 666. Please see the screenshot for more details. This is in the deployed development instance. It looks like this happens when selecting non-numeric practices.

image

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.

@sandeep-ps
Copy link
Member

There may be a bug. I selected three practice codes for CSP, but all requests are using practice code 666. Please see the screenshot for more details. This is in the deployed development instance. It looks like this happens when selecting non-numeric practices.
image

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.

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.
It's the url encoded form of /pdl/titles/title-ii/programs/csp/state-distribution?practice_code=1 - Irrigated Cropland

@pengyin-shan
Copy link
Collaborator Author

@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.

For example:
Screenshot 2024-11-25 at 1 55 52 PM

Copy link
Member

@sandeep-ps sandeep-ps left a 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.

@pengyin-shan
Copy link
Collaborator Author

As a note: the Crop Bundle (3 - Soil health rotation, No till) case is currently querying https://policydesignlab-dev.ncsa.illinois.edu/pdl/titles/title-ii/programs/csp/state-distribution?practice_code=3%20-%20Soil%20health%20rotation%2C%20No%20till. All practice-code part, including comma, is encoded.

The backend will be worked to handle this case.

@pengyin-shan pengyin-shan merged commit bb3bda2 into develop Nov 25, 2024
3 checks passed
@pengyin-shan pengyin-shan deleted the issue_329_eqip_csp_selector branch November 25, 2024 23:47
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.

4 participants