Skip to content

Added test for RC sends circ bib requests to Vega#722

Merged
chrismulholland merged 4 commits intomainfrom
SCC-5292/playwright-circ-bib-redirect
Apr 14, 2026
Merged

Added test for RC sends circ bib requests to Vega#722
chrismulholland merged 4 commits intomainfrom
SCC-5292/playwright-circ-bib-redirect

Conversation

@chrismulholland
Copy link
Copy Markdown
Collaborator

@chrismulholland chrismulholland commented Apr 9, 2026

Ticket:

This PR does the following:

  • Adds a test to redirects.spec.ts for asserting that RC sends circ bib requests to Vega

How has this been tested?

  • all test pass locally

Accessibility concerns or updates

Checklist:

  • I updated the CHANGELOG with the appropriate information and JIRA ticket number (if applicable).
  • I have added relevant accessibility documentation for this pull request.
  • All new and existing tests passed.

@vercel
Copy link
Copy Markdown

vercel bot commented Apr 9, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
research-catalog Ready Ready Preview, Comment Apr 14, 2026 9:25pm

Request Review

maxRedirects: 0,
})

expect(response.status()).toBeGreaterThanOrEqual(300)
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.

Should specifically be a 307, so we should test for that

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

await expect(page).toHaveURL(`${BASE_URL}/browse`)
})

test("RC sends circ bib requests to Vega", async ({ request }) => {
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 move this into a separate describe block– the other redirects in this test suite are set in middleware, which is different than this redirect which happens in the bib API route

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

done


test("RC sends circ bib requests to Vega", async ({ request }) => {
const response = await request.get(`${BASE_URL}/bib/b17782484`, {
maxRedirects: 0,
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.

what's the intention of this flag?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

it's there so Playwright doesn't follow the redirect, but just verifys the redirect response itself (status and location header)

CHANGELOG Outdated

### Added

- Added Playwright test for 'RC sends circ bib requests to Vega' [SCC-5292](https://newyorkpubliclibrary.atlassian.net/browse/SCC-5292)
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.

total nit but this case doesn't need to be in quotes, should follow pattern of other changelog entries ("Added Playwright test for redirecting circ bib requests to Vega")

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

fixed

Copy link
Copy Markdown
Collaborator

@7emansell 7emansell left a comment

Choose a reason for hiding this comment

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

Looks good just one final nit about how we want to describe these test blocks!

await expect(page).toHaveURL(`${BASE_URL}/browse`)
})

test.describe("circ bib redirects", () => {
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 add a describe block around the other tests ("middleware" ?) and rename this one more accurately ("bib request redirects"?)

@chrismulholland chrismulholland merged commit f181180 into main Apr 14, 2026
6 checks passed
@chrismulholland chrismulholland deleted the SCC-5292/playwright-circ-bib-redirect branch April 14, 2026 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants