openlibrary
openlibrary copied to clipboard
Make BWB + Amz prices lazy load on editions page
Next Steps
- [ ] Decide on approach (e.g. partials + js to fetch prices?) @jimchamp
- [ ] Needs breakdown on which files to edit
- [ ] Needs instructions for how to test -- for now if BWB doesn't return back data, have the partial endpoint return static/fake data. Ignore amz for now.
Design
Patron should see the affiliate/vendors table but show static "tomb stone" grey placeholders until the amz + bwb data becomes available.
Describe the problem that you'd like solved
Justification: Currently BWB + amazon are expensive network calls which result in slow books page load times in sentry
We want to load this data async to speed up the perf.
Proposal & Constraints
Additional context
Stakeholders
can i work on this?
@mekarpeles Please assign me this issue.
@nick2432 I've assigned you to work on this (sorry for the delay).
Here's some more information about this:
The template for our affiliate links is here. I don't think that this file will need to be modified at all, but it will need to be rendered server-side and returned to the client.
The template for our book page is here. Currently, we store the links as affiliate_links
and render this value twice on the book page. This will need to be updated to instead store the HTML for the "tomb stone" placeholder. You can define a new template function that takes isbn
, asin
, prices
, and edition_key
as parameters and returns a placeholder element that includes data attributes for each parameter. These values will be used to fetch the HTML for the AffiliateLinks
macro.
You'll have to create a new endpoint which will be used to fetch the rendered AffiliateLinks
. This can be added to this file for now. You can model the GET
handler on this handler, which returns partials for another feature. Since AffiliateLinks.html
is macro, the render_template
call will need to be replaced with something like macro.macrostore['AffiliateLinks'](doc, opts)
, where doc is the edition, and opts
is a dict
containing asin
, isbn
, and prices
. You can get a reference to the edition by calling web.ctx.site.get(edition_key)
.
The path for the new endpoint can be something like /affiliate-links/partials
.
Finally, you'll need to replace the placeholder with the HTML for the affiliate links. Make sure to only fetch the links once per book page, and replace both placeholders with the results.
Please assign me to this issue
@jimchamp Please assign me this issue on a first come basis I am the one whom you assign sir.
@tusharv01 happy to assign you, before starting work, can you please post a comment here describing how you will implement the solution and make sure you tag @jimchamp so he can review your approach. Thank you!
@mekarpeles @jimchamp Proposed Implementation Approach:
1. Update AffiliateLinks Template:
- Ensure the AffiliateLinks.html template includes necessary placeholders for BWB and Amazon prices.
2. Update Book Page Template:
- Replace the current rendering of 'affiliate_links' with a placeholder element.
- Create a new template function, render_affiliate_links_placeholder, to generate the placeholder.
3. Create a New Endpoint for AffiliateLinks:
- Add a new endpoint '/affiliate-links/partials' to fetch rendered AffiliateLinks.
- Model the GET handler on an existing handler, using the macro.macrostore['AffiliateLinks'](doc, opts) syntax.
4. Replace Placeholders with HTML:
- In the JavaScript part of book_page_template.html, fetch affiliate links using the new endpoint.
- Replace placeholders with the fetched HTML.
@tusharv01 are you still working on this issue?
@tusharv01 are you still working on this issue?
Yes
@tusharv01, this issue is rising in priority, and is something that we'd like finished relatively soon. When do you expect to open a PR with your solution?
Edit: Just noticed that you don't have the repo forked, so I'm assuming that you are not working on this. Assigning myself.