Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Indicate the the 'observation search' request takes pagination parameters. #389

Closed
wants to merge 1 commit into from

Conversation

frewsxcv
Copy link
Contributor

@frewsxcv frewsxcv commented Jul 27, 2023

Assuming I'm reading the source correctly, this request does honor these parameters:

page: Joi.number( ).integer( ),
per_page: Joi.number( ).integer( ),
order: Joi.string( ).valid(
"desc",
"asc"
),
order_by: Joi.string( ).valid(
"observed_on",
"updated_at",
"species_guess",
"votes",
"id",
"created_at",
"random"
),

@frewsxcv frewsxcv changed the title Indicate the the 'observation search' request takes pagination parame… Indicate the the 'observation search' request takes pagination parameters. Jul 27, 2023
@frewsxcv
Copy link
Contributor Author

frewsxcv commented Jul 30, 2023

Hmm this might not be this simple, since the 'observation observers' endpoint also has type = search:

/observations/observers:
get:
summary: Observation Observers
description: |
Given zero to many of following parameters, returns observers of
observations matching the search criteria and the count of
observations and distinct taxa of rank `species` they have observed. A
maximum of 500 results will be returned
parameters:
<%- include( "_observation_search_params_v1.yml.ejs", { type: "search" } ) %>

...but has a different set of expected parameters:

order_by: Joi.string( ).valid(
"observation_count",
"species_count"
).default( "observation_count" )

@pleary
Copy link
Member

pleary commented Sep 12, 2023

Could you please be more clear about what the problem is that this PR is attempting to fix? The title says Indicate the the 'observation search' request takes pagination parameters, but the documentation for observation search in both API v1 and v2 do include pagination parameters in their swagger documentation, e.g. https://api.inaturalist.org/v1/docs/#!/Observations/get_observations and https://api.inaturalist.org/v2/docs/#/Observations/get_observations.

Specifically what endpoints (including API version) don't document pagination parameters but should?

@frewsxcv
Copy link
Contributor Author

@pleary If I recall correctly, I ran into this when using the /observations/species_count endpoint, which inherits much of the logic from observation search but doesn't include the pagination parameters

@frewsxcv
Copy link
Contributor Author

And agreed the way I worded the original issue was not clear, sorry about that!

pleary added a commit that referenced this pull request Sep 14, 2023
@pleary
Copy link
Member

pleary commented Sep 14, 2023

I pushed a commit fbcb988 where I explicitly add the page and per_page parameters for the species_count endpoint. They are now included in the V1 documentation https://api.inaturalist.org/v1/docs/#!/Observations/get_observations_species_counts . Does that look like it resolves the issue at hand here?

We have several endpoints that share a lot of the same parameters for filtering data, but not some of the more usability oriented parameters like ordering and sorting. OpenAPI/swagger doesn't have a great way of supporting the idea of reusable subsets of parameters, so the implementation here isn't as clean as I'd prefer. Thanks for making the pull request and raising the issue about the undocumented parameters!

@frewsxcv
Copy link
Contributor Author

Perfect! Thank you so much @pleary 🌻

@frewsxcv frewsxcv closed this Sep 15, 2023
@frewsxcv frewsxcv deleted the patch-1 branch September 15, 2023 05:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants