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

Make FOLIO item sort configurable. #2726

Merged
merged 5 commits into from Feb 27, 2023

Conversation

demiankatz
Copy link
Member

This PR adds a config setting to control the sort order of items within holding locations. It defaults to using volumes. Note that this is imperfect, as it uses an ASCII-based sort that will sort "100" before "90", etc. However, even with that limitation, it is an improvement on the completely random order we otherwise get. It could be augmented in future with additional settings to apply custom sorting in VuFind after the data has loaded.

@demiankatz
Copy link
Member Author

@bbusenius, I'm interested to know how you're dealing with this situation at UChicago...

@bbusenius
Copy link
Contributor

Hi @demiankatz, for this we just do a usort before we return everything. If the holding is a serial, we return it in reverse order. It's likely not very efficient but this is something that people were very particular about at our institution. Ours does not give perfect results but it has been deemed good enough for the most part. https://github.com/uchicago-library/vufind/blob/uc-master/module/UChicago/src/UChicago/ILS/Driver/Folio.php#L360

@demiankatz
Copy link
Member Author

Thanks, @bbusenius! I've revised this PR so it now offers two options: you can configure folio_sort to perform a sort on the FOLIO side using native capabilities, or you can configure vufind_sort to do a post-processing sort within VuFind. I think this should address most use cases one way or another. What do you think?

Also note that this changes the way VuFind generates copy numbers so that each holding location has its own numeric sequence; I think this probably makes more sense than the previous approach (though the change comes mostly as a side effect of refactoring rather than as a conscious design decision).

@demiankatz
Copy link
Member Author

@bbusenius, I also wonder if it might be valuable to refactor this very long method a bit so that there's a support method that formats a single item record, and another support record that formats a single holdings record... This will not only improve readability but also enable custom code like yours to be more narrowly targeted. Do you think that's worth doing here, or as a follow-up?

@bbusenius
Copy link
Contributor

I had forgotten that since we use enumeration and copy number for the number field, we don't need to renumber them. I think the new numbering sequence is fine. I really like how this offers both pre and post sort options. I do like the idea of separate support methods for item and holdings formatting but I would probably do it as a follow-up since it's not directly related to the sorting.

@demiankatz
Copy link
Member Author

Thanks, @bbusenius -- if you can give me an approval on this PR, then I can merge this and open a follow-up to refactor as discussed. (Of course, if there's anything here you don't feel comfortable approving, or if you'd like to get more eyes on this first, let me know and we can discuss/ask for more input first).

Copy link
Contributor

@bbusenius bbusenius left a comment

Choose a reason for hiding this comment

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

This looks good to me! Thanks for taking us into consideration with the post sort!

@demiankatz
Copy link
Member Author

Thanks, @bbusenius, I'll get to work on the refactoring as soon as time permits. :-)

@demiankatz demiankatz merged commit 76640bc into vufind-org:dev Feb 27, 2023
@demiankatz demiankatz deleted the folio-item-sort branch February 27, 2023 21:49
EreMaijala pushed a commit to EreMaijala/vufind that referenced this pull request Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants