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

search: expose GitHub/GitLab topics in UI#58927

Merged
stefanhengl merged 8 commits into
mainfrom
sh/expose-topics
Dec 18, 2023
Merged

search: expose GitHub/GitLab topics in UI#58927
stefanhengl merged 8 commits into
mainfrom
sh/expose-topics

Conversation

@stefanhengl

@stefanhengl stefanhengl commented Dec 12, 2023

Copy link
Copy Markdown
Member

Currently, we sync topics from GitHub and GitLab and it is possible to
filter repositories by their topics with the repo:has.topic filter.

However, unlike user-added metadata, the synced topics are not visible in the UI,
neither for repo matches nor on the repo tree page. This means it is impossible
for users to figure out which topics they can filter by.

With this PR we display topics, such as "language" or "golang", in addition to
metadata (EG fruit:banana) for repo matches and on the repo tree page (see screenshots).

Search results
Screenshot 2023-12-15 at 12 44 54

Repository tree page (right panel)
Screenshot 2023-12-15 at 12 45 03

Test plan

Note: Our search language distinguishes between user provided metadata (has.meta) and automatically synced metadata (has.topic).

  • manual testing
    • I checked that clicking on a "topic" badge adds a "has.topic" filter, while clicking on a "metadata" badge add a "has.meta" filter
    • Adding and deleting metadata works like before
    • Topics and metadata are sorted alphabetically
    • Results without metadata or topics are displayed correctly
  • added new unit test

@cla-bot cla-bot Bot added the cla-signed label Dec 12, 2023
This PR adds a resolver for topics and exposes them in the UI.

Currently, we sync topics from GitHub and GitLab and it is possible to
filter repositoroes by their topicss with the `repo:has.topic` filter.

However, the synced topics are not visible in the UI, neither in the
search results nor on the repo tree page.
@stefanhengl stefanhengl marked this pull request as ready for review December 15, 2023 13:34
@sourcegraph-bot

sourcegraph-bot commented Dec 15, 2023

Copy link
Copy Markdown
Contributor

Codenotify: Notifying subscribers in CODENOTIFY files for diff bc35ae5...f9fdc6a.

Notify File(s)
@eseliger internal/database/repos.go
internal/database/repos_test.go
@fkling client/branded/src/search-ui/components/RepoMetadata.tsx
client/branded/src/search-ui/components/RepoSearchResult.tsx
client/shared/src/search/stream.ts
@jtibshirani cmd/frontend/internal/search/search.go
internal/search/streaming/http/events.go
@keegancsmith cmd/frontend/internal/search/search.go
internal/search/streaming/http/events.go

Comment thread internal/database/repos.go Outdated
Comment on lines +376 to +381
metadata, ok, err := unmarshalMetadata(s.logger, typ, metadataBytes)
if err != nil {
return err
} else if ok {
r.Topics = GetTopics(metadata)
}

@camdencheek camdencheek Dec 15, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is a virtual column on the repo table called topics that you can just query for this info. We should use that.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Oops. I didn't see that. That makes things a bit tidier.

@camdencheek camdencheek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks great, thank you!

const showRepoMetadata = enableRepositoryMetadata && !!metadata
const showExtraInfo = result.archived || result.fork || result.private

const metadataTags = !metadata

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think we want to assert with ! here since we're doing a type-safe check with ... ? ... : ..., right? Same with topics below?

<ExtraInfoSectionItemHeader title="Description" tooltip="Synchronized from the code host" />
{repo.description && <Text>{repo.description}</Text>}
</ExtraInfoSectionItem>
<ExtraInfoSectionItem>

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this section only be rendered if the repo has topics? In particular, it seems odd to show a section for this, say, on Bitbucket.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Should this section only be rendered if the repo has topics?

Done

Comment thread cmd/frontend/graphqlbackend/graphqlbackend_test.go Outdated
@stefanhengl stefanhengl merged commit 7da4c4e into main Dec 18, 2023
@stefanhengl stefanhengl deleted the sh/expose-topics branch December 18, 2023 08:12
sourcegraph-release-bot pushed a commit that referenced this pull request Dec 22, 2023
Currently, we sync topics from GitHub and GitLab and it is possible to
filter repositories by their topics with the `repo:has.topic` filter.

However, unlike user-added metadata, the synced topics are not visible in the UI,
neither for repo matches nor on the repo tree page. This means it is impossible
for users to figure out which topics they can filter by.

With this PR we display topics, such as "language" or "golang", in addition to
user-added metadata for repo matches and on the repo tree page.

## Test plan

Note: Our search language distinguishes between user provided metadata (has.meta) and automatically synced metadata (has.topic).

- manual testing
  - I checked that clicking on a "topic" badge adds a "has.topic" filter, while clicking on a "metadata" badge add a "has.meta" filter
  - Adding and deleting metadata works like before
  - Topics and metadata are sorted alphabetically
  - Results without metadata or topics are displayed correctly
- added new unit test

(cherry picked from commit 7da4c4e)
stefanhengl added a commit that referenced this pull request Dec 22, 2023
Relates to #58927

test plan:
just a doc change
stefanhengl added a commit that referenced this pull request Dec 22, 2023
Relates to #58927

test plan:
just a doc change
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.

3 participants