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

CRDCDH-1738 Data Submission List - Unlock Organization dropdown #516

Merged
merged 18 commits into from
Nov 5, 2024

Conversation

Alejandro-Vega
Copy link
Collaborator

@Alejandro-Vega Alejandro-Vega commented Oct 31, 2024

Overview

Update the Organization select dropdown from the Data Submission List table filters so that any user can select from relevant Organizations.

Change Details (Specifics)

  • Updated listSubmissions API to retrieve Organizations
  • Updated Data Submission List filters to not use OrganizationListContext, and replace with orgs from listSubmissions API
  • Updated Org dropdown to reset to "All" when the selected org is no longer an option
  • After a new submission is created, update the orgs, submitter names, and dataCommons lists
  • Updated tests to match new implementation
  • Fixed lineHeight issue in Data Submission summary section. The collaborators property was taller than all the other properties
  • Added a bandage fix for an existing bug. Unsure of cause, but please test to ensure it is fixed for you as well. Added comment with more details below

Related Ticket(s)

CRDCDH-1639 (Task)
CRDCDH-1639 (User Story)

@Alejandro-Vega Alejandro-Vega added the 🚧 Do Not Merge This PR is not ready for merging label Oct 31, 2024
@Alejandro-Vega Alejandro-Vega added this to the 3.1.0 (PMVP-M2) milestone Oct 31, 2024
@coveralls
Copy link
Collaborator

coveralls commented Oct 31, 2024

Pull Request Test Coverage Report for Build 11693245495

Details

  • 15 of 31 (48.39%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.04%) to 53.758%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/content/dataSubmissions/Controller.tsx 0 2 0.0%
src/components/DataSubmissions/DataSubmissionListFilters.tsx 15 19 78.95%
src/content/dataSubmissions/DataSubmissionsListView.tsx 0 10 0.0%
Files with Coverage Reduction New Missed Lines %
src/content/dataSubmissions/DataSubmissionsListView.tsx 1 0.0%
Totals Coverage Status
Change from base Build 11692532065: -0.04%
Covered Lines: 3715
Relevant Lines: 6424

💛 - Coveralls

@Alejandro-Vega
Copy link
Collaborator Author

@amattu2 The existing bug was where if you populate all dropdown fields to something other than "All", then switch the page, then switch one of the three dropdowns Org, Data Commons, or Submitter Names, it would switch to the new value but then switch back to the original value like so: "Alex" => "All" => "Alex". You should be able to see this on DEV. I believe this is a race-condition with the setting of the filters and resetting the table page back to the first.

Testing Steps:

  • Populate all dropdown fields to something other than "All"
  • Change to a page other than the first one
  • Modify a dropdown filter (Org, Data Commons, or Submitter Names) to something else
  • It should change the page back to 1 and the dropdown filter should change to the one you selected

Somehow adding a debounce with a 0 timer fixes the issue? Unsure how, but that's all I could come up with for now. If you have a better solution, please share. I've been debugging this for a while, but all my other solutions break something else unfortunately.

@Alejandro-Vega Alejandro-Vega removed the 🚧 Do Not Merge This PR is not ready for merging label Nov 5, 2024
@Alejandro-Vega Alejandro-Vega marked this pull request as ready for review November 5, 2024 19:58
@amattu2
Copy link
Member

amattu2 commented Nov 5, 2024

@amattu2 The existing bug was where if you populate all dropdown fields to something other than "All", then switch the page, then switch one of the three dropdowns Org, Data Commons, or Submitter Names, it would switch to the new value but then switch back to the original value like so: "Alex" => "All" => "Alex". You should be able to see this on DEV. I believe this is a race-condition with the setting of the filters and resetting the table page back to the first.

Testing Steps:

  • Populate all dropdown fields to something other than "All"
  • Change to a page other than the first one
  • Modify a dropdown filter (Org, Data Commons, or Submitter Names) to something else
  • It should change the page back to 1 and the dropdown filter should change to the one you selected

Somehow adding a debounce with a 0 timer fixes the issue? Unsure how, but that's all I could come up with for now. If you have a better solution, please share. I've been debugging this for a while, but all my other solutions break something else unfortunately.

I can replicate this somewhat consistently on DEV following your steps. Some observations:

  • The URL correctly updates by removing the filter, even though the filter itself doesn't reflect that
  • The Data Activity tab doesn't have this issue (also uses debouncing, so wanted to check)
  • handleFormChange is called 3 times under this scenario
    1. Correct values
    2. Old values
    3. Old values

I spent some time looking into it, but I can't think of what the root cause would be ATM. With your solution, it seems to be smooth and stable enough to just leave as-is for now.

amattu2
amattu2 previously approved these changes Nov 5, 2024
Copy link
Member

@amattu2 amattu2 left a comment

Choose a reason for hiding this comment

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

I left one annotation for a type issue, but it's not that significant. Approving as-is, if you decide to update it, feel free to merge without another approval.

src/graphql/listSubmissions.ts Outdated Show resolved Hide resolved
@Alejandro-Vega Alejandro-Vega merged commit 5378f55 into 3.1.0 Nov 5, 2024
5 checks passed
@Alejandro-Vega Alejandro-Vega deleted the CRDCDH-1738-org-dropdown branch November 5, 2024 22:07
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.

3 participants