-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
fix(studio): refetch GitHub repositories after new install #39021
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
0168146
e0248a8
79e8705
f8b72d5
66534d9
cff00a5
8dd7327
75e3f7e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,6 +47,7 @@ import { FormItemLayout } from 'ui-patterns/form/FormItemLayout/FormItemLayout' | |
|
||
const GITHUB_ICON = ( | ||
<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 98 96" className="w-6"> | ||
<title>GitHub icon</title> | ||
<path | ||
fill="#ffffff" | ||
fillRule="evenodd" | ||
|
@@ -70,7 +71,7 @@ const GitHubIntegrationConnectionForm = ({ | |
const [isConfirmingBranchChange, setIsConfirmingBranchChange] = useState(false) | ||
const [isConfirmingRepoChange, setIsConfirmingRepoChange] = useState(false) | ||
const [repoComboBoxOpen, setRepoComboboxOpen] = useState(false) | ||
const isParentProject = !Boolean(selectedProject?.parent_project_ref) | ||
const isParentProject = !selectedProject?.parent_project_ref | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: Unnecessary boolean. |
||
|
||
const { can: canUpdateGitHubConnection } = useAsyncCheckPermissions( | ||
PermissionAction.UPDATE, | ||
|
@@ -81,14 +82,24 @@ const GitHubIntegrationConnectionForm = ({ | |
'integrations.github_connections' | ||
) | ||
|
||
const { data: gitHubAuthorization } = useGitHubAuthorizationQuery() | ||
const { data: gitHubAuthorization, refetch: refetchGitHubAuthorization } = | ||
useGitHubAuthorizationQuery() | ||
|
||
const { data: githubReposData, isLoading: isLoadingGitHubRepos } = useGitHubRepositoriesQuery< | ||
any[] | ||
>({ | ||
const { | ||
data: githubReposData, | ||
isLoading: isLoadingGitHubRepos, | ||
refetch: refetchGitHubRepositories, | ||
} = useGitHubRepositoriesQuery({ | ||
enabled: Boolean(gitHubAuthorization), | ||
}) | ||
|
||
const refetchGitHubAuthorizationAndRepositories = () => { | ||
setTimeout(() => { | ||
refetchGitHubAuthorization() | ||
refetchGitHubRepositories() | ||
}, 2000) // 2 second to delay to let github authorization and repositories to be updated | ||
} | ||
Comment on lines
+96
to
+101
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: Github api takes ~2s per testing to acknowledge changes, retrying the query after 2s allows the pop-up to update with the latest repositories without the need to interact / search over the pop-up. |
||
|
||
const { mutate: updateBranch } = useBranchUpdateMutation({ | ||
onSuccess: () => { | ||
toast.success('Production branch settings successfully updated') | ||
|
@@ -130,7 +141,7 @@ const GitHubIntegrationConnectionForm = ({ | |
|
||
const githubRepos = useMemo( | ||
() => | ||
githubReposData?.map((repo: any) => ({ | ||
githubReposData?.map((repo) => ({ | ||
id: repo.id.toString(), | ||
name: repo.name, | ||
installation_id: repo.installation_id, | ||
|
@@ -161,7 +172,7 @@ const GitHubIntegrationConnectionForm = ({ | |
repositoryId: Number(repositoryId), | ||
branchName: val.branchName, | ||
}) | ||
} catch (error) { | ||
} catch { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: Unused error |
||
const selectedRepo = githubRepos.find((repo) => repo.id === repositoryId) | ||
const repoName = | ||
selectedRepo?.name || connection?.repository.name || 'selected repository' | ||
|
@@ -337,11 +348,11 @@ const GitHubIntegrationConnectionForm = ({ | |
const data = githubSettingsForm.getValues() | ||
const selectedRepo = githubRepos.find((repo) => repo.id === data.repositoryId) | ||
|
||
if (!selectedRepo || !connection) return | ||
if (!selectedRepo || !connection || !selectedOrganization?.id) return | ||
|
||
try { | ||
await deleteConnection({ | ||
organizationId: selectedOrganization!.id, | ||
organizationId: selectedOrganization.id, | ||
connectionId: connection.id, | ||
}) | ||
|
||
|
@@ -390,7 +401,10 @@ const GitHubIntegrationConnectionForm = ({ | |
</p> | ||
<Button | ||
onClick={() => { | ||
openInstallGitHubIntegrationWindow('authorize') | ||
openInstallGitHubIntegrationWindow( | ||
'authorize', | ||
refetchGitHubAuthorizationAndRepositories | ||
) | ||
}} | ||
> | ||
Authorize GitHub | ||
|
@@ -482,7 +496,12 @@ const GitHubIntegrationConnectionForm = ({ | |
<CommandGroup_Shadcn_> | ||
<CommandItem_Shadcn_ | ||
className="flex gap-2 items-center cursor-pointer" | ||
onSelect={() => openInstallGitHubIntegrationWindow('install')} | ||
onSelect={() => | ||
openInstallGitHubIntegrationWindow( | ||
'install', | ||
refetchGitHubAuthorizationAndRepositories | ||
) | ||
} | ||
> | ||
<PlusIcon size={16} /> | ||
Add GitHub Repositories | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,8 +10,7 @@ export async function getGitHubRepositories(signal?: AbortSignal) { | |
}) | ||
|
||
if (error) handleError(error) | ||
// [Alaister]: temp fix until we have a proper response type | ||
return (data as any).repositories | ||
return data.repositories | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note This is now properly typed. |
||
} | ||
|
||
export type GitHubRepositoriesData = Awaited<ReturnType<typeof getGitHubRepositories>> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -19,7 +19,10 @@ const GITHUB_INTEGRATION_AUTHORIZATION_URL = `https://github.com/login/oauth/aut | |
export const GITHUB_INTEGRATION_INSTALLATION_URL = `https://github.com/apps/${GITHUB_INTEGRATION_APP_NAME}/installations/new` | ||
export const GITHUB_INTEGRATION_REVOKE_AUTHORIZATION_URL = `https://github.com/settings/connections/applications/${GITHUB_INTEGRATION_CLIENT_ID}` | ||
|
||
export function openInstallGitHubIntegrationWindow(type: 'install' | 'authorize') { | ||
export function openInstallGitHubIntegrationWindow( | ||
type: 'install' | 'authorize', | ||
closeCallback?: () => void | ||
) { | ||
const w = 600 | ||
const h = 800 | ||
|
||
|
@@ -37,10 +40,10 @@ export function openInstallGitHubIntegrationWindow(type: 'install' | 'authorize' | |
? document.documentElement.clientHeight | ||
: screen.height | ||
|
||
let windowUrl | ||
let windowUrl: string | undefined | ||
if (type === 'install') { | ||
windowUrl = GITHUB_INTEGRATION_INSTALLATION_URL | ||
} else if (type === 'authorize') { | ||
} else { | ||
const state = makeRandomString(32) | ||
localStorage.setItem(LOCAL_STORAGE_KEYS.GITHUB_AUTHORIZATION_STATE, state) | ||
windowUrl = `${GITHUB_INTEGRATION_AUTHORIZATION_URL}&state=${state}` | ||
|
@@ -60,6 +63,20 @@ export function openInstallGitHubIntegrationWindow(type: 'install' | 'authorize' | |
` | ||
) | ||
if (newWindow) { | ||
if (closeCallback) { | ||
// Poll to check if window is closed | ||
const checkClosed = setInterval(() => { | ||
if (newWindow.closed) { | ||
clearInterval(checkClosed) | ||
closeCallback() | ||
} | ||
}, 500) // Check every 500ms | ||
|
||
// Add a timeout to prevent infinite polling | ||
setTimeout(() => { | ||
clearInterval(checkClosed) | ||
}, 300000) // 5 minutes timeout | ||
} | ||
Comment on lines
+66
to
+79
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note This trick allow to trigger refetch when the pop-up is closed after installation. |
||
newWindow.focus() | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note
Svg must have a title in it (biome says)