-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Hide bots and apps UI when managed by Apps Framework #7850
Changes from all commits
6def23a
92360f1
f119431
9e2f4b0
3e0e521
2363f6e
459f5a6
4741d5a
568529c
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 |
---|---|---|
|
@@ -21,6 +21,7 @@ describe('components/integrations/bots/Bots', () => { | |
getUser: jest.fn(), | ||
disableBot: jest.fn(), | ||
enableBot: jest.fn(), | ||
fetchAppsBotIDs: jest.fn(), | ||
}; | ||
|
||
it('bots', () => { | ||
|
@@ -46,6 +47,8 @@ describe('components/integrations/bots/Bots', () => { | |
owners={{}} | ||
users={users} | ||
actions={actions} | ||
appsEnabled={false} | ||
appsBotIDs={[]} | ||
/>, | ||
); | ||
wrapperFull.instance().setState({loading: false}); | ||
|
@@ -60,6 +63,7 @@ describe('components/integrations/bots/Bots', () => { | |
accessTokens={{}} | ||
team={team} | ||
actions={actions} | ||
fromApp={false} | ||
/>, | ||
)).toEqual(true); | ||
expect(wrapper.find('EnabledSection').shallow().contains( | ||
|
@@ -71,6 +75,7 @@ describe('components/integrations/bots/Bots', () => { | |
accessTokens={{}} | ||
team={team} | ||
actions={actions} | ||
fromApp={false} | ||
/>, | ||
)).toEqual(true); | ||
expect(wrapper.find('EnabledSection').shallow().contains( | ||
|
@@ -82,6 +87,75 @@ describe('components/integrations/bots/Bots', () => { | |
accessTokens={{}} | ||
team={team} | ||
actions={actions} | ||
fromApp={false} | ||
/>, | ||
)).toEqual(true); | ||
}); | ||
|
||
it('bots with bots from apps', () => { | ||
const bot1 = TestHelper.getBotMock({user_id: '1'}); | ||
const bot2 = TestHelper.getBotMock({user_id: '2'}); | ||
const bot3 = TestHelper.getBotMock({user_id: '3'}); | ||
const bots = { | ||
[bot1.user_id]: bot1, | ||
[bot2.user_id]: bot2, | ||
[bot3.user_id]: bot3, | ||
}; | ||
const users = { | ||
[bot1.user_id]: TestHelper.getUserMock({id: bot1.user_id}), | ||
[bot2.user_id]: TestHelper.getUserMock({id: bot2.user_id}), | ||
[bot3.user_id]: TestHelper.getUserMock({id: bot3.user_id}), | ||
}; | ||
|
||
const wrapperFull = shallow( | ||
<Bots | ||
bots={bots} | ||
team={team} | ||
accessTokens={{}} | ||
owners={{}} | ||
users={users} | ||
actions={actions} | ||
appsEnabled={true} | ||
appsBotIDs={[bot3.user_id]} | ||
/>, | ||
); | ||
wrapperFull.instance().setState({loading: false}); | ||
const wrapper = shallow(<div>{(wrapperFull.instance() as Bots).bots()[0]}</div>); | ||
|
||
expect(wrapper.find('EnabledSection').shallow().contains( | ||
<Bot | ||
key={bot1.user_id} | ||
bot={bot1} | ||
owner={undefined} | ||
user={users[bot1.user_id]} | ||
accessTokens={{}} | ||
team={team} | ||
actions={actions} | ||
fromApp={false} | ||
/>, | ||
)).toEqual(true); | ||
expect(wrapper.find('EnabledSection').shallow().contains( | ||
<Bot | ||
key={bot2.user_id} | ||
bot={bot2} | ||
owner={undefined} | ||
user={users[bot2.user_id]} | ||
accessTokens={{}} | ||
team={team} | ||
actions={actions} | ||
fromApp={false} | ||
/>, | ||
)).toEqual(true); | ||
expect(wrapper.find('EnabledSection').shallow().contains( | ||
<Bot | ||
key={bot3.user_id} | ||
bot={bot3} | ||
owner={undefined} | ||
user={users[bot3.user_id]} | ||
accessTokens={{}} | ||
team={team} | ||
actions={actions} | ||
fromApp={true} | ||
/>, | ||
)).toEqual(true); | ||
}); | ||
|
@@ -120,6 +194,8 @@ describe('components/integrations/bots/Bots', () => { | |
owners={owners} | ||
users={users} | ||
actions={actions} | ||
appsEnabled={false} | ||
appsBotIDs={[]} | ||
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. Same here, can we have some tests that have app bots involved? |
||
/>, | ||
); | ||
wrapperFull.instance().setState({loading: false}); | ||
|
@@ -134,6 +210,7 @@ describe('components/integrations/bots/Bots', () => { | |
accessTokens={passedTokens} | ||
team={team} | ||
actions={actions} | ||
fromApp={false} | ||
/>, | ||
)).toEqual(true); | ||
}); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,6 +25,16 @@ type Props = { | |
*/ | ||
bots: Record<string, BotType>; | ||
|
||
/** | ||
* List of bot IDs managed by the app framework | ||
*/ | ||
appsBotIDs: string[]; | ||
|
||
/** | ||
* Whether apps framework is enabled | ||
*/ | ||
appsEnabled: boolean; | ||
|
||
/** | ||
* Map from botUserId to accessTokens. | ||
*/ | ||
|
@@ -79,6 +89,11 @@ type Props = { | |
* Enable a bot | ||
*/ | ||
enableBot: (userId: string) => Promise<ActionResult>; | ||
|
||
/** | ||
* Load bot IDs managed by the apps | ||
*/ | ||
fetchAppsBotIDs: () => Promise<ActionResult>; | ||
}; | ||
|
||
/** | ||
|
@@ -124,6 +139,9 @@ export default class Bots extends React.PureComponent<Props, State> { | |
} | ||
}, | ||
); | ||
if (this.props.appsEnabled) { | ||
this.props.actions.fetchAppsBotIDs(); | ||
} | ||
} | ||
|
||
DisabledSection(props: {hasDisabled: boolean; disabledBots: JSX.Element[]; filter?: string}): JSX.Element | null { | ||
|
@@ -169,6 +187,7 @@ export default class Bots extends React.PureComponent<Props, State> { | |
accessTokens={(this.props.accessTokens && this.props.accessTokens[bot.user_id]) || {}} | ||
actions={this.props.actions} | ||
team={this.props.team} | ||
fromApp={this.props.appsBotIDs.includes(bot.user_id)} | ||
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. Is there an async operation that could momentarily have 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. If getAppBotIDs takes too long, the user will see the whole information, and then disappear when that information finally arrives. We could keep the loading state until we have all the information. What do you think? 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. I think we should keep showing the loading animation. But if the bot id fetch fails, then show all of the information. |
||
/> | ||
); | ||
}; | ||
|
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 we add a test for a bot that has
fromApp={true}
?