Skip to content

Commit e740c9d

Browse files
knightedcodemonkeyknightedcodemonkey
committed
refactor: close pr flow.
1 parent e1208e9 commit e740c9d

7 files changed

Lines changed: 170 additions & 38 deletions

File tree

‎playwright/github-pr-drawer/active-context-switch.spec.ts‎

Lines changed: 30 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ test('Switching active workspace to closed preserves switched-from record integr
3737
page,
3838
targetState: 'closed',
3939
})
40+
41+
await expect(page.getByRole('button', { name: 'Open pull request' })).toBeVisible()
42+
await expect(
43+
page.getByRole('button', { name: 'Close active pull request context' }),
44+
).toBeHidden()
45+
4046
await expect(page.getByRole('status', { name: 'App status' })).toContainText('Rendered')
4147
})
4248

@@ -274,6 +280,26 @@ test('Switching active workspaces with different module sync paths keeps remote
274280
await expect
275281
.poll(async () => {
276282
const records = await getAllWorkspaceRecords(page)
283+
284+
await expect
285+
.poll(async () => {
286+
const records = await getAllWorkspaceRecords(page)
287+
const closedRecord = records.find(
288+
record =>
289+
record?.repo === 'knightedcodemonkey/develop' &&
290+
record?.prContextState === 'closed' &&
291+
record?.prNumber === 2,
292+
)
293+
294+
return {
295+
prContextState: closedRecord?.prContextState,
296+
prNumber: closedRecord?.prNumber,
297+
}
298+
})
299+
.toEqual({
300+
prContextState: 'closed',
301+
prNumber: 2,
302+
})
277303
const alphaRecord = records.find(record => {
278304
const recordId = typeof record?.id === 'string' ? record.id.trim() : ''
279305
const recordHead = typeof record?.head === 'string' ? record.head.trim() : ''
@@ -925,8 +951,7 @@ test('Active PR context updates controls and can be closed from AI controls', as
925951
).toBeVisible()
926952
await expect(
927953
page.getByRole('list', { name: 'Workspace editor tabs' }).getByRole('listitem'),
928-
).toHaveCount(1)
929-
await expect(page.locator('#preview-host iframe')).toHaveCount(0)
954+
).toHaveCount(2)
930955

931956
await expect
932957
.poll(async () => {
@@ -941,12 +966,15 @@ test('Active PR context updates controls and can be closed from AI controls', as
941966
return {
942967
prContextState: closedRecord?.prContextState,
943968
prNumber: closedRecord?.prNumber,
969+
prTitle: closedRecord?.prTitle,
944970
}
945971
})
946972
.toEqual({
947973
prContextState: 'closed',
948974
prNumber: 2,
975+
prTitle: 'Existing PR context from storage',
949976
})
977+
950978
expect(closePullRequestRequestCount).toBe(1)
951979
})
952980

‎playwright/github-pr-drawer/github-pr-drawer.helpers.ts‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -926,10 +926,7 @@ export const runActiveWorkspaceSwitchIntegrityScenario = async ({
926926
JSON.stringify(snapshot.active) === JSON.stringify(originalSnapshot.active)
927927

928928
const target = snapshot.target
929-
const targetStateMatches =
930-
targetState === 'closed'
931-
? target?.prContextState === 'closed' || target?.prContextState === 'inactive'
932-
: target?.prContextState === expectedTargetPrContextState
929+
const targetStateMatches = target?.prContextState === expectedTargetPrContextState
933930

934931
const targetMatches =
935932
target?.repo === originalSnapshot.target.repo &&

‎src/app.js‎

Lines changed: 47 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -812,15 +812,15 @@ const {
812812
return
813813
}
814814

815+
prDrawerController.clearSelectedRepositoryActivePrContext({ resetForm: false })
816+
815817
const nextWorkspaceRepositoryFullName =
816818
typeof workspace.repo === 'string' ? workspace.repo.trim() : ''
817819
if (nextWorkspaceRepositoryFullName) {
818820
workspaceRepositoryFullName = nextWorkspaceRepositoryFullName
819821
byotControls.setSelectedRepository(nextWorkspaceRepositoryFullName)
820822
}
821823

822-
prDrawerController.clearSelectedRepositoryActivePrContext({ resetForm: false })
823-
824824
const state =
825825
typeof workspace.prContextState === 'string'
826826
? workspace.prContextState.trim().toLowerCase()
@@ -955,6 +955,7 @@ const workspacePrSessionHandoffController = createWorkspacePrSessionHandoffContr
955955
defaults: {
956956
defaultComponentTabName,
957957
defaultComponentTabPath,
958+
defaultComponentTabContent: defaultJsx,
958959
},
959960
state: {
960961
getWorkspacePrNumber: () => workspacePrNumber,
@@ -998,17 +999,6 @@ const workspacePrSessionHandoffController = createWorkspacePrSessionHandoffContr
998999
},
9991000
})
10001001

1001-
const archivePrSessionAndStartFreshLocal = ({ result, archivedState, statusMessage }) => {
1002-
hasObservedActivePrContextInSession = false
1003-
setWorkspacePrNumber(result?.pullRequestNumber)
1004-
byotControls.clearSelectedRepositoryPreference()
1005-
workspaceRepositoryFullName = ''
1006-
workspacePrSessionHandoffController.archivePrWorkspaceAndStartFreshLocal({
1007-
archivedState,
1008-
statusMessage,
1009-
})
1010-
}
1011-
10121002
const onPrContextStateChange = createPrContextStateChangeHandler({
10131003
toNonEmptyWorkspaceText,
10141004
toPullRequestNumber,
@@ -1120,16 +1110,25 @@ const githubWorkflows = createGitHubWorkflowsSetup({
11201110
if (nextPrNumber !== null) {
11211111
setWorkspacePrNumber(nextPrNumber)
11221112
}
1123-
persistWorkspacePrContextState('closed')
1113+
setWorkspacePrContextState('closed')
11241114

11251115
const persistClosedRecords = async () => {
1116+
const activeWorkspaceId = toNonEmptyWorkspaceText(activeWorkspaceRecordId)
1117+
const activeWorkspaceRecord = activeWorkspaceId
1118+
? await workspaceStorage.getWorkspaceById(activeWorkspaceId)
1119+
: null
1120+
const preservedPrTitle =
1121+
toNonEmptyWorkspaceText(activeWorkspaceRecord?.prTitle) ||
1122+
toNonEmptyWorkspaceText(githubPrTitle?.value)
1123+
11261124
await persistClosedPrContextRecords({
11271125
workspaceStorage,
11281126
selectedRepository: toNonEmptyWorkspaceText(
11291127
getCurrentSelectedRepositoryFullName(),
11301128
),
11311129
nextPrNumber,
11321130
normalizedHead: toNonEmptyWorkspaceText(githubPrHeadBranch?.value),
1131+
fallbackPrTitle: preservedPrTitle,
11331132
toNonEmptyWorkspaceText,
11341133
refreshLocalContextOptions,
11351134
})
@@ -1140,11 +1139,40 @@ const githubWorkflows = createGitHubWorkflowsSetup({
11401139
})
11411140
},
11421141
onPrContextClosed: result => {
1143-
archivePrSessionAndStartFreshLocal({
1144-
result,
1145-
archivedState: 'closed',
1146-
statusMessage:
1147-
'PR context closed. Open Workspaces to load a saved workspace or continue with this local workspace.',
1142+
hasObservedActivePrContextInSession = false
1143+
const nextPrNumber =
1144+
toPullRequestNumber(result?.pullRequestNumber) ??
1145+
parsePullRequestNumberFromUrl(result?.pullRequestUrl)
1146+
if (nextPrNumber !== null) {
1147+
setWorkspacePrNumber(nextPrNumber)
1148+
}
1149+
setWorkspacePrContextState('closed')
1150+
1151+
const persistClosedRecords = async () => {
1152+
const activeWorkspaceId = toNonEmptyWorkspaceText(activeWorkspaceRecordId)
1153+
const activeWorkspaceRecord = activeWorkspaceId
1154+
? await workspaceStorage.getWorkspaceById(activeWorkspaceId)
1155+
: null
1156+
const preservedPrTitle =
1157+
toNonEmptyWorkspaceText(result?.prTitle) ||
1158+
toNonEmptyWorkspaceText(activeWorkspaceRecord?.prTitle) ||
1159+
toNonEmptyWorkspaceText(githubPrTitle?.value)
1160+
1161+
await persistClosedPrContextRecords({
1162+
workspaceStorage,
1163+
selectedRepository: toNonEmptyWorkspaceText(
1164+
getCurrentSelectedRepositoryFullName(),
1165+
),
1166+
nextPrNumber,
1167+
normalizedHead: toNonEmptyWorkspaceText(githubPrHeadBranch?.value),
1168+
fallbackPrTitle: preservedPrTitle,
1169+
toNonEmptyWorkspaceText,
1170+
refreshLocalContextOptions,
1171+
})
1172+
}
1173+
1174+
void persistClosedRecords().catch(() => {
1175+
/* Save failures are already surfaced through saver onError. */
11481176
})
11491177
},
11501178
getPersistedActivePrContext,

‎src/modules/app-core/github-workflows.js‎

Lines changed: 64 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,22 @@ const initializeGitHubWorkflows = ({
120120
const toSafeRepositoryFullName = value =>
121121
typeof value === 'string' ? value.trim() : ''
122122

123+
const toWorkspaceIdentitySegment = value => {
124+
const normalized = typeof value === 'string' ? value.trim().toLowerCase() : ''
125+
126+
if (!normalized) {
127+
return ''
128+
}
129+
130+
return normalized.replace(/[^a-z0-9]+/g, '-').replace(/^-+|-+$/g, '')
131+
}
132+
133+
const toWorkspaceRecordKey = ({ repositoryFullName, headBranch } = {}) => {
134+
const repoSegment = toWorkspaceIdentitySegment(repositoryFullName) || 'local'
135+
const headSegment = toWorkspaceIdentitySegment(headBranch) || 'draft'
136+
return `${repoSegment}::${headSegment}`
137+
}
138+
123139
const shouldApplyActivePrEditorSync = ({ repository, activeContext }) => {
124140
const syncedContextKey = getActivePrContextSyncKey(activeContext)
125141
const currentSyncKey = getActivePrEditorSyncKey()
@@ -285,6 +301,28 @@ const initializeGitHubWorkflows = ({
285301
activeWorkspaceRecordId,
286302
)
287303
if (activeWorkspaceRecord && typeof activeWorkspaceRecord === 'object') {
304+
const nextHeadBranch =
305+
typeof githubAiContextState.activePrContext?.headBranch === 'string' &&
306+
githubAiContextState.activePrContext.headBranch.trim()
307+
? githubAiContextState.activePrContext.headBranch.trim()
308+
: typeof branch === 'string' && branch.trim()
309+
? branch.trim()
310+
: typeof activeWorkspaceRecord.head === 'string'
311+
? activeWorkspaceRecord.head
312+
: ''
313+
const nextBaseBranch =
314+
typeof githubAiContextState.activePrContext?.baseBranch === 'string' &&
315+
githubAiContextState.activePrContext.baseBranch.trim()
316+
? githubAiContextState.activePrContext.baseBranch.trim()
317+
: typeof activeWorkspaceRecord.base === 'string'
318+
? activeWorkspaceRecord.base
319+
: ''
320+
const nextRepositoryFullName =
321+
toSafeRepositoryFullName(repositoryFullName) ||
322+
toSafeRepositoryFullName(
323+
githubAiContextState.activePrContext?.repositoryFullName,
324+
) ||
325+
toSafeRepositoryFullName(activeWorkspaceRecord.repo)
288326
const nextPrTitle =
289327
typeof githubAiContextState.activePrContext?.prTitle === 'string' &&
290328
githubAiContextState.activePrContext.prTitle.trim()
@@ -303,6 +341,14 @@ const initializeGitHubWorkflows = ({
303341

304342
const savedWorkspaceRecord = await workspaceStorage.upsertWorkspace({
305343
...activeWorkspaceRecord,
344+
workspaceScope: nextRepositoryFullName ? 'repository' : 'local',
345+
workspaceKey: toWorkspaceRecordKey({
346+
repositoryFullName: nextRepositoryFullName,
347+
headBranch: nextHeadBranch,
348+
}),
349+
repo: nextRepositoryFullName,
350+
base: nextBaseBranch,
351+
head: nextHeadBranch,
306352
prContextState: 'active',
307353
prNumber: nextPrNumber,
308354
prTitle: nextPrTitle,
@@ -544,8 +590,10 @@ const initializeGitHubWorkflows = ({
544590
onPrContextStateChange(prDrawerController.getActivePrContext())
545591
}
546592

593+
let isClosingActivePullRequest = false
594+
547595
githubPrContextClose?.addEventListener('click', () => {
548-
if (!githubAiContextState.activePrContext) {
596+
if (!githubAiContextState.activePrContext || isClosingActivePullRequest) {
549597
return
550598
}
551599

@@ -559,6 +607,15 @@ const initializeGitHubWorkflows = ({
559607
copy: `${referenceLine}PR title: ${githubAiContextState.activePrContext.prTitle}\nHead branch: ${githubAiContextState.activePrContext.headBranch}\n\nThis will close the pull request on GitHub and clear the active pull request context for the selected repository.`,
560608
confirmButtonText: 'Close PR on GitHub',
561609
onConfirm: () => {
610+
if (isClosingActivePullRequest) {
611+
return
612+
}
613+
614+
isClosingActivePullRequest = true
615+
if (githubPrContextClose instanceof HTMLButtonElement) {
616+
githubPrContextClose.disabled = true
617+
}
618+
562619
void prDrawerController
563620
.closeActivePullRequestOnGitHub()
564621
.then(result => {
@@ -586,6 +643,12 @@ const initializeGitHubWorkflows = ({
586643
setStatus(`Close context failed: ${message}`, 'error')
587644
showAppToast(`Close context failed: ${message}`)
588645
})
646+
.finally(() => {
647+
isClosingActivePullRequest = false
648+
if (githubPrContextClose instanceof HTMLButtonElement) {
649+
githubPrContextClose.disabled = false
650+
}
651+
})
589652
},
590653
})
591654
})

‎src/modules/app-core/pr-context-records.js‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,21 @@ const persistClosedPrContextRecords = async ({
33
selectedRepository,
44
nextPrNumber,
55
normalizedHead,
6+
fallbackPrTitle,
67
toNonEmptyWorkspaceText,
78
refreshLocalContextOptions,
89
}) => {
910
const siblingRecords = selectedRepository
1011
? await workspaceStorage.listWorkspaces({ repo: selectedRepository })
1112
: await workspaceStorage.listWorkspaces()
1213

13-
const activeRecordsForContext = siblingRecords.filter(record => {
14+
const recordsForContext = siblingRecords.filter(record => {
1415
if (!record || typeof record !== 'object') {
1516
return false
1617
}
1718

18-
if (toNonEmptyWorkspaceText(record.prContextState).toLowerCase() !== 'active') {
19+
const normalizedState = toNonEmptyWorkspaceText(record.prContextState).toLowerCase()
20+
if (normalizedState !== 'active' && normalizedState !== 'closed') {
1921
return false
2022
}
2123

@@ -32,20 +34,25 @@ const persistClosedPrContextRecords = async ({
3234
return hasMatchingPrNumber || hasMatchingHead
3335
})
3436

35-
if (activeRecordsForContext.length === 0) {
37+
if (recordsForContext.length === 0) {
3638
return
3739
}
3840

41+
const normalizedFallbackTitle = toNonEmptyWorkspaceText(fallbackPrTitle)
3942
const now = Date.now()
4043
await Promise.all(
41-
activeRecordsForContext.map(record =>
42-
workspaceStorage.upsertWorkspace({
44+
recordsForContext.map(record => {
45+
const preservedTitle =
46+
toNonEmptyWorkspaceText(record.prTitle) || normalizedFallbackTitle
47+
48+
return workspaceStorage.upsertWorkspace({
4349
...record,
4450
prContextState: 'closed',
4551
prNumber: nextPrNumber,
52+
prTitle: preservedTitle,
4653
lastModified: now,
47-
}),
48-
),
54+
})
55+
}),
4956
)
5057

5158
await refreshLocalContextOptions()

‎src/modules/app-core/workspace-pr-session-handoff-controller.js‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ export const createWorkspacePrSessionHandoffController = ({
77
selectors,
88
utils,
99
}) => {
10-
const { defaultComponentTabName, defaultComponentTabPath } = defaults
10+
const { defaultComponentTabName, defaultComponentTabPath, defaultComponentTabContent } =
11+
defaults
1112
const {
1213
getWorkspacePrNumber,
1314
setWorkspacePrContextState,
@@ -50,6 +51,8 @@ export const createWorkspacePrSessionHandoffController = ({
5051

5152
const createFreshLocalEntryTab = () => {
5253
const now = Date.now()
54+
const normalizedDefaultComponentTabContent =
55+
typeof defaultComponentTabContent === 'string' ? defaultComponentTabContent : ''
5356

5457
return {
5558
id: 'entry',
@@ -59,7 +62,7 @@ export const createWorkspacePrSessionHandoffController = ({
5962
role: 'entry',
6063
isActive: true,
6164
scroll: 0,
62-
content: '',
65+
content: normalizedDefaultComponentTabContent,
6366
targetPrFilePath: null,
6467
isDirty: false,
6568
syncedAt: null,

0 commit comments

Comments
 (0)