fix: lazy-resolve Post.sourceUrl and deprecate Post.sourceUrlsBySize
#3214
Conversation
![qlty-cloud-legacy[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
Do we have tests that query for the image sizes already and can at least confirm this is not a regression? We could also add specific unit test(s) that call the function directly but not necessarily execute a GraphQL query like most of our wpunit tests do. |
Haha yes, that's why the test is failing now. Got pulled away but basically returning a generator would be a breaking change for classes extending the Post model, while surfacing a model-level function doesn't fit with current conventions. Probably just needs to be wrapped better. 🤔 |
justlevine
force-pushed
the
fix/lazy-load-sizes
branch
from
28d8d6e to
79685af
Compare
|
Code Climate has analyzed commit 79685af and detected 5 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
justlevine
changed the title
Post.sourceUrlsBySizePost.sourceUrl and deprecate Post.sourceUrlsBySize
|
Callback approach won't work due to how fields are "cached" on the model. Instead I've made the |
|
PS: this is ready once tests are added. |
justlevine
added
the
needs: tests
|
@jasonbahl this needed tests 🫣 |
- tests querying the sourceUrl field and fileSize field on Media Item with various size arguments - unit tests the testGetSourceUrlBySize function
|
@justlevine I just opened this PR with some tests: #3219 Let me know if you think there are other tests we should consider adding. |
3 participants
What does this implement/fix? Explain your changes.
This PR fixes an issue where trying to get a
sourceUrlbysizewould cause the image URLs for all the sizes to resolve. This inevitably leads to slowdowns on sites with lots of registered image sizes (e.g. WooCommerce sites)The fix works by introducing a new
Post::get_source_url_by_size()public method used instead of thesourceUrlsBySizemodel field, which has been deprecated.Does this close any currently open issues?
Discord: https://discord.com/channels/838854618406453300/1282730629305208832/1283796889615073321
Any relevant logs, error output, GraphiQL screenshots, etc?
Any other comments?
Post::sourceUrlsBySizeinto a callback, but that would be a breaking change.sourceUrlandfileSizeresolvers inRegistry\Utils\PostObject::register_attachment_fields(), but that's unaffected by this particular bug/fix.Not entirely sure if/how to test this, since this is an under-the-hood fix.Where has this been tested?
Operating System: Ubuntu 20.04 (wsl2 + devilbox + php 8.1.15)
WordPress Version: 6.6.2