-
Notifications
You must be signed in to change notification settings - Fork 1.3k
feat(batches): overview of personal github apps #64105
base: main
Are you sure you want to change the base?
Changes from all commits
5bf273c
79e04ae
2806ce7
bc931be
5da1840
8465b6f
7de840d
1837f5a
f7056c6
cf73f1a
f44f016
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 |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| import type { FC } from 'react' | ||
|
|
||
| import { Route, Routes } from 'react-router-dom' | ||
|
|
||
| import { useQuery } from '@sourcegraph/http-client' | ||
| import type { AuthenticatedUser } from '@sourcegraph/shared/src/auth' | ||
| import type { PlatformContextProps } from '@sourcegraph/shared/src/platform/context' | ||
| import type { TelemetryProps } from '@sourcegraph/shared/src/telemetry/telemetryService' | ||
| import { lazyComponent } from '@sourcegraph/shared/src/util/lazyComponent' | ||
| import { ErrorAlert, LoadingSpinner } from '@sourcegraph/wildcard' | ||
|
|
||
| import { type SiteExternalServiceConfigResult, type SiteExternalServiceConfigVariables } from '../../graphql-operations' | ||
| import { SITE_EXTERNAL_SERVICE_CONFIG } from '../../site-admin/backend' | ||
|
|
||
| const GitHubAppPage = lazyComponent(() => import('../../components/gitHubApps/GitHubAppPage'), 'GitHubAppPage') | ||
| const GitHubAppsPage = lazyComponent(() => import('../../components/gitHubApps/GitHubAppsPage'), 'GitHubAppsPage') | ||
|
|
||
| interface Props extends TelemetryProps, PlatformContextProps { | ||
| authenticatedUser: AuthenticatedUser | ||
| batchChangesEnabled: boolean | ||
| } | ||
|
|
||
| export const UserGitHubAppsArea: FC<Props> = props => { | ||
|
Contributor
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. I don't think we should allow creation of GitHub apps in here, why?
Contributor
Author
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. That's reasonable, and since we have text there indicating that github apps are for batch changes, they should find their way to the batch changes settings. I updated the code to hide the button if we're not in the siteAdmin context. |
||
| const { data, error, loading } = useQuery<SiteExternalServiceConfigResult, SiteExternalServiceConfigVariables>( | ||
| SITE_EXTERNAL_SERVICE_CONFIG, | ||
| {} | ||
| ) | ||
|
|
||
| if (error && !loading) { | ||
| return <ErrorAlert error={error} /> | ||
| } | ||
|
|
||
| if (loading && !error) { | ||
| return <LoadingSpinner /> | ||
| } | ||
bahrmichael marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if (!data) { | ||
| return null | ||
| } | ||
|
|
||
| return ( | ||
| <Routes> | ||
| <Route | ||
| index={true} | ||
| element={ | ||
| <GitHubAppsPage | ||
| batchChangesEnabled={props.batchChangesEnabled} | ||
| telemetryRecorder={props.platformContext.telemetryRecorder} | ||
| userOwned={true} | ||
| /> | ||
| } | ||
| /> | ||
|
|
||
| <Route | ||
| path=":appID" | ||
| element={ | ||
| <GitHubAppPage | ||
| headerParentBreadcrumb={{ to: '/user/github-apps', text: 'GitHub Apps' }} | ||
| telemetryRecorder={props.platformContext.telemetryRecorder} | ||
| userOwned={true} | ||
| {...props} | ||
| /> | ||
| } | ||
| /> | ||
| </Routes> | ||
| ) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -136,6 +136,7 @@ func (srv *gitHubAppServer) stateHandler(w http.ResponseWriter, r *http.Request) | |
| gqlID := r.URL.Query().Get("id") | ||
| domain := r.URL.Query().Get("domain") | ||
| baseURL := r.URL.Query().Get("baseURL") | ||
| kind := r.URL.Query().Get("kind") | ||
| if gqlID == "" { | ||
| // we marshal an empty `gitHubAppStateDetails` struct because we want the values saved in the cache | ||
| // to always conform to the same structure. | ||
|
|
@@ -159,6 +160,7 @@ func (srv *gitHubAppServer) stateHandler(w http.ResponseWriter, r *http.Request) | |
| AppID: int(id64), | ||
| Domain: domain, | ||
| BaseURL: baseURL, | ||
| Kind: kind, | ||
|
Member
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. Were we accidentally omitting kind before? |
||
| }) | ||
| if err != nil { | ||
| http.Error(w, fmt.Sprintf("Unexpected error when marshalling state: %s", err.Error()), http.StatusInternalServerError) | ||
|
|
||
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.
Q: why did this change? Does adding the kind here still work for site admin setup?