Skip to content

v0.28: Changes to /documents #1736

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

Merged
merged 6 commits into from
Jun 30, 2022
Merged

v0.28: Changes to /documents #1736

merged 6 commits into from
Jun 30, 2022

Conversation

guimachiavelli
Copy link
Member

Closes #1700

@guimachiavelli guimachiavelli changed the base branch from master to v0.28 June 16, 2022 11:58
@guimachiavelli guimachiavelli changed the title v0.28: Changes to /documents v0.28: Changes to /documents Jun 16, 2022
@guimachiavelli guimachiavelli marked this pull request as ready for review June 16, 2022 15:50
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

According to the spec, the fields parameter is now also available for the GET /indexes/:uid/documents/:document_uid route.

Copy link
Contributor

@maryamsulemani97 maryamsulemani97 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! 🦋
A few suggestions:

  • Update learn/getting_started/algolia_migration.md line 193: displayedAttributes setting does not impact the displayed fields returned
  • I think we should update the get_documents_1 code sample to show how to use the fields parameter
  • Add a note that fields values are case-sensitive.

@guimachiavelli guimachiavelli added this to the v0.28 milestone Jun 20, 2022
@guimachiavelli
Copy link
Member Author

@maryamsulemani97: thanks for the suggestions. I agree with the second and third, but unless I'm mistaken the attributesToRetrieve mentioned in the line you indicated refers to the search parameter attributesToRetrieve, not /documents's fields.

Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

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

🤘

@maryamsulemani97
Copy link
Contributor

unless I'm mistaken the attributesToRetrieve mentioned in the line you indicated refers to the search parameter attributesToRetrieve, not /documents's fields.

Two rows use attributesToRetrieve, lines 182 and 193. I'm assuming 182 refers to the search parameter

@guimachiavelli
Copy link
Member Author

@maryamsulemani97: hmmmm…I'm not sure why it's there two times—could be an error, or that Algolia does not distinguish between things we do (and thus need two separate names). In any case, the line introducing the table reads (emphasis mine):

The below table compares Algolia's API parameters with the equivalent Meilisearch setting or search parameter.

It explicitly states that this concerns settings and search parameters while fields is only concerned with the documents route. The main goal of this update seems to be to further distance /documents from /search— to the point that fields is now completely independent from displayedAttributes.

So I still think that, since the table is about index settings and search parameters, it should not be affected by changes to the /documents route.

Copy link
Contributor

@maryamsulemani97 maryamsulemani97 left a comment

Choose a reason for hiding this comment

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

🥷

@guimachiavelli guimachiavelli mentioned this pull request Jun 30, 2022
17 tasks
@guimachiavelli
Copy link
Member Author

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 30, 2022

Build succeeded:

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.

v0.28: Document API changes
3 participants