Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@ images/settings
gitlens-*.vsix
product.json
tsconfig*.tsbuildinfo
localdev*.instructions.md
28 changes: 13 additions & 15 deletions src/autolinks/autolinksProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import type { ConfigurationChangeEvent } from 'vscode';
import { Disposable } from 'vscode';
import { GlyphChars } from '../constants';
import type { IntegrationIds } from '../constants.integrations';
import { IssuesCloudHostIntegrationId } from '../constants.integrations';
import type { Container } from '../container';
import type { GitRemote } from '../git/models/remote';
import type { RemoteProvider, RemoteProviderId } from '../git/remotes/remoteProvider';
Expand Down Expand Up @@ -177,16 +176,11 @@ export class AutolinksProvider implements Disposable {
return getAutolinks(message, refsets);
}

getAutolinkEnrichableId(autolink: Autolink): string {
// TODO: this should return linking key for all types of providers: such as TST-123 or #89 or PR 89 (or a pair: key+id).
// Each provider should form whatever ID they need in their specific getIssueOrPullRequest() method.
switch (autolink.provider?.id) {
case IssuesCloudHostIntegrationId.Jira:
case IssuesCloudHostIntegrationId.Linear:
return `${autolink.prefix}${autolink.id}`;
default:
return autolink.id;
}
getAutolinkEnrichableId(autolink: Autolink): { id: string; key: string } {
return {
id: autolink.id,
key: `${autolink.prefix}${autolink.id}`,
};
}

async getEnrichedAutolinks(
Expand Down Expand Up @@ -258,15 +252,19 @@ export class AutolinksProvider implements Disposable {
integration != null &&
integrationId === integration.id &&
link.provider?.domain === integration.domain
? integration.getIssueOrPullRequest(
? integration.getLinkedIssueOrPullRequest(
link.descriptor ?? remote.provider.repoDesc,
this.getAutolinkEnrichableId(link),
{ type: link.type },
)
: link.descriptor != null
? linkIntegration?.getIssueOrPullRequest(link.descriptor, this.getAutolinkEnrichableId(link), {
type: link.type,
})
? linkIntegration?.getLinkedIssueOrPullRequest(
link.descriptor,
this.getAutolinkEnrichableId(link),
{
type: link.type,
},
)
: undefined;
enrichedAutolinks.set(id, [issueOrPullRequestPromise, link]);
}
Expand Down
14 changes: 7 additions & 7 deletions src/plus/integrations/models/integration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -499,9 +499,9 @@ export abstract class IntegrationBase<
): Promise<IssueShape[] | undefined>;

@debug()
async getIssueOrPullRequest(
async getLinkedIssueOrPullRequest(
resource: T,
id: string,
link: { id: string; key: string },
options?: { expiryOverride?: boolean | number; type?: IssueOrPullRequestType },
): Promise<IssueOrPullRequest | undefined> {
const scope = getLogScope();
Expand All @@ -512,17 +512,17 @@ export abstract class IntegrationBase<
await this.refreshSessionIfExpired(scope);

const issueOrPR = this.container.cache.getIssueOrPullRequest(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we update the cache function's name and parameter as well since this has been updated everywhere else?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@axosoft-ramint
I'm not sure about that. Integration provider is smart while cache is stupid.

Integration provider is smart enough to understand why it have to pick among two incoming strings (id and key). So, when a developer creates, e.g. Gerrit integration it knows that it's a Gerrit. getLinkedIssueOrPullRequest() and picks a better value for retrieving the linked object by API.

While cache is just a key-value storage that stores issues or PR: Hey cache, take a key and give me whatever you have.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@axosoft-ramint
well, now I'm thinking that maybe it's worth to add type (type?: IssueOrPullRequestType) to the cache key, because it plays role in the retrieving algorithm (in the provider-specific part of the method).

what do you think?

id,
link.key,
options?.type,
resource,
this,
() => ({
value: (async () => {
try {
const result = await this.getProviderIssueOrPullRequest(
const result = await this.getProviderLinkedIssueOrPullRequest(
this._session!,
resource,
id,
link,
options?.type,
);
this.resetRequestExceptionCount('getIssueOrPullRequest');
Expand All @@ -538,10 +538,10 @@ export abstract class IntegrationBase<
return issueOrPR;
}

protected abstract getProviderIssueOrPullRequest(
protected abstract getProviderLinkedIssueOrPullRequest(
session: ProviderAuthenticationSession,
resource: T,
id: string,
link: { id: string; key: string },
type: undefined | IssueOrPullRequestType,
): Promise<IssueOrPullRequest | undefined>;

Expand Down
4 changes: 2 additions & 2 deletions src/plus/integrations/providers/azureDevOps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -259,10 +259,10 @@ export abstract class AzureDevOpsIntegrationBase<
return Promise.resolve(undefined);
}

protected override async getProviderIssueOrPullRequest(
protected override async getProviderLinkedIssueOrPullRequest(
{ accessToken }: AuthenticationSession,
repo: AzureRepositoryDescriptor,
id: string,
{ id }: { id: string; key: string },
type: undefined | IssueOrPullRequestType,
): Promise<IssueOrPullRequest | undefined> {
return (await this.container.azure)?.getIssueOrPullRequest(this, accessToken, repo.owner, repo.name, id, {
Expand Down
4 changes: 2 additions & 2 deletions src/plus/integrations/providers/bitbucket-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,10 +105,10 @@ export class BitbucketServerIntegration extends GitHostIntegration<
return Promise.resolve(undefined);
}

protected override async getProviderIssueOrPullRequest(
protected override async getProviderLinkedIssueOrPullRequest(
{ accessToken }: AuthenticationSession,
repo: BitbucketRepositoryDescriptor,
id: string,
{ id }: { id: string; key: string },
type: undefined | IssueOrPullRequestType,
): Promise<IssueOrPullRequest | undefined> {
if (type === 'issue') {
Expand Down
4 changes: 2 additions & 2 deletions src/plus/integrations/providers/bitbucket.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ export class BitbucketIntegration extends GitHostIntegration<
return Promise.resolve(undefined);
}

protected override async getProviderIssueOrPullRequest(
protected override async getProviderLinkedIssueOrPullRequest(
{ accessToken }: AuthenticationSession,
repo: BitbucketRepositoryDescriptor,
id: string,
{ id }: { id: string; key: string },
type: undefined | IssueOrPullRequestType,
): Promise<IssueOrPullRequest | undefined> {
return (await this.container.bitbucket)?.getIssueOrPullRequest(
Expand Down
4 changes: 2 additions & 2 deletions src/plus/integrations/providers/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,10 +83,10 @@ abstract class GitHubIntegrationBase<ID extends GitHubIntegrationIds> extends Gi
});
}

protected override async getProviderIssueOrPullRequest(
protected override async getProviderLinkedIssueOrPullRequest(
{ accessToken }: AuthenticationSession,
repo: GitHubRepositoryDescriptor,
id: string,
{ id }: { id: string; key: string },
): Promise<IssueOrPullRequest | undefined> {
return (await this.container.github)?.getIssueOrPullRequest(
this,
Expand Down
4 changes: 2 additions & 2 deletions src/plus/integrations/providers/gitlab.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,10 @@ abstract class GitLabIntegrationBase<ID extends GitLabIntegrationIds> extends Gi
});
}

protected override async getProviderIssueOrPullRequest(
protected override async getProviderLinkedIssueOrPullRequest(
{ accessToken }: AuthenticationSession,
repo: GitLabRepositoryDescriptor,
id: string,
{ id }: { id: string; key: string },
): Promise<IssueOrPullRequest | undefined> {
return (await this.container.gitlab)?.getIssueOrPullRequest(
this,
Expand Down
6 changes: 3 additions & 3 deletions src/plus/integrations/providers/jira.ts
Original file line number Diff line number Diff line change
Expand Up @@ -272,15 +272,15 @@ export class JiraIntegration extends IssuesIntegration<IssuesCloudHostIntegratio
return results;
}

protected override async getProviderIssueOrPullRequest(
protected override async getProviderLinkedIssueOrPullRequest(
session: AuthenticationSession,
resource: JiraOrganizationDescriptor,
id: string,
{ key }: { id: string; key: string },
): Promise<IssueOrPullRequest | undefined> {
const api = await this.getProvidersApi();
const issue = await api.getIssue(
this.id,
{ resourceId: resource.id, number: id },
{ resourceId: resource.id, number: key },
{ accessToken: session.accessToken },
);
return issue != null ? toIssueShape(issue, this) : undefined;
Expand Down
4 changes: 2 additions & 2 deletions src/plus/integrations/providers/linear.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,10 +210,10 @@ export class LinearIntegration extends IssuesIntegration<IssuesCloudHostIntegrat
}
return issues;
}
protected override async getProviderIssueOrPullRequest(
protected override async getProviderLinkedIssueOrPullRequest(
session: ProviderAuthenticationSession,
resource: ResourceDescriptor,
id: string,
{ id }: { id: string; key: string },
_type: undefined | IssueOrPullRequestType,
): Promise<IssueOrPullRequest | undefined> {
const issue = await this.getRawProviderIssue(session, resource, id);
Expand Down