Skip to content

Conversation

@moseshll
Copy link
Contributor

@moseshll moseshll commented Dec 17, 2025

  • Add playwright tests for the various Bib API endpoints, including 400 and 404 conditions.
  • static/api/volumes.php modified to handle the brevity parameter with an appropriate default even when unset. (May still cause errors on production API, not sure about the PHP 8-adjacent variants we have floating around.)
  • Suppress errors about api/error.tpl (which originate in handlePEARError [index.php]) for API queries that don't conform to a structure the rewrite rules can handle. This PR just bails out but we could consider putting some JSON in a template and pointing to it in the error handler -- if we wanted to provide something more developer-tailored for an API error.
  • 400 errors originating inside the API now return some kind of message as JSON.
  • Not doing anything about 404s (for now) since there's no obvious additional content to return other than the 404 itself.

See also https://github.com/hathitrust/catalog/wiki/Volume-API which has been modified as a companion document to the work on the API tests.

- Right now this just has some tests for successful queries against `htid` and `recordnumber`.
- TODO: handle any errors that make it thru the rewrite rules, in `static/api/volumes.php`
- TODO: provide an `api/error.tpl` file for API queries that don't conform to a structure the rewrite rules can handle.
…ty` params, make sure it is set in `volumes.php` if not set.

- Pass `brevity` parameter to `QObj` initializer so `$brevity` does not have to be flagged as a global variable.
@moseshll moseshll marked this pull request as ready for review December 22, 2025 14:56
@moseshll moseshll requested review from aelkiss and liseli December 22, 2025 15:25
Copy link
Member

@aelkiss aelkiss left a comment

Choose a reason for hiding this comment

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

Documentation and tests look good to me; I haven't tried it to see if there are still any warnings in the PHP error log, though, or that I get a reasonable 404 page.

… was being returned as HTML instead of `application/json`.

  - Fixed in `volumes.php` and tested in playwright.
- Explicitly test emptiness of empty responses.
- Add notes about the forms `records` may take in different JSON structures due to the stupidity of PHP.
@moseshll
Copy link
Contributor Author

@aelkiss this branch has been tested on dev-2 against the example API error URLs (found in the playwright tests) and none of them appear to produce anything in the error logs there.

Copy link
Contributor

@liseli liseli left a comment

Choose a reason for hiding this comment

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

All these changes make sense, I've tested it locally and it works fine.
I've created this wiki to document the volume endpoint.

Please review it and probably update it with additional information after these changes.

Approved it

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.

4 participants