Skip to content

refactor: pass request prefs explicitly (#13537) - #13608

Open
ElenaGe216 wants to merge 1 commit into
internetarchive:masterfrom
ElenaGe216:13537/refactor/pref-helpers
Open

ElenaGe216 wants to merge 1 commit into
internetarchive:masterfrom
ElenaGe216:13537/refactor/pref-helpers

Conversation

@ElenaGe216

Copy link
Copy Markdown

Closes #13537

refactor

Technical

  • Removes hidden web.input() reads from the three preference helpers.
  • Threads provider_pref through book-page edition selection, lending state, provider-aware macros, and search-result enrichment.
  • Keeps providerPref separate from Solr query parameters, so /search?providerPref=ia retains the existing empty-search behavior.
  • Preserves the empty layout query value as an explicit override of the remembered cookie.
  • Adds no tests, per the issue instructions.

Testing

  • Focused existing suite: 72 passed (upstream code/models, lending, and worksearch).
  • Full local suite with temporary socksio dependency: 6,095 passed. It is not fully green because docker/mockservices is unavailable locally (31 errors) and one existing async expectation does not reproduce under Python 3.14.
  • Project pre-commit hooks passed for all changed files. The CSS and JS hooks were skipped because no CSS or JS changed and their Node hook installation is blocked in this local environment.
  • Verified the three target helpers contain no web.input() calls and checked layout precedence for no value, cookie value, and empty query value.

Screenshot

No visual-layout change; this is request-parameter plumbing only.

Stakeholders

@RayBB

@openlibrary-bot

Copy link
Copy Markdown
Collaborator

Thanks for the contribution, @ElenaGe216 — and welcome, this looks like your first PR to Open Library! 🎉

The linked issue hasn't been triaged yet — triage happens on Mondays and Fridays. There are currently 99 open non-draft PRs ahead of yours.

PR triage checklist (maintainers / Pierre)
  • PR description — not empty; explains what the change does and how to verify it
  • References an issue — PR body contains a #NNN reference
    • Linked issue is triaged — has a Priority: * label (not just Needs: Triage)
    • Linked issue is assigned — has at least one assignee
  • Commit history clean — no WIP/fixup/conflict noise; commit messages are meaningful
  • CI passing — no failing check-runs
  • Test cases present — if the change touches substantive logic, test coverage exists or is explained
  • Proof of testing — PR body includes a description of what was tested, a screenshot, or a video

Note

This comment was automatically generated by Pierre, Open Library's PR first-touch assistant, on behalf of @mekarpeles. This is NOT a code review or sign-off.

@RayBB RayBB self-assigned this Sep 12, 2026
@github-project-automation github-project-automation Bot moved this to Waiting Review/Merge from Staff in Ray's Project Sep 12, 2026
@RayBB
RayBB force-pushed the 13537/refactor/pref-helpers branch from 04a3f84 to 7c7bc1d Compare September 16, 2026 09:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Waiting Review/Merge from Staff

Development

Successfully merging this pull request may close these issues.

Thread pref params: provider_order, ia_host, remembered_layout

4 participants