Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

insights: graphql query schema to support insights based on a search query#39518

Merged
chwarwick merged 5 commits into
mainfrom
cw/query-insight-preview
Jul 27, 2022
Merged

insights: graphql query schema to support insights based on a search query#39518
chwarwick merged 5 commits into
mainfrom
cw/query-insight-preview

Conversation

@chwarwick

Copy link
Copy Markdown
Contributor

Adds a new query that will eventually support some just in time insights that are based off a search query. This PR adds only the graphql schema and hard codes the result to the QueryInsightsNotAvailable type.

related to https://github.com/sourcegraph/sourcegraph/issues/39222

Test plan

query

query{
  queryInsights(query:"TEST QUERY"){
    __typename
  }
}

result:

{
  "data": {
    "queryInsights": {
      "__typename": "QueryInsightsNotAvailable"
    }
  }
}

@chwarwick chwarwick requested review from a team and vovakulikov July 27, 2022 16:43
@cla-bot cla-bot Bot added the cla-signed label Jul 27, 2022
@sourcegraph-bot

sourcegraph-bot commented Jul 27, 2022

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff 2531c58...3c24e0a.

Notify File(s)
@sourcegraph/code-insights-backend cmd/frontend/graphqlbackend/insights.go
enterprise/internal/insights/resolvers/disabled_resolver.go
enterprise/internal/insights/resolvers/resolver.go
enterprise/internal/insights/resolvers/search_query_insights_resolver.go

"""
30 day percentage change in result count rounded to nearest whole number
"""
thirtyDayPercentChange: Int!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If we calculate the percentage in the insights backend we will be unable to use the result count number that comes from the initial search. I think that for the purposes of a prototype this is fine, but I think long term we need to be thinking about minimizing the impact of this on search performance.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was assuming I was going to need to modify the query to include a count:99999 so that we get a full count but if it was less than 500 it could certainly be reused. In that case I could instead provide a 30 day old count or a resultCount(daysAgo int).

"""
QueryInsights are Insights that are available based on a search query. This type is experimental and should be considered unstable in the API.
"""
type QueryInsights {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This might make more sense named SearchQueryInsights, but it's not a huge deal.

@vovakulikov vovakulikov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

GQL schema seems legit from the frontend perspective

@chwarwick chwarwick merged commit 3b7722c into main Jul 27, 2022
@chwarwick chwarwick deleted the cw/query-insight-preview branch July 27, 2022 18:24
leonore added a commit that referenced this pull request Aug 17, 2022
leonore pushed a commit that referenced this pull request Aug 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants