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

Align search API with search page #656

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

adpi2
Copy link
Member

@adpi2 adpi2 commented Feb 16, 2021

This PR deprecates the current api/search endpoint by renaming it api/search-old.

Then it introduces a new api/search endpoint whose query inputs, behavior and response are similar to the search page enpoint.

This new API can be tested on the dev environment: https://index-dev.scala-lang.org

Example:

@adpi2
Copy link
Member Author

adpi2 commented Feb 16, 2021

@romanowski

@adpi2 adpi2 requested a review from bishabosha February 16, 2021 14:31
cli: Boolean
)(project: Project): SearchApi.Project = {
val artifacts = if (cli) project.cliArtifacts.toList else project.artifacts
SearchApi.Project(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we also include defaultArtifact since search API under the hood redirects to it anyway.

Copy link
Member Author

Choose a reason for hiding this comment

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

I have added the defaultArtifact field coming from the Project data class. You can try it here.

This field is not set automatically, it is set manually by the library maintainer. So it can be empty or completely nonsense.

@@ -10,29 +10,33 @@ package object routes {
(
"q" ? "*",
"page".as[Int] ? 1,
"total".as[Int] ? SearchParams.resultsPerPage,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could we also include information about available pages? Currently one needs to call next page until the response is empty (but this is probably not in the scope of this PR).

Copy link
Member Author

Choose a reason for hiding this comment

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

@@ -91,7 +104,8 @@ class SearchApi(
val routes: Route =
pathPrefix("api") {
cors() {
path("search") {
// deprecated endpoint replaced by api/search endpoint
path("search-old") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we instead keep this one and introduce a new endpoint api/search/v2, to not break existing users?

Base automatically changed from master to main March 19, 2021 14:32
@adpi2 adpi2 marked this pull request as draft November 22, 2022 15:51
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