Set gitHostFactory for gitlab self hosted instances #4775
Conversation
|
@fgLouro is attempting to deploy a commit to the GitButler Team on Vercel. A member of the Team first needs to authorize it. |
fgLouro
force-pushed
the
fix-gitlab-branch-links
branch
from
e3808c0
to
76987e8
Compare
fgLouro
force-pushed
the
fix-gitlab-branch-links
branch
from
5946df9
to
8bff511
Compare
| @@ -8,7 +8,8 @@ describe.concurrent('GitHubBranch', () => { | |||
| const repo = { | |||
| source: 'github.com', | |||
| name: 'test-repo', | |||
| owner: 'test-owner' | |||
| owner: 'test-owner', | |||
| resource: 'test-resource' | |||
There was a problem hiding this comment.
Let's not add these unless necessary for the test to pass?
apps/desktop/src/lib/url/gitUrl.ts
Outdated
| @@ -4,20 +4,22 @@ export type RepoInfo = { | |||
| source: string; | |||
| name: string; | |||
| owner: string; | |||
| resource: string; | |||
There was a problem hiding this comment.
Wouldn't it be misleading to use an optional attribute for the resource?
The gitUrlParse function seems to always return a resource attribute when the url is valid, even if it's an empty 'resource' it's not undefined.
There was a problem hiding this comment.
Sorry, I forgot what it represents, and sorry for the back and forth here. I think.. we should only have one field for url, instead of calling it source and resource. If you agree, would you mind creating a type and replacing the one we borrow from the parsing lib?
There was a problem hiding this comment.
Back and forth is how good software is made 😄
You mean removing the resource and source from the RepoInfo type and have only a url attribute which will be the value of resource (the full url)?
I think the source is only used in DefaultGitHostFactory class, so should be easy to replace it.
There was a problem hiding this comment.
Yes, exactly! I would probably go with domain over url since it is likely to have additional things like scheme, path, etc. Also, my previous message was probably confusing because I forgot we have our own RepoInfo type, I thought we were using the one returned from git-url-parse.
There was a problem hiding this comment.
@mtsgrd Done!
Removed all references of source and resource in favour or using domain.
☕️ Reasoning
The
DefaultGitHostFactoryclass assumes that gitlab domain is alwaysgitlab.com, which is not correct for self hosted instances of gitlab, and leads to branch link buttons to be unusable.🧢 Changes
resourcetoRepoInfotype to be used for adding the correctbaseUrlresourcefor gitlab subdomain to initialize thegitHostFactorywith a Gitlab class🎫 Affected issues
Fixes: #4558 (comment)
Things to consider
Self hosted instance might not have
gitlabin their subdomain, which will result in the same issue.