NYPL / discovery-api

Discovery API: the JSON-LD API supporting NYPL Shared Collection Catalog
MIT License
6 stars 0 forks source link

Scc 3958/one scholar #359

Closed charmingduchess closed 7 months ago

charmingduchess commented 8 months ago

Incorporates updates to nypl-core and nypl-core-objects to filter out the scholar locations. A scholar with a specific scholar room will only see that one returned. A scholar type without a specific room will return all of the scholar room options.

charmingduchess commented 8 months ago

@nonword Updated to match AC:

I want to refactor this to not use the deliveryLocationTypes value, because scholar ptypes without a scholar room result in a deliveryLocationTypes array that includes 'Scholar'. I didn't realize the expected behavior was to NOT return any scholar rooms for these, so I ended up editing my work to essentially override that. Not sure if that refactor should just happen now or should we move forward with this functioning code?

nonword commented 8 months ago

I want to refactor this to not use the deliveryLocationTypes value

I'm not sure I know exactly what kind of refactoring you mean, but I do think some simplification is possible (see other comment). I think checking the deliveryLocationTypes value is unavoidable (because only when it includes "Scholar" do you want to match on scholarRoom). What you've written seems fine to me - and could be simplified by removing the onlyDeliverabletToLpaOrSchomburg logic, if you agree with that