Skip to content

Refactor AffiliateLinks to use async partials with parallelized fetching#12292

Closed
bhardwajparth51 wants to merge 10 commits intointernetarchive:masterfrom
bhardwajparth51:12271/refactor/affiliate-links
Closed

Refactor AffiliateLinks to use async partials with parallelized fetching#12292
bhardwajparth51 wants to merge 10 commits intointernetarchive:masterfrom
bhardwajparth51:12271/refactor/affiliate-links

Conversation

@bhardwajparth51
Copy link
Copy Markdown
Contributor

Refactor AffiliateLinks to Remove Template-Side Network Requests

Closes #12271

refactor Moves high-latency price fetching from the template layer to an asynchronous FastAPI partial.

Technical

  • Backend Concurrency: Optimized AffiliateLinksPartial with asyncio.gather for parallel Better World Books and Amazon API metadata fetches.
  • Lazy Loading: Refactored affiliate-links.js to use IntersectionObserver, deferring network requests until the "Buy" section is visible.
  • Architectural Cleanup: Consolidated duplicated store logic from AffiliateLinks.html into a shared get_affiliate_stores helper in vendors.py.
  • Robustness: Added try-except guards for h.affiliate_id to ensure vendors.py functions work correctly in non-web contexts (tests/CLI).

Testing

  1. Navigate to a book edition page (e.g., /books/OL7353617M).
  2. Scroll down towards the "Buy" section.
  3. Verify that a loading spinner appears briefly and is replaced by the affiliate links once fetched.
  4. Check the browser's Network tab to confirm a request to /partials/AffiliateLinks.json.
  5. Run backend logic tests: pytest openlibrary/tests/core/test_vendors_async.py.

Screenshot

Screenshot from 2026-04-05 19-02-40 Screenshot from 2026-04-05 19-02-31

Stakeholders

@RayBB


@github-project-automation github-project-automation Bot moved this to Waiting Review/Merge from Staff in Ray's Project Apr 5, 2026
@bhardwajparth51 bhardwajparth51 force-pushed the 12271/refactor/affiliate-links branch from 125f642 to 98811f3 Compare April 5, 2026 20:35
@bhardwajparth51 bhardwajparth51 force-pushed the 12271/refactor/affiliate-links branch from 45a328d to 1f95c24 Compare April 5, 2026 20:37
@bhardwajparth51 bhardwajparth51 force-pushed the 12271/refactor/affiliate-links branch from 8e62829 to 4f88ffb Compare April 5, 2026 20:44
@bhardwajparth51 bhardwajparth51 force-pushed the 12271/refactor/affiliate-links branch from 60e0cba to 8dcd577 Compare April 5, 2026 20:53
@bhardwajparth51 bhardwajparth51 force-pushed the 12271/refactor/affiliate-links branch from ba8127b to e9afc8d Compare April 5, 2026 20:56
@bhardwajparth51 bhardwajparth51 force-pushed the 12271/refactor/affiliate-links branch from 31d931f to e9c92a0 Compare April 5, 2026 21:01
@bhardwajparth51
Copy link
Copy Markdown
Contributor Author

bhardwajparth51 commented Apr 5, 2026

@RayBB up for a review whenever you have bandwidth

Copy link
Copy Markdown
Collaborator

@RayBB RayBB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bhardwajparth51 thanks for taking a swing at this one.

Currently this PR has too much going on in one change. I think all of the ideas here are good but we should split it up more for easier review and testing.

I realize it wasn't super clear in the issue (the parent issue said it a bit more clearly) I tagged you on but this PR should NOT make the functions async. It should just focus on pulling out the code from the template into a python function that we can call to pass in the data. Doing it in a smaller step like this makes it much easier to review especially since the async code alone is a bit difficult to test.

So please remove the concurrency frontend changes and generally try to minimize the changes for this PR. The frontend change is interesting but I don't know if the added complexity is worth it since it's already high up on the page and loads often.

Thanks!

@github-actions github-actions Bot added the Needs: Response Issues which require feedback from lead label Apr 6, 2026
@RayBB RayBB added Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] and removed Needs: Response Issues which require feedback from lead labels Apr 8, 2026
@bhardwajparth51
Copy link
Copy Markdown
Contributor Author

Hi @RayBB, apologies for the late response. I’ve been caught up with university work lately. Thanks for the feedback, I understand your point about the PR having too many moving parts. I’m currently working on simplifying it and will update the PR soon.

@github-actions github-actions Bot added Needs: Response Issues which require feedback from lead and removed Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] labels Apr 10, 2026
Copy link
Copy Markdown
Collaborator

@RayBB RayBB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the most part this looks correct. However, a few things to fix before I start testing this.

Frontend changes are out of scope for this PR. Please remove them.

affiliate_server_url = None
BWB_AFFILIATE_LINK = 'http://www.anrdoezrs.net/links/{}/type/dlg/http://www.betterworldbooks.com/-id-%s'.format(
h.affiliate_id('betterworldbooks')
BWB_AFFILIATE_LINK_TEMPLATE = (
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain the benefit of making this change? It doesn't seem needed and if it's not please remove it.

Comment on lines +776 to +788
"""Standardizes how we return price info from Better World Books.

:param str qlt: Quality of the book, e.g. "new", "used"
:param str price: Price of the book as a decimal str, e.g. "4.28"
We collect the raw price and quality info here to make it easier
to display in the UI or use for comparison.
"""
price_fmt = f"${price} ({qlt})" if price and qlt else None
try:
bwb_id = h.affiliate_id('betterworldbooks')
except (LookupError, AttributeError):
bwb_id = ''
url = BWB_AFFILIATE_LINK_TEMPLATE.format(bwb_id) % isbn
return {
'url': BWB_AFFILIATE_LINK % isbn,
'url': url,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, is there a reason we need this change?

@RayBB RayBB added Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed] and removed Needs: Response Issues which require feedback from lead labels Apr 12, 2026
@RayBB
Copy link
Copy Markdown
Collaborator

RayBB commented Apr 15, 2026

@bhardwajparth51 thanks for starting this. It has become a high priority change so I'm going to need to take this over. Apologies for that. I'm converting to a draft for now.

@RayBB RayBB marked this pull request as draft April 15, 2026 20:40
@RayBB
Copy link
Copy Markdown
Collaborator

RayBB commented Apr 21, 2026

Done via
#12404
and
#12393

Thanks for you help with this! We build upon your work to move quickly and finish this!

@RayBB RayBB closed this Apr 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Needs: Submitter Input Waiting on input from the creator of the issue/pr [managed]

Projects

Status: Waiting Review/Merge from Staff

Development

Successfully merging this pull request may close these issues.

Refactor AffiliateLinks to Remove Template-Side Network Requests

2 participants