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

Design doc: new search API #9533

Merged
merged 6 commits into from
Nov 17, 2022
Merged

Design doc: new search API #9533

merged 6 commits into from
Nov 17, 2022

Conversation

stsewd
Copy link
Member

@stsewd stsewd commented Aug 22, 2022

@stsewd stsewd requested a review from a team as a code owner August 22, 2022 21:21
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

I should have read the whole document before commenting, sorry! I really like the syntax approach here, but I do want to have a larger vision for our overall search across the whole product. I think there's a lot of good ideas here, but we should address site search as well with this work.

docs/dev/design/new-search-api.rst Show resolved Hide resolved
docs/dev/design/new-search-api.rst Outdated Show resolved Hide resolved
docs/dev/design/new-search-api.rst Show resolved Hide resolved
docs/dev/design/new-search-api.rst Show resolved Hide resolved
docs/dev/design/new-search-api.rst Outdated Show resolved Hide resolved
docs/dev/design/new-search-api.rst Show resolved Hide resolved
docs/dev/design/new-search-api.rst Show resolved Hide resolved
docs/dev/design/new-search-api.rst Outdated Show resolved Hide resolved
Is this feature useful? Should we implement
this as a way to filter projects from https://readthedocs.org/dashboard/ instead?
This will be using just https://docs.djangoproject.com/en/4.0/ref/contrib/postgres/search/
or ``contains``.
Copy link
Member

Choose a reason for hiding this comment

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

I think this is probably a good solution. Having project search just use the DB, and then ES is only for searching files. I'd want to have a different UX even for it though, if it's not using the same backend...

@ericholscher
Copy link
Member

@stsewd I'd love to see an update of this document that really focuses on a unified search experience. I feel pretty strongly that that's the next major project we should do with search, basically bringing the live search the dashboard and to docs in a nicer way.

I think the syntax you're proposing is great, and gives us a lot of flexibility around how to expose this to the user.

@stsewd
Copy link
Member Author

stsewd commented Aug 26, 2022

I'd love to see an update of this document that really focuses on a unified search experience

@ericholscher do you have a preference on #9533 (comment)?

}
},
{
"type": "domain",
Copy link
Member Author

Choose a reason for hiding this comment

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

We could also maybe just drop domains from the new API (#9571), but that decision can be made later, since we would just stop returning these type of blocks.

Comment on lines +231 to +232
Another approach could be linking to the global search
with ``project:{project.slug}`` filled in the query.
Copy link
Member Author

Choose a reason for hiding this comment

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

If we do this, we could do something similar to github, having a global search bar, and offering the scopes depending if you are on a project/organization or not.

Screenshot from 2022-08-31 19-02-43

Copy link
Member

Choose a reason for hiding this comment

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

Yea, that is what I'm imagining. Basically we just prepend the project slug in project search mode, but the same UX can be used globally.

Copy link
Member Author

Choose a reason for hiding this comment

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

So, are we okay for now to just redirect to the global search with the filter set when clicking on Search in

Screenshot 2022-09-01 at 12-50-19 docs Read the Docs

or do we want to have the new search bar together with this implementation?

Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

These are good updates 💯

Comment on lines +231 to +232
Another approach could be linking to the global search
with ``project:{project.slug}`` filled in the query.
Copy link
Member

Choose a reason for hiding this comment

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

Yea, that is what I'm imagining. Basically we just prepend the project slug in project search mode, but the same UX can be used globally.

~~~~~~~~~~~~~~~~~~~~~~~~

We can keep the project search as is,
without using the new syntax (since it doesn't make sense there).
Copy link
Member

Choose a reason for hiding this comment

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

I think for .org, we probably want some kind of "explore" functionality. It's not a huge priority, but I think that's the natural outcome of project search & tag data. But outside the scope of this document.

What does "as is" mean here? Is it the same search bar? A different one?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the same search bar, as an option

Screenshot 2022-09-01 at 12-25-37 Search docs Read the Docs

~~~~~~~~~~~~~~~~~~~~~

Using the same syntax from the API will be allowed,
by default it will search all projects in .org,
Copy link
Member

Choose a reason for hiding this comment

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

I think we should switch this to searching your projects by default on .org too, but with an obvious for global?

Copy link
Member Author

Choose a reason for hiding this comment

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

We can start the search with the user:@me filter set, and if you want a global search, you just remove that filter.

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good 👍

Another approach could be to allow
filtering by user on .org, this is ``user:stsewd`` or ``user:@me``
so a user can search all their projects easily.
We could allow just ``@me`` to start.
Copy link
Member

Choose a reason for hiding this comment

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

This is a neat idea. This is the start of something like collections #3784 that I've talked about for a long time, if we added tag: filtering, for example. We probably want org: filtering too.

Copy link
Member

Choose a reason for hiding this comment

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

These would be definitely great additions! 💯

docs/dev/design/new-search-api.rst Show resolved Hide resolved
docs/dev/design/new-search-api.rst Show resolved Hide resolved
docs/dev/design/new-search-api.rst Outdated Show resolved Hide resolved
@ericholscher
Copy link
Member

I think this mostly looks good to me 👍 Time to start trying to build it, and break the JS from the livesearch out 👍

This was referenced Sep 21, 2022
@ericholscher
Copy link
Member

@stsewd if this is ready, we should merge it, since we're actively implementing it.

Copy link
Member

@humitos humitos left a comment

Choose a reason for hiding this comment

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

This document looks good to me. I'd love to see the updates from comments reflected into the document and get merged.

I also asked some questions that I'd like some answers. None of them are blocking, I guess.

Comment on lines +82 to +86
``subprojects:project/version`` (exclusive)
This is the same as the above,
but it doesn't include the parent project in the results.
If we want to include the results from the project, then
the query will be ``project:project/latest subprojects:project/latest``.
Copy link
Member

Choose a reason for hiding this comment

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

I like this pattern. It's pretty explicit what the user wants:

  • project:docs/latest: won't include subprojects at all
  • subprojects:docs: will include only results from all the subprojects from the docs project
  • project:docs/latest subprojects:docs: will include results from docs and all its subprojects

BTW, I would name it subproject (singular) to match the other key project and avoid confusions.

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'm implementing the inclusive option of subprojects, but I'm also fine with the exclusive option. @ericholscher thoughts? I did the first because it mimics our current behavior.

I would name it subproject (singular) to match the other key project and avoid confusions.

I think the plural version is more explicit, since the relationship is one to many, and that's the word we use in our code base/documentation.

Copy link
Member

Choose a reason for hiding this comment

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

Plural is the correct, yeah. I re-read it today and it makes sense. In fact, it could also be subprojects_of if we want to be more explicit about its meaning.

docs/dev/design/new-search-api.rst Show resolved Hide resolved
docs/dev/design/new-search-api.rst Show resolved Hide resolved
docs/dev/design/new-search-api.rst Show resolved Hide resolved
docs/dev/design/new-search-api.rst Outdated Show resolved Hide resolved
Another approach could be to allow
filtering by user on .org, this is ``user:stsewd`` or ``user:@me``
so a user can search all their projects easily.
We could allow just ``@me`` to start.
Copy link
Member

Choose a reason for hiding this comment

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

These would be definitely great additions! 💯

@stsewd
Copy link
Member Author

stsewd commented Nov 17, 2022

If there is anything else to keep discussing, we can comment in the implementation PR.

@stsewd stsewd merged commit fc7f68b into main Nov 17, 2022
@stsewd stsewd deleted the design-doc-new-search-api branch November 17, 2022 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants