search

wellcomecollection / catalogue-api

:crystal_ball: The API for searching the Wellcome Collection catalogue.
https://developers.wellcomecollection.org
MIT License
3 stars 0 forks source link

guard against getting stuff out of an empty list #760

Closed paul-butcher closed 4 months ago

paul-butcher commented 4 months ago

Fixes https://github.com/wellcomecollection/catalogue-api/issues/759

This is a fairly quick fix, to stop these errors happening, but I would like us to revisit this section of the codebase again soon.

Although the fix was pretty simple to write, the only existing tests that exercise this function are a bit unwieldy and end-to-endy. updateItems is not very testable and requires a load of context manager setup and boilerplate, when it would be preferable to break it up into a function that takes some inputs and produces some outputs. Then it would be much simpler to define a more comprehensive suite of tests that exercise all the possibilities.

kenoir commented 4 months ago

whoops :( So we're sending back items without their location then? I've seen it on wc.org i think, it looks like it's loading forever

I think i've seen this too.

paul-butcher commented 4 months ago

Yes, on some occasions, we are getting items that are either missing locations, or locations that are missing accessConditions.

I don't really know what the actual root cause of this is. I noticed that some of these alerts were for records that should have items, and all of the examples I looked at on the website behaved as I expected when I looked. So I wonder if there is something ephemeral elsewhere that is sending dodgy data, but this needs to be guarded against here anyway.