-
Notifications
You must be signed in to change notification settings - Fork 9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[AXON-17] Fix weird site list behaviour #79
base: main
Are you sure you want to change the base?
Conversation
07415bd
to
d2b7f08
Compare
d2b7f08
to
041356e
Compare
041356e
to
92bd5e9
Compare
@@ -120,6 +121,10 @@ export interface DetailedSiteInfo extends SiteInfo { | |||
credentialId: string; | |||
} | |||
|
|||
export function getSiteInfoKey(site: DetailedSiteInfo): string { | |||
return `${site.product.key} - ${site.host} - ${site.credentialId}`; | |||
} |
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.
This bit here is more or less the key to everything - we can't index credential stuff with only credentialId
since the credential is shared across all JIRA sites the user has access to when doing 3LO. So, let's use host
in addition - and the keyword of jira
/bitbucket
for good measure ;)
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.
Good idea on the product key! 🎉
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.
New bug is introduced.
What is this
This PR addresses the weird behaviour that happens whenever we try to delete one site from several sharing a credential:
After the change, it's possible to remove the sites individually. However, while working on this, I've found a couple other rather frustrating issues which will require a more comprehensive rework 😢
Specifically:
I've tried addressing these quickly - but since ultimately they are not a priority at the moment, I've left this PR at addressing the actual problem we got reported 😛
How was this tested?
Since this is mainly a UI change - by setting up the extension and clicking around a bunch? 😉