Skip to content

Commit 87d3a8b

Browse files
apeniapeni
authored
AXON-776: fix issue with deleting multiple instances (#692)
* AXON-776: fix issue with deleting multiple instances * AXON-776: remove async for onDidAuthChange
1 parent 7ecff9a commit 87d3a8b

File tree

7 files changed

+36
-19
lines changed

7 files changed

+36
-19
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
- Removed setting options that had no practical or business logic use.
1414

1515
- Fixed a bug where Jira site icon is broken in create Jira issue page
16+
- Fixed a bug with getting the error when deleting multiple instances
1617

1718
## What's new in 3.8.8
1819

‎src/atlclients/authStore.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -189,7 +189,7 @@ describe('CredentialManager', () => {
189189
await credentialManager.getAuthInfo(mockJiraSite);
190190

191191
expect(Container.clientManager.removeClient).toHaveBeenCalledWith(mockJiraSite);
192-
expect(Container.siteManager.removeSite).toHaveBeenCalledWith(mockJiraSite);
192+
expect(Container.siteManager.removeSite).toHaveBeenCalledWith(mockJiraSite, false, false);
193193
});
194194

195195
it('should return non-OAuth auth info without token refresh', async () => {

‎src/atlclients/authStore.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -214,7 +214,7 @@ export class CredentialManager implements Disposable {
214214
`Removing dead site for product ${site.product.key}: auth info not found in keychain`,
215215
);
216216
await Container.clientManager.removeClient(site);
217-
Container.siteManager.removeSite(site);
217+
await Container.siteManager.removeSite(site, false, false);
218218
}
219219
} else {
220220
// else if keychain does not exist, we check if the current site has been saved, if yes then we should remove it
@@ -226,7 +226,7 @@ export class CredentialManager implements Disposable {
226226
`Removing dead site for product ${site.product.key}: keychain not found`,
227227
);
228228
await Container.clientManager.removeClient(site);
229-
Container.siteManager.removeSite(site);
229+
await Container.siteManager.removeSite(site, false, false);
230230
}
231231
}
232232
}

‎src/siteManager.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -207,7 +207,7 @@ describe('SiteManager', () => {
207207

208208
storedSites.set(`${ProductJira.key}Sites`, [site]);
209209

210-
jest.spyOn(siteManager, 'removeSite').mockImplementation(() => true);
210+
jest.spyOn(siteManager, 'removeSite').mockImplementation(() => Promise.resolve(true));
211211

212212
const removeEvent: RemoveAuthInfoEvent = {
213213
type: AuthChangeType.Remove,
@@ -218,7 +218,7 @@ describe('SiteManager', () => {
218218

219219
siteManager.onDidAuthChange(removeEvent);
220220

221-
expect(siteManager.removeSite).toHaveBeenCalledWith(site);
221+
expect(siteManager.removeSite).toHaveBeenCalledWith(site, false, false);
222222
});
223223

224224
it('should fire sites available event when auth is updated', () => {
@@ -243,25 +243,25 @@ describe('SiteManager', () => {
243243
});
244244

245245
describe('removeSite', () => {
246-
it('should remove a site and clean up related resources', () => {
246+
it('should remove a site and clean up related resources', async () => {
247247
const site = createDetailedSiteInfo(ProductJira, 'site1');
248248

249249
storedSites.set(`${ProductJira.key}Sites`, [site]);
250250

251251
jest.spyOn(configuration, 'setLastCreateSiteAndProject');
252252

253-
const result = siteManager.removeSite(site);
253+
const result = await siteManager.removeSite(site);
254254

255255
expect(result).toBe(true);
256256
expect(mockGlobalStore.update).toHaveBeenCalledWith(`${ProductJira.key}Sites`, []);
257257
expect(mockCredentialManager.removeAuthInfo).toHaveBeenCalledWith(site);
258258
expect(configuration.setLastCreateSiteAndProject).toHaveBeenCalledWith(undefined);
259259
});
260260

261-
it('should return false if site is not found', () => {
261+
it('should return false if site is not found', async () => {
262262
const site = createDetailedSiteInfo(ProductJira);
263263

264-
const result = siteManager.removeSite(site);
264+
const result = await siteManager.removeSite(site);
265265

266266
expect(result).toBe(false);
267267
expect(mockGlobalStore.update).not.toHaveBeenCalled();

‎src/siteManager.ts

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
import { CredentialManager } from './atlclients/authStore';
1616
import { configuration } from './config/configuration';
1717
import { Container } from './container';
18+
import { Logger } from './logger';
1819

1920
export type SitesAvailableUpdateEvent = {
2021
sites: DetailedSiteInfo[];
@@ -114,12 +115,9 @@ export class SiteManager extends Disposable {
114115
onDidAuthChange(e: AuthInfoEvent) {
115116
if (isRemoveAuthEvent(e)) {
116117
const deadSites = this.getSitesAvailable(e.product).filter((site) => site.credentialId === e.credentialId);
117-
deadSites.forEach((s) => this.removeSite(s));
118+
118119
if (deadSites.length > 0) {
119-
this._onDidSitesAvailableChange.fire({
120-
sites: this.getSitesAvailable(e.product),
121-
product: e.product,
122-
});
120+
this.handleSiteRemoval(deadSites, e.product);
123121
}
124122
} else if (isUpdateAuthEvent(e)) {
125123
this._onDidSitesAvailableChange.fire({
@@ -129,6 +127,19 @@ export class SiteManager extends Disposable {
129127
}
130128
}
131129

130+
private async handleSiteRemoval(deadSites: DetailedSiteInfo[], product: Product): Promise<void> {
131+
try {
132+
const removalPromises = deadSites.map(async (s) => await this.removeSite(s, false, false));
133+
await Promise.all(removalPromises);
134+
this._onDidSitesAvailableChange.fire({
135+
sites: this.getSitesAvailable(product),
136+
product,
137+
});
138+
} catch (error) {
139+
Logger.error(error, 'Error during async site removal');
140+
}
141+
}
142+
132143
public getSitesAvailable(product: Product): DetailedSiteInfo[] {
133144
return this.getSitesAvailableForKey(product.key);
134145
}
@@ -247,7 +258,7 @@ export class SiteManager extends Disposable {
247258
return this.getSitesAvailable(product).find((site) => site.id === id);
248259
}
249260

250-
public removeSite(site: SiteInfo): boolean {
261+
public async removeSite(site: SiteInfo, removeCredentials = true, fireEvent = true): Promise<boolean> {
251262
const sites = this.readSitesFromGlobalStore(site.product.key);
252263
if (sites && sites.length > 0) {
253264
const foundIndex = sites.findIndex((availableSite) => availableSite.host === site.host);
@@ -256,8 +267,13 @@ export class SiteManager extends Disposable {
256267
sites.splice(foundIndex, 1);
257268
this._globalStore.update(`${site.product.key}${SitesSuffix}`, sites);
258269
this._sitesAvailable.set(site.product.key, sites);
259-
this._onDidSitesAvailableChange.fire({ sites: sites, product: site.product });
260-
Container.credentialManager.removeAuthInfo(deletedSite);
270+
271+
if (removeCredentials) {
272+
await Container.credentialManager.removeAuthInfo(deletedSite);
273+
}
274+
if (fireEvent) {
275+
this._onDidSitesAvailableChange.fire({ sites: sites, product: site.product });
276+
}
261277

262278
if (deletedSite.id === Container.config.jira.lastCreateSiteAndProject.siteId) {
263279
configuration.setLastCreateSiteAndProject(undefined);

‎src/webview/config/vscConfigActionApi.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ export class VSCConfigActionApi implements ConfigActionApi {
4949

5050
public async clearAuth(site: DetailedSiteInfo): Promise<void> {
5151
await Container.clientManager.removeClient(site);
52-
Container.siteManager.removeSite(site);
52+
await Container.siteManager.removeSite(site);
5353
}
5454

5555
public async fetchJqlOptions(site: DetailedSiteInfo): Promise<JQLAutocompleteData> {

‎src/webview/onboarding/vscOnboardingActionApi.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ export class VSCOnboardingActionApi implements OnboardingActionApi {
3636

3737
public async clearAuth(site: DetailedSiteInfo): Promise<void> {
3838
await Container.clientManager.removeClient(site);
39-
Container.siteManager.removeSite(site);
39+
await Container.siteManager.removeSite(site);
4040
}
4141

4242
public getSitesAvailable(): [DetailedSiteInfo[], DetailedSiteInfo[]] {

0 commit comments

Comments
 (0)