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

New Lookup PORO #2679

Merged
merged 40 commits into from
Jan 28, 2025
Merged

New Lookup PORO #2679

merged 40 commits into from
Jan 28, 2025

Conversation

nimmolo
Copy link
Contributor

@nimmolo nimmolo commented Jan 26, 2025

Lookup is a new class of POROs to flexibly parse index filtering params. This was previously handled by instance methods inside Query::Modules::LookupObjects and LookupNames. Moving this to a PORO makes these lookups available to model scopes and tests, independently testable, and hopefully easier to debug.

These lookups are used in searches like "Observations by (given) Users" "Observations for (given) Projects". We can filter more than one project at a time, for example "NEMF 2023" and "NEMF 2024". Query needs the IDs, though (as would ActiveRecord), so the lookup handler makes the param format flexible for the caller. With projects, we can pass strings or ids, and Query will make sense of them. projects: ["NEMF 2024", 234]. With a users filter, the choices are instances or ids: users: [users(:mary), 4567432].

The new class Lookup is intended as a more "omnivorous" looker-upper of records. It can handle any identifiers we're likely to throw at it: a string, ID, instance, or a mixed array of any of those. The lookup_method has to be configured in the Lookup child class, because the lookup column names are different for each model.

A Lookup instance has accessor methods that return either an array of ids, instances or titles of the records matched.

fred_ids = Lookup::Users.new(["Fred", "Freddie", "Freda", "Anni Frid"]).ids

Query already has LookupObjects and LookupNames modules that do something very much like this, but they don't handle as many cases: Query param configurations handle either names+ids or instances+ids. After this PR, it will be possible to adjust Query to handle all three in all cases, allowing us to consolidate duplicative params like project/projects that make the class more confusing.

This is intended to be an "omnivorous" looker-upper, that can handle a single string, ID, or instance, or a mixed array of those.

It has methods to return ids or instances, or in the case of names, subtaxa/synonymy subsets for testing.
@coveralls
Copy link
Collaborator

coveralls commented Jan 26, 2025

Coverage Status

coverage: 93.368% (-0.01%) from 93.38%
when pulling defe252 on nimmo-lookups-PORO
into 14e2cc0 on main.

@nimmolo nimmolo changed the title Nimmo lookups poro New Lookup PORO Jan 26, 2025
@nimmolo nimmolo marked this pull request as ready for review January 27, 2025 08:47
@nimmolo nimmolo requested a review from JoeCohen January 27, 2025 09:43
@nimmolo nimmolo requested a review from mo-nathan January 27, 2025 09:43
@nimmolo nimmolo merged commit 2510d12 into main Jan 28, 2025
5 of 8 checks passed
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.

2 participants