Add an endpoint for image usages by supplier - #4879
Conversation
6a59f46 to
683d09a
Compare
36f5cb4 to
d8925be
Compare
5f2b474 to
732441d
Compare
|
|
||
| val query = boolQuery().must(matchAllQuery()).filter(boolQuery().must(beSupplier, haveQualifyingUsage)) | ||
|
|
||
| val search = prepareSearch(query).trackTotalHits(true).from(offset).size(length) |
There was a problem hiding this comment.
There is an upper bound to consider for elasticsearch pagination - beyond a certain limit performance drops off a cliff, so it might be good to sanitise the offset value unless that's taken care of elsewhere. See https://www.elastic.co/docs/reference/elasticsearch/rest-apis/paginate-search-results
There was a problem hiding this comment.
It might also be that you'll never get anywhere close to the limit in this context.
There was a problem hiding this comment.
Within the API endpoint I'm limiting length to the maxSize set in SearchParams which is 200. I could enforce this check within this function too
Edit: added :)
There was a problem hiding this comment.
Sorry I misunderstood your point entirely! That's a useful callout for sure. I've tweaked how search params are validated and included that.
We don't seem to have that guard on the images search though, unless I'm missing something? Curious how we'd handle that 👀
There was a problem hiding this comment.
I'm not really familiar with Grid enough myself yet. If there is already some paging going on, then it's likely to be fine.
I just know that in CAPI we have to have a hard limit on how deeply we can page and it's waaay less that 10K. The change you've made sets the default cap limit but in practice the cluster's actual performance may dictate a lower value.
I'd have to defer all this to anyone who's closer to the code for the final approval. This was the only thing that caught my eye.
There was a problem hiding this comment.
I think a cursor-based pagination would bypass this problem. But I don't really want to introduce a new pattern. As this behaves pretty similarly as offset-based paginated image search, keeping the 10k limit seems to be the simple safeguard. Realistically we have ~6000 unique images for the highest-usage supplier in a 30-day window which is what we're interested in (the rest are more around or under 1000)
732441d to
b24f7dd
Compare
hk-15
left a comment
There was a problem hiding this comment.
I have been playing around with this and as far as I can see it is all working as it should 🎉
bc5498c to
6253922
Compare
6253922 to
1529498
Compare
|
Overdue on auth, usage, image-loader, metadata-editor, thrall, leases, cropper, collections, media-api, kahuna (merged by @junyuanxue 30 minutes and 3 seconds ago) What's gone wrong? |
|
Seen on auth, usage, image-loader, metadata-editor, thrall, leases, cropper, collections, media-api, kahuna (merged by @junyuanxue 51 minutes and 1 second ago) Please check your changes! |

What does this change?
Adds an endpoint
/usage/suppliers/:id/images, which returns a paginated list of images along with its relevant usages for each agency.We only care about usages that are under
published,unknown, andremovedstatuses for now, ondigitalandprintplatforms.I've allowed for usage
addeddate range to be passed in via search params to give us more flexibility around investigating various date ranges and compare against Kevin's RCS email, without being limited to the 'last 30 days' logic.This endpoint would be extremely useful for providing some transparency around image usages as we investigate usage reporting in the Grid and quota counting from RCS.
How should a reviewer test this change?
./dev/script/start.sh --use-TESToffsetandlengthin the search params{ "offset": 0, "length": 73, "total": 73, "data": [ { "id": "09f2c303a1b00697a092272156b4b149cb386354", "supplier": "Getty Images", "usages": [ { "id": "front/34efed86f207c04264e9ee2ee2cb4bf4_d9a2c6f3b5c5d77d95dfdfd74b834ea3", "references": [ { "type": "front", "name": "olly" } ], "platform": "digital", "media": "image", "status": "unknown", "dateAdded": "2023-04-27T14:29:21.150Z", "lastModified": "2023-04-27T14:29:21.150Z", "frontUsageMetadata": { "addedBy": "[email protected]", "front": "olly" } } ] }, { "id": "0345c73bf81cafeb11ef8bc1ccc26ff2410a67f4", "supplier": "Getty Images", "usages": [ { "id": "front/2b46ae683500c561863adfb0856324e_a3f8975262b7c4085f9d07aba3876f56", "references": [ { "type": "front", "name": "uk" } ], "platform": "digital", "media": "image", "status": "unknown", "dateAdded": "2023-05-04T08:24:04.750Z", "lastModified": "2023-05-04T08:24:04.750Z", "frontUsageMetadata": { "addedBy": "[email protected]", "front": "uk" } } ] }, // ...etc etc ] }Tested? Documented?