GitHub proxy part 4.5: git server in UI #49644
Conversation
greedy52
added
no-changelog
greedy52
force-pushed
the
STeve/48762_git_server_cache_unified_resource
branch
from
d1e8553 to
982bf3d
Compare
greedy52
force-pushed
the
STeve/48762_git_server_cache_unified_resource_ui
branch
6 times, most recently
from
d3e099b to
fb9aeb1
Compare
greedy52
force-pushed
the
STeve/48762_git_server_cache_unified_resource_ui
branch
from
fb9aeb1 to
acaca65
Compare
|
I didn't get a chance to check this out today but i will look at it the first thing in the morning tomorrow |
| hostname: string; | ||
| labels: ResourceLabel[]; | ||
| subKind: 'github'; | ||
| github?: { |
There was a problem hiding this comment.
When is resource.github nil?
| import { AuthType } from 'teleport/services/user'; | ||
| import { generateTshLoginCommand } from 'teleport/lib/util'; | ||
|
|
||
| export default function ConnectDialog({ |
There was a problem hiding this comment.
Let's use named exports
| export default function ConnectDialog({ | |
| export function ConnectDialog({ |
| onClose, | ||
| authType, | ||
| accessRequestId, | ||
| }: Props) { |
There was a problem hiding this comment.
We don't need to keep Props in a separate type, we can inline them here.
| let repoURL = `https://github.com/orgs/${organization}/repositories`; | ||
| let title = `Use 'git' for GitHub Organization '${organization}'`; |
There was a problem hiding this comment.
| let repoURL = `https://github.com/orgs/${organization}/repositories`; | |
| let title = `Use 'git' for GitHub Organization '${organization}'`; | |
| const repoURL = `https://github.com/orgs/${organization}/repositories`; | |
| const title = `Use 'git' for GitHub Organization '${organization}'`; |
| @@ -327,6 +331,42 @@ const KubeConnect = ({ kube }: { kube: Kube }) => { | |||
| ); | |||
| }; | |||
|
|
|||
| function GitServerConnect({ gitserver }: { gitserver: GitServer }) { | |||
| <Text bold as="span"> | ||
| Step 2 | ||
| </Text> | ||
| {' - Connect'} |
There was a problem hiding this comment.
I'm not sure if we should say 'Connect', we don't connect to anything.
Maybe 'Clone or configure a repository'?
| {'To clone a new repository, find the SSH url of the repository on '} | ||
| <Link href={repoURL} target="_blank"> | ||
| github.com | ||
| </Link> | ||
| {' then'} |
There was a problem hiding this comment.
Shouldn't it be 'To clone a new repository, find the SSH url of the repository on github.com, and then'?
| <DialogHeader mb={4}> | ||
| <DialogTitle>{title}</DialogTitle> | ||
| </DialogHeader> | ||
| <DialogContent minHeight="240px" flex="0 0 auto"> |
There was a problem hiding this comment.
Spacing is a little off in the dialog, labels are closer to the previous field than to the next one.
Here is an improved version (I also got rid of some curly braces as they are not needed):
<DialogContent gap={4}>
<Box>
<Text bold as="span">
Step 1
</Text>
{' - Login to Teleport'}
<TextSelectCopy
mt="1"
mb="2"
text={generateTshLoginCommand({
authType,
clusterId,
username,
accessRequestId,
})}
/>
</Box>
<Box>
<Text bold as="span">
Step 2
</Text>
{' - Connect'}
<br />
{'To clone a new repository, find the SSH url of the repository on '}
<Link href={repoURL} target="_blank">
github.com
</Link>
{' then'}
<TextSelectCopy
mt="1"
mb="2"
text="tsh git clone <git-clone-ssh-url>"
/>
To configure an existing Git repository, go to the repository then
<TextSelectCopy mt="1" mb="2" text="tsh git config update" />
</Box>
<Box>
Once the repository is cloned or configured, use 'git' as normal.
</Box>
</DialogContent>| const organization = gitserver.github | ||
| ? gitserver.github.organization | ||
| : undefined; |
There was a problem hiding this comment.
| const organization = gitserver.github | |
| ? gitserver.github.organization | |
| : undefined; | |
| const organization = gitserver.github?.organization; |
|
|
||
| import { UserGroup } from '../userGroups'; | ||
| import { UserGroup } from 'teleport/services/userGroups'; | ||
| import { GitServer } from 'teleport/services/gitservers'; |
There was a problem hiding this comment.
I don't have any strong opinion, but gitservers seems inconsistent with other exports. I'd rename it to gitServers.
| if server.GetKind() == types.KindGitServer && | ||
| server.GetSubKind() == types.SubKindGitHub { | ||
| if github := server.GetGitHub(); github != nil { | ||
| uiServer.GitHub = &GitHubServerMetadata{ | ||
| Integration: github.Integration, | ||
| Organization: github.Organization, | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
it seems like this should be in its own method, something like MakeGitServer. If its inside here, it gives the impression that this is a "server type" instead of its own thing.
Related:
tsh git lswith unified resource #49596e:
(note, this PR is optional for the initial MVP release but very nice to have)
Storybook screenshots:
