Skip to content

Commit

Permalink
Add autoclose on github connection confirmation
Browse files Browse the repository at this point in the history
  • Loading branch information
AdityaHegde committed Jul 31, 2024
1 parent 880f729 commit a27c050
Show file tree
Hide file tree
Showing 11 changed files with 87 additions and 41 deletions.
52 changes: 42 additions & 10 deletions admin/server/github.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,17 +360,21 @@ func (s *Server) ConnectProjectToGithub(ctx context.Context, req *adminv1.Connec
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}
err = s.pushToGit(ctx, func(dir, projPath string) error {
downloadDst := filepath.Join(dir, "zipped_repo.tar.gz")
err = s.pushToGit(ctx, func(projPath string) error {
downloadDir, err := os.MkdirTemp(os.TempDir(), "extracted_archives")
if err != nil {
return err
}
downloadDst := filepath.Join(downloadDir, "zipped_repo.tar.gz")
// extract the archive once the folder is prepped with git
return archive.Download(ctx, downloadURL, downloadDst, projPath, false)
}, req.Repo, req.Branch, req.Subpath, token, req.Force)
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
}
} else if proj.GithubURL != nil {
err = s.pushToGit(ctx, func(dir, projPath string) error {
return copyFromSrcGit(dir, projPath, *proj.GithubURL, proj.ProdBranch, proj.Subpath, token)
err = s.pushToGit(ctx, func(projPath string) error {
return copyFromSrcGit(projPath, *proj.GithubURL, proj.ProdBranch, proj.Subpath, token)
}, req.Repo, req.Branch, req.Subpath, token, req.Force)
if err != nil {
return nil, status.Error(codes.InvalidArgument, err.Error())
Expand Down Expand Up @@ -523,6 +527,18 @@ func (s *Server) githubConnectCallback(w http.ResponseWriter, r *http.Request) {
}

remoteURL := qry.Get("state")
if remoteURL == "autoclose" {
// signal from UI flow to autoclose the confirmation dialog
// TODO: if we ever want more complex signals, we should consider converting this to an object using proto or json
connectSuccessUrl, err := urlutil.WithQuery(s.urls.githubConnectSuccess, map[string]string{"autoclose": "true"})

Check warning on line 533 in admin/server/github.go

View workflow job for this annotation

GitHub Actions / lint

var-naming: var connectSuccessUrl should be connectSuccessURL (revive)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
http.Redirect(w, r, connectSuccessUrl, http.StatusTemporaryRedirect)
return
}

account, repo, ok := gitutil.SplitGithubURL(remoteURL)
if !ok {
// request without state can come in multiple ways like
Expand Down Expand Up @@ -710,6 +726,18 @@ func (s *Server) githubAuthCallback(w http.ResponseWriter, r *http.Request) {
}
delete(sess.Values, githubcookieFieldRemote)

if remote == "autoclose" {
// signal from UI flow to autoclose the confirmation dialog
// TODO: if we ever want more complex signals, we should consider converting this to an object using proto or json
connectSuccessUrl, err := urlutil.WithQuery(s.urls.githubConnectSuccess, map[string]string{"autoclose": "true"})

Check warning on line 732 in admin/server/github.go

View workflow job for this annotation

GitHub Actions / lint

var-naming: var connectSuccessUrl should be connectSuccessURL (revive)
if err != nil {
http.Error(w, err.Error(), http.StatusInternalServerError)
return
}
http.Redirect(w, r, connectSuccessUrl, http.StatusTemporaryRedirect)
return
}

account, repo, ok := gitutil.SplitGithubURL(remote)
if !ok {
http.Redirect(w, r, s.urls.githubConnectSuccess, http.StatusTemporaryRedirect)
Expand Down Expand Up @@ -972,12 +1000,12 @@ func (s *Server) fetchReposForInstallation(ctx context.Context, client *github.C
return repos, nil
}

func (s *Server) pushToGit(ctx context.Context, copyData func(dir, projPath string) error, repo, branch, subpath, token string, force bool) error {
func (s *Server) pushToGit(ctx context.Context, copyData func(projPath string) error, repo, branch, subpath, token string, force bool) error {
ctx, cancel := context.WithTimeout(ctx, archivePullTimeout)
defer cancel()

// generate a temp dir to extract the archive
dir, err := os.MkdirTemp(os.TempDir(), "extracted_archives")
dir, err := os.MkdirTemp(os.TempDir(), "dest_git_repos")
if err != nil {
return err
}
Expand Down Expand Up @@ -1047,7 +1075,7 @@ func (s *Server) pushToGit(ctx context.Context, copyData func(dir, projPath stri
}
}

err = copyData(dir, projPath)
err = copyData(projPath)
if err != nil {
return fmt.Errorf("failed to copy data: %w", err)
}
Expand Down Expand Up @@ -1170,14 +1198,18 @@ func readFile(f billy.File) (string, error) {

// copyFromSrcGit clones a repo, branch and a subpath and copies the content to the projPath
// used to switch a project to a new github repo connection
func copyFromSrcGit(dir, projPath, repo, branch, subpath, token string) error {
srcGitPath := filepath.Join(dir, "src")
func copyFromSrcGit(projPath, repo, branch, subpath, token string) error {
srcGitPath, err := os.MkdirTemp(os.TempDir(), "src_git_repos")
if err != nil {
return err
}

// projPath is the target for extracting the archive
srcProjPath := srcGitPath
if subpath != "" {
srcProjPath = filepath.Join(srcProjPath, subpath)
}
err := os.MkdirAll(srcProjPath, fs.ModePerm)
err = os.MkdirAll(srcProjPath, fs.ModePerm)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions admin/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,7 @@ func (s *Server) ServeGRPC(ctx context.Context) error {
server := grpc.NewServer(
grpc.ChainStreamInterceptor(
middleware.TimeoutStreamServerInterceptor(timeoutSelector),
observability.LoggingStreamServerInterceptor(s.logger),
//observability.LoggingStreamServerInterceptor(s.logger),

Check failure on line 153 in admin/server/server.go

View workflow job for this annotation

GitHub Actions / lint

commentFormatting: put a space between `//` and comment text (gocritic)
errorMappingStreamServerInterceptor(),
grpc_auth.StreamServerInterceptor(checkUserAgent),
grpc_validator.StreamServerInterceptor(),
Expand All @@ -159,7 +159,7 @@ func (s *Server) ServeGRPC(ctx context.Context) error {
),
grpc.ChainUnaryInterceptor(
middleware.TimeoutUnaryServerInterceptor(timeoutSelector),
observability.LoggingUnaryServerInterceptor(s.logger),
//observability.LoggingUnaryServerInterceptor(s.logger),

Check failure on line 162 in admin/server/server.go

View workflow job for this annotation

GitHub Actions / lint

commentFormatting: put a space between `//` and comment text (gocritic)
errorMappingUnaryServerInterceptor(),
grpc_auth.UnaryServerInterceptor(checkUserAgent),
grpc_validator.UnaryServerInterceptor(),
Expand Down
4 changes: 2 additions & 2 deletions runtime/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ func (s *Server) ServeGRPC(ctx context.Context) error {
server := grpc.NewServer(
grpc.ChainStreamInterceptor(
middleware.TimeoutStreamServerInterceptor(timeoutSelector),
observability.LoggingStreamServerInterceptor(s.logger),
//observability.LoggingStreamServerInterceptor(s.logger),

Check failure on line 127 in runtime/server/server.go

View workflow job for this annotation

GitHub Actions / lint

commentFormatting: put a space between `//` and comment text (gocritic)
grpc_validator.StreamServerInterceptor(),
auth.StreamServerInterceptor(s.aud),
middleware.ActivityStreamServerInterceptor(s.activity),
Expand All @@ -133,7 +133,7 @@ func (s *Server) ServeGRPC(ctx context.Context) error {
),
grpc.ChainUnaryInterceptor(
middleware.TimeoutUnaryServerInterceptor(timeoutSelector),
observability.LoggingUnaryServerInterceptor(s.logger),
//observability.LoggingUnaryServerInterceptor(s.logger),

Check failure on line 136 in runtime/server/server.go

View workflow job for this annotation

GitHub Actions / lint

commentFormatting: put a space between `//` and comment text (gocritic)
grpc_validator.UnaryServerInterceptor(),
auth.UnaryServerInterceptor(s.aud),
middleware.ActivityUnaryServerInterceptor(s.activity),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@
},
);
// we need a popup window so we cannot use href
window.open("https://github.com/new", "", "width=1024,height=600");
window.open(
"https://github.com/new",
"githubNewWindow",
"width=1024,height=600",
);
}
function handleContinue() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@ export class GithubConnectionUpdater {
private isConnected: boolean;
private defaultBranch = "";

public constructor(
private readonly organization: string,
private readonly project: string,
) {}
public constructor() {}

public init(githubUrl: string, subpath: string, branch: string) {
this.githubUrl.set(githubUrl);
Expand All @@ -42,9 +39,13 @@ export class GithubConnectionUpdater {
}

public async update({
organization,
project,
force,
instanceId,
}: {
organization: string;
project: string;
force: boolean;
instanceId: string;
}) {
Expand All @@ -56,8 +57,8 @@ export class GithubConnectionUpdater {

try {
await get(this.connectToGithubMutation).mutateAsync({
organization: this.organization,
project: this.project,
organization,
project,
data: {
repo: githubUrl,
subpath,
Expand All @@ -77,7 +78,7 @@ export class GithubConnectionUpdater {
);

void queryClient.refetchQueries(
getAdminServiceGetProjectQueryKey(this.organization, this.project),
getAdminServiceGetProjectQueryKey(organization, project),
{
// avoid refetching createAdminServiceGetProjectWithBearerToken
exact: true,
Expand Down
3 changes: 2 additions & 1 deletion web-admin/src/features/projects/github/GithubData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ export class GithubData {
}
if (this.windowCheckTimer) clearInterval(this.windowCheckTimer);
this.userPromptWindow = window.open(
url,
// add `remote` to indicate the callback success dialog should auto close
`${url}?remote=autoclose`,
"githubWindow",
"width=1024,height=600",
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
let confirmInput = "";
$: confirmed = confirmInput === "overwrite";
$: path = getRepoNameFromGithubUrl(githubUrl) + subpath;
$: path = `${getRepoNameFromGithubUrl(githubUrl)}/${subpath}`;
function close() {
onCancel();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,8 @@
label: `${r.owner}/${r.name}`,
})) ?? [];
const githubConnectionUpdater = new GithubConnectionUpdater(
organization,
project,
);
const githubConnectionUpdater = new GithubConnectionUpdater();
// update data from project, this is needed if the user never leaves the status page and this component is not unmounted
$: githubConnectionUpdater.init(currentUrl, currentSubpath, currentBranch);
const connectToGithubMutation =
Expand Down Expand Up @@ -84,6 +82,8 @@
async function updateGithubUrl(force: boolean) {
if (
!(await githubConnectionUpdater.update({
organization,
project,
force,
instanceId: $projectQuery.data?.prodDeployment?.runtimeInstanceId ?? "",
}))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,14 +113,11 @@
</div>
{/if}

{#key organization + project}
<!-- make sure to remount -->
<GithubRepoSelectionDialog
bind:open={$repoSelectionOpen}
currentUrl={$proj.data?.project?.githubUrl}
currentSubpath={$proj.data?.project?.subpath}
currentBranch={$proj.data?.project?.prodBranch}
{organization}
{project}
/>
{/key}
<GithubRepoSelectionDialog
bind:open={$repoSelectionOpen}
currentUrl={$proj.data?.project?.githubUrl}
currentSubpath={$proj.data?.project?.subpath}
currentBranch={$proj.data?.project?.prodBranch}
{organization}
{project}
/>
3 changes: 2 additions & 1 deletion web-admin/src/features/projects/github/github-utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
export function getRepoNameFromGithubUrl(githubUrl: string) {
const repoName = githubUrl.split("github.com/")[1];
return repoName;
// remove trailing forwards slash if present
return repoName.replace("//$/", "");
}
12 changes: 11 additions & 1 deletion web-admin/src/routes/-/github/connect/success/+page.svelte
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
<script>
<script lang="ts">
import { page } from "$app/stores";
import GithubSuccess from "@rilldata/web-common/components/icons/GithubSuccess.svelte";
import CtaContentContainer from "@rilldata/web-common/components/calls-to-action/CTAContentContainer.svelte";
import CtaHeader from "@rilldata/web-common/components/calls-to-action/CTAHeader.svelte";
import CtaLayoutContainer from "@rilldata/web-common/components/calls-to-action/CTALayoutContainer.svelte";
import CtaMessage from "@rilldata/web-common/components/calls-to-action/CTAMessage.svelte";
import { onMount } from "svelte";
onMount(() => {
if ($page.url.searchParams.get("autoclose") === "true") {
// close the window autoclose was set.
// this happens when the connection was requested through UI flow
window.close();
}
});
</script>

<svelte:head>
Expand Down

0 comments on commit a27c050

Please sign in to comment.