Feature: Label selection when creating merge request #11619
Conversation
|
@rhmndnt is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
rhmndnt
force-pushed
the
feature/forge/label_selection
branch
from
8044334 to
5d36f20
Compare
|
Thanks a lot for tackling this, it's much appreciated! While CI is failing, I thought it's best to keep this as draft until you think it's ready as well. Something that would also help the review is a video showing the feature in action, maybe with the network tab of the dev-tools open to get an idea of the requests during a typical interaction. In any case, I wouldn't be able to review this, but will help finding someone who can once the PR approaches a ready-to-review state. |
There was a problem hiding this comment.
Pull request overview
This pull request adds label selection functionality when creating merge/pull requests, allowing users to assign labels to their MRs/PRs directly from the GitButler interface.
Key Changes:
- Added a new
LabelSectioncomponent for selecting and managing labels in the PR creation flow - Extended both GitHub and GitLab forge services to support fetching repository labels and including them in PR/MR creation
- Modified the
Selectcomponent to optionally keep the dropdown open after selection via acloseOnSelectprop
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
packages/ui/src/lib/components/select/Select.svelte |
Added closeOnSelect prop to allow multi-select behavior without closing the dropdown |
apps/desktop/src/lib/state/clientState.svelte.ts |
Disabled refetchOnFocus for all forge APIs to prevent unnecessary refetching |
apps/desktop/src/lib/forge/interface/types.ts |
Added labels array to CreatePullRequestArgs type |
apps/desktop/src/lib/forge/interface/forgePrService.ts |
Added labels() method to the ForgePrService interface |
apps/desktop/src/lib/forge/gitlab/gitlabPrService.svelte.ts |
Implemented label fetching and inclusion in MR creation for GitLab |
apps/desktop/src/lib/forge/github/githubPrService.svelte.ts |
Implemented label fetching and inclusion in PR creation for GitHub |
apps/desktop/src/components/ReviewCreation.svelte |
Integrated LabelSection component and passed selected labels to PR creation |
apps/desktop/src/components/LabelSection.svelte |
New component providing label selection UI with filtering and badge display |
| })) | ||
| }; | ||
| }, | ||
| providesTags: [ReduxTag.PullRequests] |
There was a problem hiding this comment.
The getLabels query is tagged with ReduxTag.PullRequests, which means invalidating pull requests will also invalidate the labels cache. This could cause unnecessary refetching of labels. Consider using a dedicated tag like ReduxTag.Labels or using providesType(ReduxTag.PullRequests) instead to avoid unintended cache invalidation when individual PRs are updated.
| providesTags: [ReduxTag.PullRequests] | |
| providesTags: [ReduxTag.Labels] |
| return { error: toSerializable(e) }; | ||
| } | ||
| }, | ||
| providesTags: [ReduxTag.GitLabPullRequests] |
There was a problem hiding this comment.
The getLabels query is tagged with ReduxTag.GitLabPullRequests, which means invalidating pull requests will also invalidate the labels cache. This could cause unnecessary refetching of labels. Consider using a dedicated tag like ReduxTag.Labels or using providesType(ReduxTag.GitLabPullRequests) instead to avoid unintended cache invalidation when individual MRs are updated.
| providesTags: [ReduxTag.GitLabPullRequests] | |
| providesTags: [ReduxTag.Labels] |
| const { api, upstreamProjectId } = gitlab(query.extra); | ||
| const labels = await api.ProjectLabels.all(upstreamProjectId); | ||
| return { | ||
| data: labels.map((l: any) => ({ |
There was a problem hiding this comment.
The parameter type is typed as any. This should be typed properly to ensure type safety. Based on the GitLab API documentation, labels returned from ProjectLabels.all() should have a specific structure that can be typed.
| return { error: result.error }; | ||
| } | ||
| return { | ||
| data: result.data.map((l: any) => ({ |
There was a problem hiding this comment.
The parameter type is typed as any. This should be typed properly to ensure type safety. Based on the GitHub API documentation, labels returned from listLabelsForRepo should have a specific structure that can be typed.
| invalidationBehavior: 'immediately' as const, | ||
| baseQuery: fakeBaseQuery, | ||
| refetchOnFocus: true, | ||
| refetchOnFocus: false, |
There was a problem hiding this comment.
Disabling refetchOnFocus for all forge APIs is a significant behavioral change that affects more than just labels. This change impacts all GitHub and GitLab API calls (PRs, users, etc.). If the intent is to prevent labels from refetching on focus, consider making this configuration more granular by setting it per-endpoint instead of globally for all forge APIs.
| refetchOnFocus: false, |
2 participants
🧢 Changes
LabelSectionLabelSectionintoReviewCreation☕️ Screenshot
🎫 Affected issues
Fixes: #9982