Skip to content

Commit d5cdf14

Browse files
mtsgrdmtsgrd
committed
Remove duplicate checks status and create pr button
1 parent 17f5f1f commit d5cdf14

File tree

6 files changed

+46
-134
lines changed

6 files changed

+46
-134
lines changed

‎apps/desktop/cypress/e2e/review.cy.ts

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -540,7 +540,7 @@ describe('Review - stacked branches', () => {
540540
cy.getByTestId('branch-header', mockBackend.bottomBranchName).should('be.visible').click();
541541

542542
// Both 'create review' buttons should be visible.
543-
cy.getByTestId('create-review-button').should('have.length', 3);
543+
cy.getByTestId('create-review-button').should('have.length', 2);
544544

545545
// Click the bottom branch 'create review' button.
546546
cy.getByDataValue('series-name', mockBackend.bottomBranchName).within(() => {
@@ -796,13 +796,12 @@ describe('Review - stacked branches', () => {
796796
cy.getByTestId('stacked-pull-request-card').within(() => {
797797
cy.getByTestId('pr-status-badge').should('be.visible');
798798
cy.getByDataValue('pr-status', 'open').should('be.visible');
799-
cy.getByTestId('pr-checks-badge').should('be.visible').contains('Checks running');
800799
});
801800

802801
cy.getByTestId('branch-card', mockBackend.topBranchName)
803802
.should('be.visible')
804803
.within(() => {
805-
cy.getByTestId('pr-checks-badge-reduced').should('be.visible');
804+
cy.getByTestId('pr-checks-badge').should('be.visible');
806805
});
807806

808807
cy.wait(
@@ -817,17 +816,16 @@ describe('Review - stacked branches', () => {
817816
cy.getByTestId('stacked-pull-request-card').within(() => {
818817
cy.getByTestId('pr-status-badge').should('be.visible');
819818
cy.getByDataValue('pr-status', 'open').should('be.visible');
820-
cy.getByTestId('pr-checks-badge').should('be.visible').contains('Checks passed');
821819
});
822820

823821
cy.getByTestId('branch-card', mockBackend.topBranchName)
824822
.should('be.visible')
825823
.within(() => {
826-
cy.getByTestId('pr-checks-badge-reduced').should('be.visible');
824+
cy.getByTestId('pr-checks-badge').should('be.visible');
827825
});
828826
});
829827

830-
it('Should fail fast when checking for multiple checks', () => {
828+
it.only('Should fail fast when checking for multiple checks', () => {
831829
const data: CustomChecksData = {
832830
total_count: 2,
833831
check_runs: [
@@ -941,13 +939,12 @@ describe('Review - stacked branches', () => {
941939
cy.getByTestId('stacked-pull-request-card').within(() => {
942940
cy.getByTestId('pr-status-badge').should('be.visible');
943941
cy.getByDataValue('pr-status', 'open').should('be.visible');
944-
cy.getByTestId('pr-checks-badge').should('be.visible').contains('Checks running');
945942
});
946943

947944
cy.getByTestId('branch-card', mockBackend.topBranchName)
948945
.should('be.visible')
949946
.within(() => {
950-
cy.getByTestId('pr-checks-badge-reduced').should('be.visible');
947+
cy.getByTestId('pr-checks-badge').should('be.visible');
951948
});
952949

953950
cy.wait(
@@ -962,16 +959,17 @@ describe('Review - stacked branches', () => {
962959
cy.getByTestId('branch-card', mockBackend.topBranchName)
963960
.should('be.visible')
964961
.within(() => {
965-
cy.getByTestId('pr-checks-badge-reduced').should('be.visible');
962+
cy.getByTestId('pr-checks-badge').should('be.visible');
966963
});
967964

968965
cy.getByTestId('stacked-pull-request-card').within(() => {
969966
cy.getByTestId('pr-status-badge').should('be.visible');
970967
cy.getByDataValue('pr-status', 'open').should('be.visible');
971-
cy.getByTestId('pr-checks-badge')
972-
.should('be.visible')
973-
.contains('Checks failed')
974-
.trigger('mouseover');
975968
});
969+
970+
cy.getByTestId('pr-checks-badge')
971+
.should('be.visible')
972+
.contains('Failed', { timeout: 10000 })
973+
.trigger('mouseover');
976974
});
977975
});

‎apps/desktop/src/components/BranchCard.svelte

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -202,7 +202,6 @@
202202
branchName={pr.sourceBranch}
203203
isFork={pr.fork}
204204
isMerged={pr.merged}
205-
reduced
206205
/>
207206
{/if}
208207
{/if}

‎apps/desktop/src/components/BranchReview.svelte

Lines changed: 17 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,9 @@
11
<script lang="ts">
2-
import BranchReviewButRequest from '$components/BranchReviewButRequest.svelte';
32
import CanPublishReviewPlugin from '$components/CanPublishReviewPlugin.svelte';
43
import PullRequestCard from '$components/PullRequestCard.svelte';
54
import ReviewCreation from '$components/ReviewCreation.svelte';
65
import ReviewCreationControls from '$components/ReviewCreationControls.svelte';
76
import StackedPullRequestCard from '$components/StackedPullRequestCard.svelte';
8-
import { syncBrToPr } from '$lib/forge/brToPrSync.svelte';
9-
import { syncPrToBr } from '$lib/forge/prToBrSync.svelte';
10-
import { StackService } from '$lib/stacks/stackService.svelte';
11-
import { UiState } from '$lib/state/uiState.svelte';
12-
import { TestId } from '$lib/testing/testIds';
13-
import { getContext } from '@gitbutler/shared/context';
14-
import { reactive } from '@gitbutler/shared/reactiveUtils.svelte';
157
import Button from '@gitbutler/ui/Button.svelte';
168
import Modal from '@gitbutler/ui/Modal.svelte';
179
import type { Snippet } from 'svelte';
@@ -32,39 +24,12 @@
3224
3325
let canPublishReviewPlugin = $state<ReturnType<typeof CanPublishReviewPlugin>>();
3426
35-
const stackService = getContext(StackService);
36-
const uiState = getContext(UiState);
37-
const commits = $derived(
38-
stackId ? stackService.commits(projectId, stackId, branchName) : undefined
39-
);
40-
41-
const branchConflicted = $derived(
42-
commits?.current.data?.some((commit) => commit.hasConflicts) || false
43-
);
44-
45-
const allowedToPublishBR = $derived(!!canPublishReviewPlugin?.imports.allowedToPublishBR);
4627
const canPublishPR = $derived(!!canPublishReviewPlugin?.imports.canPublishPR);
47-
const ctaLabel = $derived(canPublishReviewPlugin?.imports.ctaLabel);
48-
const branchEmpty = $derived(canPublishReviewPlugin?.imports.branchIsEmpty);
49-
50-
const disabled = $derived(branchEmpty || branchConflicted);
51-
const tooltip = $derived(
52-
branchConflicted ? 'Please resolve the conflicts before creating a PR' : undefined
53-
);
5428
5529
let modal = $state<Modal>();
5630
let confirmCreatePrModal = $state<ReturnType<typeof Modal>>();
5731
let reviewCreation = $state<ReturnType<typeof ReviewCreation>>();
5832
59-
syncPrToBr(
60-
reactive(() => prNumber),
61-
reactive(() => reviewId)
62-
);
63-
syncBrToPr(
64-
reactive(() => prNumber),
65-
reactive(() => reviewId)
66-
);
67-
6833
const submitDisabled = $derived(reviewCreation ? !reviewCreation.imports.creationEnabled : false);
6934
</script>
7035

@@ -122,42 +87,23 @@
12287
</Modal>
12388
{/if}
12489

125-
<div class="branch-action">
126-
{#if prNumber || (reviewId && allowedToPublishBR)}
127-
<div class="status-cards">
128-
{#if prNumber && stackId}
129-
<StackedPullRequestCard {projectId} {stackId} {branchName} {prNumber} poll />
130-
{:else if prNumber}
131-
<PullRequestCard {branchName} {prNumber} poll />
132-
{/if}
133-
{#if reviewId && allowedToPublishBR}
134-
<BranchReviewButRequest {reviewId} />
135-
{/if}
136-
</div>
137-
{/if}
138-
139-
{#if branchStatus}
140-
{@render branchStatus()}
141-
{/if}
142-
143-
{#if canPublishPR}
144-
<Button
145-
testId={TestId.CreateReviewButton}
146-
onclick={() => {
147-
uiState.project(projectId).exclusiveAction.set({
148-
type: 'create-pr',
149-
stackId,
150-
branchName
151-
});
152-
}}
153-
kind="outline"
154-
{disabled}
155-
{tooltip}
156-
>
157-
{ctaLabel}
158-
</Button>
159-
{/if}
160-
</div>
90+
{#if prNumber || branchStatus}
91+
<div class="branch-action">
92+
{#if prNumber}
93+
<div class="status-cards">
94+
{#if prNumber && stackId}
95+
<StackedPullRequestCard {projectId} {stackId} {branchName} {prNumber} poll />
96+
{:else if prNumber}
97+
<PullRequestCard {branchName} {prNumber} poll />
98+
{/if}
99+
</div>
100+
{/if}
101+
102+
{#if branchStatus}
103+
{@render branchStatus()}
104+
{/if}
105+
</div>
106+
{/if}
161107

162108
<style lang="postcss">
163109
.branch-action {

‎apps/desktop/src/components/ChecksPolling.svelte

Lines changed: 18 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@
1212
hasChecks?: boolean;
1313
isFork?: boolean;
1414
isMerged?: boolean;
15-
reduced?: boolean;
1615
};
1716
1817
type StatusInfo = {
@@ -24,7 +23,7 @@
2423
tooltip?: string;
2524
};
2625
27-
let { branchName, isFork, isMerged, hasChecks = $bindable(), reduced = false }: Props = $props();
26+
let { branchName, isFork, isMerged, hasChecks = $bindable() }: Props = $props();
2827
2928
const [forge] = inject(DefaultForgeFactory);
3029
@@ -151,35 +150,20 @@
151150
});
152151
</script>
153152

154-
{#if !reduced}
155-
<div data-testid={TestId.PRChecksBadge}>
156-
<Badge
157-
icon={checksTagInfo.icon}
158-
style={checksTagInfo.style}
159-
kind={checksTagInfo.icon === 'success-small' ? 'solid' : 'soft'}
160-
tooltip={checksTagInfo.tooltip}
161-
onclick={() => {
162-
checksService?.fetch(branchName, { forceRefetch: true });
163-
}}
164-
>
165-
{checksTagInfo.text}
166-
</Badge>
167-
</div>
168-
{:else}
169-
<Badge
170-
testId={TestId.PRChecksBadgeReduced}
171-
size="icon"
172-
icon={checksTagInfo.icon}
173-
style={checksTagInfo.style}
174-
kind={checksTagInfo.icon === 'success-small' ? 'solid' : 'soft'}
175-
tooltip={checksTagInfo.tooltip}
176-
reversedDirection
177-
onclick={() => {
178-
checksService?.fetch(branchName, { forceRefetch: true });
179-
}}
180-
>
181-
<span class="truncate">
182-
{checksTagInfo.reducedText}
183-
</span>
184-
</Badge>
185-
{/if}
153+
<Badge
154+
testId={TestId.PRChecksBadge}
155+
size="icon"
156+
icon={checksTagInfo.icon}
157+
style={checksTagInfo.style}
158+
kind={checksTagInfo.icon === 'success-small' ? 'solid' : 'soft'}
159+
tooltip={checksTagInfo.tooltip}
160+
reversedDirection
161+
onclick={(e) => {
162+
checksService?.fetch(branchName, { forceRefetch: true });
163+
e.stopPropagation();
164+
}}
165+
>
166+
<span class="truncate">
167+
{checksTagInfo.reducedText}
168+
</span>
169+
</Badge>

‎apps/desktop/src/components/PullRequestCard.svelte

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
<script lang="ts">
2-
import ChecksPolling from '$components/ChecksPolling.svelte';
32
import PrStatusBadge from '$components/PrStatusBadge.svelte';
43
import PullRequestPolling from '$components/PullRequestPolling.svelte';
54
import { writeClipboard } from '$lib/backend/clipboard';
@@ -195,19 +194,6 @@
195194
<PrStatusBadge testId={TestId.PRStatusBadge} {pr} />
196195
</div>
197196
<div class="text-12 pr-row">
198-
{#if !pr.closedAt && forge.current.checks}
199-
<div class="factoid">
200-
{#if pr.state === 'open'}
201-
<ChecksPolling
202-
branchName={pr.sourceBranch}
203-
isFork={pr.fork}
204-
isMerged={pr.merged}
205-
bind:hasChecks
206-
/>
207-
{/if}
208-
</div>
209-
<span class="seperator">•</span>
210-
{/if}
211197
<div class="factoid">
212198
{#if pr.reviewers.length > 0}
213199
<span class="label">Reviewers:</span>

‎apps/desktop/src/lib/testing/testIds.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,6 @@ export enum TestId {
8585
StackedPullRequestCard = 'stacked-pull-request-card',
8686
PRStatusBadge = 'pr-status-badge',
8787
PRChecksBadge = 'pr-checks-badge',
88-
PRChecksBadgeReduced = 'pr-checks-badge-reduced',
8988
BranchView = 'branch-view',
9089
HunkContextMenu = 'hunk-context-menu',
9190
HunkContextMenu_SelectAll = 'hunk-context-menu-select-all',

0 commit comments

Comments
 (0)