Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Hide bots and apps UI when managed by Apps Framework #7850

Merged
merged 9 commits into from
Apr 30, 2021
Merged

Hide bots and apps UI when managed by Apps Framework #7850

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6def23a
Hide bots and apps UI when managed by Apps Framework
larkox Apr 9, 2021
92360f1
Fix types and i18n
larkox Apr 9, 2021
f119431
Fix snapshot
larkox Apr 9, 2021
9e2f4b0
Make OAuthAppsIDs into OAuthAppIDs
larkox Apr 9, 2021
3e0e521
Address feedback
larkox Apr 12, 2021
2363f6e
Add tests
larkox Apr 12, 2021
459f5a6
Merge branch 'master' into HideAppsBotsUI
larkox Apr 13, 2021
4741d5a
don't hide bot account management controls
mickmister Apr 29, 2021
568529c
Merge branch 'master' into HideAppsBotsUI
mickmister Apr 29, 2021
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
Prev Previous commit
Next Next commit
Make OAuthAppsIDs into OAuthAppIDs
  • Loading branch information
@larkox
larkox committed Apr 9, 2021
commit 9e2f4b0538295945bce7c2befe3b330d903e3a8e
2 changes: 1 addition & 1 deletion actions/integration_actions.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ export function loadProfilesForCommands(commands) {
export function loadOAuthAppsAndProfiles(page = 0, perPage = DEFAULT_PAGE_SIZE) {
return async (dispatch, getState) => {
if (appsEnabled(getState())) {
dispatch(IntegrationActions.getAppsOAuthAppsIDs());
dispatch(IntegrationActions.getAppsOAuthAppIDs());
}
const {data} = await dispatch(IntegrationActions.getOAuthApps(page, perPage));
if (data) {
Expand Down
4 changes: 2 additions & 2 deletions components/integrations/installed_oauth_apps/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {connect} from 'react-redux';
import {bindActionCreators, Dispatch, ActionCreatorsMapObject} from 'redux';

import {regenOAuthAppSecret, deleteOAuthApp} from 'mattermost-redux/actions/integrations';
import {getAppsOAuthAppsIDs, getOAuthApps} from 'mattermost-redux/selectors/entities/integrations';
import {getAppsOAuthAppIDs, getOAuthApps} from 'mattermost-redux/selectors/entities/integrations';
import {haveISystemPermission} from 'mattermost-redux/selectors/entities/roles';
import {Permissions} from 'mattermost-redux/constants';
import {getConfig} from 'mattermost-redux/selectors/entities/general';
Expand All @@ -29,7 +29,7 @@ function mapStateToProps(state: GlobalState) {
return {
canManageOauth: haveISystemPermission(state, {permission: Permissions.MANAGE_OAUTH}),
oauthApps: getOAuthApps(state),
appsOAuthAppsIDs: appsEnabled(state) ? getAppsOAuthAppsIDs(state) : [],
appsOAuthAppIDs: appsEnabled(state) ? getAppsOAuthAppIDs(state) : [],
enableOAuthServiceProvider,
team: getCurrentTeam(state),
};
Expand Down
2 changes: 1 addition & 1 deletion components/integrations/installed_oauth_apps/installed_oauth_apps.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ describe('components/integrations/InstalledOAuthApps', () => {
deleteOAuthApp: jest.fn(),
},
enableOAuthServiceProvider: true,
appsOAuthAppsIDs: [],
appsOAuthAppIDs: [],
};

test('should match snapshot', () => {
Expand Down
4 changes: 2 additions & 2 deletions components/integrations/installed_oauth_apps/installed_oauth_apps.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ type Props = {
/**
* List of IDs for apps managed by the App Framwork
*/
appsOAuthAppsIDs: string[];
appsOAuthAppIDs: string[];

/**
* Set if user can manage oath
Expand Down Expand Up @@ -112,7 +112,7 @@ export default class InstalledOAuthApps extends React.PureComponent<Props, State
onDelete={this.deleteOAuthApp}
team={this.props.team}
creatorName=''
fromApp={this.props.appsOAuthAppsIDs.includes(app.id)}
fromApp={this.props.appsOAuthAppIDs.includes(app.id)}
/>
);
});
Expand Down
4 changes: 2 additions & 2 deletions packages/mattermost-redux/src/actions/integrations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,9 +293,9 @@ export function getOAuthApps(page = 0, perPage: number = General.PAGE_SIZE_DEFAU
});
}

export function getAppsOAuthAppsIDs(): ActionFunc {
export function getAppsOAuthAppIDs(): ActionFunc {
return bindClientFunc({
clientFunc: Client4.getAppsOAuthAppsIDs,
clientFunc: Client4.getAppsOAuthAppIDs,
onSuccess: [IntegrationTypes.RECEIVED_APPS_OAUTH_APP_IDS],
Copy link
Member

Choose a reason for hiding this comment

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

Do we need the arrays around the onSuccess values? It looks inconsistent throughout the file, but it caught me off guard at first glance here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. It is not needed. Nevertheless, for consistency, I would prefer to keep it as a list.

Copy link
Member

Choose a reason for hiding this comment

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

The array is left over from when we always used to have a _SUCCESS action dispatched for every request in addition to something like a POST_RECEIVED action. I think it might still be used in some cases, but we could definitely consider adding a non-array form in the future.

});
}
Expand Down
4 changes: 2 additions & 2 deletions packages/mattermost-redux/src/client/client4.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2548,7 +2548,7 @@ export default class Client4 {
);
};

getAppsOAuthAppsIDs = () => {
getAppsOAuthAppIDs = () => {
Copy link
Member

Choose a reason for hiding this comment

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

Is there any chance either of these will ever need paging in the future? That's obviously a bigger question than just something that affects the web app, but I find it never hurts to ask

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Being small amount of information (a list of ids) I hope there is no case where this may become a problem (e.g. having 100 Apps installed?). That being said, I am open to suggestions or comments.

return this.doFetch<string[]>(
`${this.getAppsProxyRoute()}/api/v1/oauth-app-ids`,
{method: 'get'},
Expand All @@ -2557,7 +2557,7 @@ export default class Client4 {

getAppsBotIDs = () => {
return this.doFetch<string[]>(
`${this.getAppsProxyRoute()}/api/v1/bots-ids`,
`${this.getAppsProxyRoute()}/api/v1/bot-ids`,
{method: 'get'},
);
}
Expand Down
4 changes: 2 additions & 2 deletions packages/mattermost-redux/src/reducers/entities/integrations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ function oauthApps(state: IDMappedObjects<OAuthApp> = {}, action: GenericAction)
}
}

function appsOAuthAppsIDs(state: string[] = [], action: GenericAction) {
function appsOAuthAppIDs(state: string[] = [], action: GenericAction) {
switch (action.type) {
case IntegrationTypes.RECEIVED_APPS_OAUTH_APP_IDS: {
if (state.length === 0 && action.data.length === 0) {
Expand Down Expand Up @@ -288,7 +288,7 @@ export default combineReducers({
oauthApps,

// object to represent the list of ids for oauth apps associated to apps
appsOAuthAppsIDs,
appsOAuthAppIDs: appsOAuthAppIDs,

// object to represent the list of ids for bots associated to apps
appsBotIDs,
Expand Down
4 changes: 2 additions & 2 deletions packages/mattermost-redux/src/selectors/entities/integrations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,8 @@ export function getOAuthApps(state: GlobalState) {
return state.entities.integrations.oauthApps;
}

export function getAppsOAuthAppsIDs(state: GlobalState) {
return state.entities.integrations.appsOAuthAppsIDs;
export function getAppsOAuthAppIDs(state: GlobalState) {
return state.entities.integrations.appsOAuthAppIDs;
}

export function getAppsBotIDs(state: GlobalState) {
Expand Down
2 changes: 1 addition & 1 deletion packages/mattermost-redux/src/store/initial_state.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ const state: GlobalState = {
systemCommands: {},
commands: {},
appsBotIDs: [],
appsOAuthAppsIDs: [],
appsOAuthAppIDs: [],
},
files: {
files: {},
Expand Down
2 changes: 1 addition & 1 deletion packages/mattermost-redux/src/types/integrations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export type IntegrationsState = {
incomingHooks: IDMappedObjects<IncomingWebhook>;
outgoingHooks: IDMappedObjects<OutgoingWebhook>;
oauthApps: IDMappedObjects<OAuthApp>;
appsOAuthAppsIDs: string[];
appsOAuthAppIDs: string[];
appsBotIDs: string[];
systemCommands: IDMappedObjects<Command>;
commands: IDMappedObjects<Command>;
Expand Down