-
Notifications
You must be signed in to change notification settings - Fork 662
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
Add support for deletion of an installation (InstallationStore / MemoryInstallationStore) #1272
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -184,16 +184,31 @@ const installer = new InstallProvider({ | |||||
// returns installation object from database | ||||||
fetchInstallation: async (installQuery) => { | ||||||
// replace myDB.get with your own database or OEM getter | ||||||
if (query.isEnterpriseInstall && query.enterpriseId !== undefined) { | ||||||
if (installQuery.isEnterpriseInstall && installQuery.enterpriseId !== undefined) { | ||||||
// org wide app installation lookup | ||||||
return await myDB.get(installQuery.enterpriseId); | ||||||
} | ||||||
if (query.teamId !== undefined) { | ||||||
if (installQuery.teamId !== undefined) { | ||||||
// single team app installation lookup | ||||||
return await myDB.get(installQuery.teamId); | ||||||
} | ||||||
throw new Error('Failed fetching installation'); | ||||||
}, | ||||||
// takes in an installQuery as an argument | ||||||
// installQuery = {teamId: 'string', enterpriseId: 'string', userId: 'string', conversationId: 'string', isEnterpriseInstall: boolean}; | ||||||
// returns nothing | ||||||
deleteInstallation: async (installQuery) => { | ||||||
// replace myDB.get with your own database or OEM getter | ||||||
if (query.isEnterpriseInstall && query.enterpriseId !== undefined) { | ||||||
// org wide app installation deletion | ||||||
return await myDB.delete(installQuery.enterpriseId); | ||||||
} | ||||||
if (query.teamId !== undefined) { | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
// single team app installation deletion | ||||||
return await myDB.delete(installQuery.teamId); | ||||||
} | ||||||
throw new Error('Failed to delete installation'); | ||||||
}, | ||||||
}, | ||||||
}); | ||||||
``` | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -456,9 +456,12 @@ export interface InstallationStore { | |
logger?: Logger): Promise<void>; | ||
fetchInstallation: | ||
(query: InstallationQuery<boolean>, logger?: Logger) => Promise<Installation<'v1' | 'v2', boolean>>; | ||
deleteInstallation: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we consider making this optional incase folks have created their own installationStores. By making it required, it would mean their installationStores are invalid when they upgrade. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yeah, great point. Thanks. |
||
(query: InstallationQuery<boolean>, logger?: Logger) => Promise<void>; | ||
} | ||
|
||
// using a javascript object as a makeshift database for development | ||
// storing user tokens is not supported | ||
interface DevDatabase { | ||
[teamIdOrEnterpriseId: string]: Installation; | ||
} | ||
|
@@ -513,6 +516,32 @@ class MemoryInstallationStore implements InstallationStore { | |
} | ||
throw new Error('Failed fetching installation'); | ||
} | ||
|
||
public async deleteInstallation(query: InstallationQuery<boolean>, logger?: Logger): Promise<void> { | ||
if (logger !== undefined) { | ||
logger.warn('Deleting Access Token from DB. Please use a real Installation Store for production!'); | ||
} | ||
|
||
if (query.isEnterpriseInstall && query.enterpriseId !== undefined) { | ||
if (logger !== undefined) { | ||
logger.debug('deleting org installation'); | ||
} | ||
|
||
const { [query.enterpriseId]: _, ...devDB } = this.devDB; | ||
this.devDB = devDB; | ||
|
||
} else if (query.teamId !== undefined) { | ||
if (logger !== undefined) { | ||
logger.debug('deleting single team installation'); | ||
} | ||
|
||
const { [query.teamId]: _, ...devDB } = this.devDB; | ||
this.devDB = devDB; | ||
|
||
} else { | ||
throw new Error('Failed to delete installation'); | ||
} | ||
} | ||
} | ||
|
||
/** | ||
|
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.
Can you fix the same error for
fetchInstallation
method?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 catch on both of these. Will update!