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

Add support for sorting columns in prep dialog #5214

Draft
wants to merge 11 commits into
base: production
Choose a base branch
from

Conversation

grantfitzsimmons
Copy link
Member

@grantfitzsimmons grantfitzsimmons commented Aug 12, 2024

Fixes #5142

This makes it so that the columns in the preparation select dialog are now sortable. This includes only Catalog Number, Taxon, Preparation, Available, and Unavailable. 'Selected' is not sortable at this time.

AI Disclaimer: This PR was drafted with help from GPT-4o mini. This is largely an experiment to see how capable it is.

I'm going to work on this in my free time. If we see this being an urgent priority, please have a staff developer begin work on this!

Checklist

  • Self-review the PR after opening it to make sure the changes look good
    and self-explanatory (or properly documented)
  • Add automated tests
  • Add relevant issue to release milestone

Testing instructions

Please test this side-by-side with the latest v7 tagged release. If an issue is encountered, please verify that it also does not happen in edge before requesting changes.

  • Create a series of new interaction records by clicking the Interactions menu item
    • Create a new Loan, Gift, Exchange In, Exchange Out, and Disposal record based on a record set and based on a series of catalog numbers.
    • Repeat the process for each of these interactions:
      • In the new interaction dialog, select several rows at random.
      • Enter a specific quantity in the 'selected' column and write it down alongside the catalog number, taxon name, preparation type, and quantities.
      • Sort the rows by the headers (Catalog Number, Taxon, Preparation, Available, and Unavailable)
      • Select more items and do the same step, capturing the specific preparation information
      • Upon sorting, make sure the appropriate 'selected' values remain with the appropriate preparation
      • Create the new interaction record
      • Verify that under the '{Interaction} Preparations' subview that the correct 'selected' quantities appear with the appropriate loan preparation.
      • Navigate to the CO records directly and verify that the preparations are linked to the correct interaction.
      • For Loans: Test that the 'Return Loan' function works as expected. Note that the columns are not sortable in this dialog.
    • Make sure that 'Bulk Select' works as expected in all contexts

Please test this as extensively as possible. Let me know if you have any questions.

@grantfitzsimmons grantfitzsimmons requested review from maxpatiiuk and removed request for maxpatiiuk August 12, 2024 14:40
Copy link
Member

@maxpatiiuk maxpatiiuk left a comment

Choose a reason for hiding this comment

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

👍

)
// Change to use an object for selected state allowing null values
const [selected, setSelected] = useLiveState<{
[key: string]: number | null;
Copy link
Member

Choose a reason for hiding this comment

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

(optional) instead of null, could you refactor the code you added in this file to use undefined? that would be more consistent with what we use in the rest of the codebase

return newSelected;
});
} else {
const count = parseInt(newCount, 10);
Copy link
Member

Choose a reason for hiding this comment

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

hmm, the Input.Integer is supposed to call onValueChange with number not string as far as I remember
this this parseInt here and the if (newCount === '') { check above is not right 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Dev Attention Needed
Development

Successfully merging this pull request may close these issues.

Allow sorting items in the create loan dialog
2 participants