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

MM-28733 : Admin Advisor v2 #6461

Merged
merged 25 commits into from
Sep 25, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
d608ef5
temp work
catalintomai Sep 12, 2020
cef507b
Merge branch 'master' into OCTO-6
catalintomai Sep 16, 2020
1fa0814
cleanup + make check-style fixes
catalintomai Sep 16, 2020
1f34c72
add support for 500k banner warning
catalintomai Sep 17, 2020
5a97f51
remove ContextUs texts from the StartTrial version of the modal
catalintomai Sep 17, 2020
2e04eb4
fix localization file
catalintomai Sep 17, 2020
0ccfee7
address CR comments
catalintomai Sep 17, 2020
d9cce21
address CR comments
catalintomai Sep 17, 2020
951a887
address CR comments
catalintomai Sep 17, 2020
35b3e35
addressed CR comments
catalintomai Sep 18, 2020
ecbbc9f
addressed CR comments
catalintomai Sep 18, 2020
a984fab
Merge branch 'master' into OCTO-6
catalintomai Sep 19, 2020
a5b096f
removed StartTrial from webapp following PR spec update
catalintomai Sep 20, 2020
7d70359
refactor multiple warn metric banners handling
catalintomai Sep 20, 2020
da1c74d
remove enterpriseReady references
catalintomai Sep 20, 2020
c6972b7
clean up i18n file
catalintomai Sep 20, 2020
fb5878c
updated mailto strings
catalintomai Sep 20, 2020
8c33ee5
addressed CR comments
catalintomai Sep 22, 2020
154758f
Merge branch 'master' into OCTO-6
catalintomai Sep 22, 2020
e5f1a91
updated ack banner text
catalintomai Sep 23, 2020
305c792
fix Jest test
catalintomai Sep 23, 2020
3dfbab3
address PM spec updates
catalintomai Sep 24, 2020
e89862f
make sure we don't display banner if warn metric is invalid
catalintomai Sep 24, 2020
85ca2b6
Merge branch 'master' into OCTO-6
catalintomai Sep 24, 2020
a2512d9
Merge branch 'master' into OCTO-6
catalintomai Sep 25, 2020
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
Next Next commit
temp work
  • Loading branch information
catalintomai committed Sep 12, 2020
commit d608ef5a044f6e4a10ce3cae0fc3230c8b31123d
100 changes: 70 additions & 30 deletions components/announcement_bar/configuration_bar/configuration_bar.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ class ConfigurationAnnouncementBar extends React.PureComponent {
this.props.actions.dismissNotice(AnnouncementBarMessages.NUMBER_OF_ACTIVE_USERS_WARN_METRIC_STATUS_ACK);
}

getNoticeForWarnMetric = (warnMetricStatus) => {
getNoticeForWarnMetric = (warnMetricStatus, isE0Edition) => {
if (!warnMetricStatus) {
return null;
}
Expand All @@ -65,41 +65,76 @@ class ConfigurationAnnouncementBar extends React.PureComponent {
var canCloseBar = false;

switch (warnMetricStatus.id) {
case WarnMetricTypes.SYSTEM_WARN_METRIC_NUMBER_OF_POSTS_500K:
case WarnMetricTypes.SYSTEM_WARN_METRIC_NUMBER_OF_ACTIVE_USERS_500:
if (warnMetricStatus.acked) {
message = (
<React.Fragment>
<img
className='advisor-icon'
src={ackIcon}
/>
<FormattedMarkdownMessage
id={t('announcement_bar.error.number_active_users_warn_metric_status_ack.text')}
defaultMessage={'Your trial has started! Go to the [System Console](/admin_console/environment/web_server) to check out the new features.'}
/>
</React.Fragment>
);
if (isE0Edition) {
message = (
<React.Fragment>
<img
className='advisor-icon'
src={ackIcon}
/>
<FormattedMarkdownMessage
id={t('announcement_bar.error.number_active_users_warn_metric_status_ack.text')}
catalintomai marked this conversation as resolved.
Show resolved Hide resolved
defaultMessage={'Your trial has started! Go to the [System Console](/admin_console/environment/web_server) to check out the new features.'}
catalintomai marked this conversation as resolved.
Show resolved Hide resolved
catalintomai marked this conversation as resolved.
Show resolved Hide resolved
/>
</React.Fragment>
);
} else {
message = (
<React.Fragment>
<img
className='advisor-icon'
src={ackIcon}
/>
<FormattedMarkdownMessage
catalintomai marked this conversation as resolved.
Show resolved Hide resolved
id={t('announcement_bar.warn_metric_status_ack.text')}
catalintomai marked this conversation as resolved.
Show resolved Hide resolved
defaultMessage={'Thank you for contacting Mattermost. We will follow up with you soon.'}
catalintomai marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

We're not going to mention that they're going to need to install a different instance? What's up with that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll defer to @esadur and @jasonblais on this, they review the texts.

Copy link
Contributor

Choose a reason for hiding this comment

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

@mkraft, I'm not sure I understand the concern, can you elaborate a bit?

Copy link
Contributor

Choose a reason for hiding this comment

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

We are linking to documentation in almost all cases so that they can explore the feature and determine if it would be valuable to them. I would rather lead with the value of the features in question so they can make that determination for themselves.

catalintomai marked this conversation as resolved.
Show resolved Hide resolved
/>
</React.Fragment>
);
}

type = AnnouncementBarTypes.ADVISOR_ACK;
showModal = false;
dismissFunc = this.dismissNumberOfActiveUsersWarnMetricAck;
isDismissed = this.props.dismissedNumberOfActiveUsersWarnMetricStatusAck;
canCloseBar = true;
} else {
message = (
<React.Fragment>
<img
className='advisor-icon'
src={alertIcon}
/>
<FormattedMarkdownMessage
id={t('announcement_bar.error.number_active_users_warn_metric_status.text')}
defaultMessage={'You now have over {limit} users. We strongly recommend using advanced features for large-scale servers.'}
values={{
limit: warnMetricStatus.limit,
}}
/>
</React.Fragment>
);
if (warnMetricStatus.id === WarnMetricTypes.SYSTEM_WARN_METRIC_NUMBER_OF_ACTIVE_USERS_500) {
message = (
<React.Fragment>
<img
className='advisor-icon'
src={alertIcon}
/>
<FormattedMarkdownMessage
id={t('announcement_bar.error.number_active_users_warn_metric_status.text')}
catalintomai marked this conversation as resolved.
Show resolved Hide resolved
defaultMessage={'You now have over {limit} users. We strongly recommend using advanced features for large-scale servers.'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage={'You now have over {limit} users. We strongly recommend using advanced features for large-scale servers.'}
defaultMessage='You now have over {limit} users. We strongly recommend using advanced features for large-scale servers.'

Copy link
Contributor

Choose a reason for hiding this comment

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

Users are more likely to take action if you tell them exactly what you want them to do and why. This is so vague we're making them interpret it for themselves and it's not clear what we're asking of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll defer to @esadur and @jasonblais on this, they review the texts.

Copy link
Contributor

Choose a reason for hiding this comment

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

These are the banners, and there's only so much space for text. We go into more detail in the modals.

values={{
limit: warnMetricStatus.limit,
}}
/>
</React.Fragment>
);
} else {
message = (
<React.Fragment>
<img
className='advisor-icon'
src={alertIcon}
/>
<FormattedMarkdownMessage
id={t('announcement_bar.number_posts_warn_metric_status.text')}
defaultMessage={'You now have over {limit} posts. We strongly recommend using advanced features for large-scale servers.'}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage={'You now have over {limit} posts. We strongly recommend using advanced features for large-scale servers.'}
defaultMessage='You now have over {limit} posts. We strongly recommend using advanced features for large-scale servers.'

Copy link
Contributor

Choose a reason for hiding this comment

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

Same 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.

I'll defer to @esadur and @jasonblais on this, they review the texts.

Copy link
Contributor

Choose a reason for hiding this comment

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

These are the banners, and there's only so much space for text. We go into more detail in the modals.

values={{
limit: warnMetricStatus.limit,
}}
/>
</React.Fragment>
);
}
type = AnnouncementBarTypes.ADVISOR;
showModal = true;
dismissFunc = this.dismissNumberOfActiveUsersWarnMetric;
Expand Down Expand Up @@ -176,9 +211,14 @@ class ConfigurationAnnouncementBar extends React.PureComponent {
/>
);
}
if (this.props.license.IsLicensed === 'false' && this.props.warnMetricsStatus) {
if (this.props.license &&
this.props.license.IsLicensed === 'false' &&
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use optional chaining here? Not sure if it works or not

Suggested change
this.props.license.IsLicensed === 'false' &&
this.props.license?.IsLicensed === 'false' &&

Maybe?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The current pattern in the code is the one being used here as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

See above comment

this.props.warnMetricsStatus) {
const enterpriseReady = (this.props.config.BuildEnterpriseReady === 'true');
catalintomai marked this conversation as resolved.
Show resolved Hide resolved
console.log('CITOMAI enterpriseReady' + enterpriseReady);

for (const status of Object.values(this.props.warnMetricsStatus)) {
var notice = this.getNoticeForWarnMetric(status);
var notice = this.getNoticeForWarnMetric(status, enterpriseReady);
if (!notice || notice.IsDismissed) {
continue;
}
Expand Down
8 changes: 7 additions & 1 deletion components/warn_metric_ack_modal/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,8 @@ import {connect} from 'react-redux';

import {GlobalState} from 'mattermost-redux/types/store';
import {ActionFunc} from 'mattermost-redux/types/actions';
import {getStandardAnalytics, sendWarnMetricAck} from 'mattermost-redux/actions/admin';
import {getStandardAnalytics, sendWarnMetricAck, requestTrialLicenseAndAckWarnMetric} from 'mattermost-redux/actions/admin';
import {getLicenseConfig} from 'mattermost-redux/actions/general';

import {getCurrentUser} from 'mattermost-redux/selectors/entities/common';
import {getConfig, getLicense} from 'mattermost-redux/selectors/entities/general';
Expand All @@ -32,13 +33,16 @@ function mapStateToProps(state: GlobalState, ownProps: Props) {
diagnosticId: config.DiagnosticId,
show: isModalOpen(state, ModalIdentifiers.WARN_METRIC_ACK),
closeParentComponent: ownProps.closeParentComponent,
enterpriseReady: config.BuildEnterpriseReady === 'true',
};
}

type Actions = {
closeModal: (arg: string) => void;
getStandardAnalytics: () => any;
sendWarnMetricAck: (arg0: string, arg1: boolean) => ActionFunc & Partial<{error?: string}>;
requestTrialLicenseAndAckWarnMetric: (arg0: string) => ActionFunc & Partial<{error?: string}>;
getLicenseConfig: () => void;
};

function mapDispatchToProps(dispatch: Dispatch) {
Expand All @@ -48,6 +52,8 @@ function mapDispatchToProps(dispatch: Dispatch) {
closeModal,
getStandardAnalytics,
sendWarnMetricAck,
requestTrialLicenseAndAckWarnMetric,
getLicenseConfig,
},
dispatch,
),
Expand Down
27 changes: 25 additions & 2 deletions components/warn_metric_ack_modal/warn_metric_ack_modal.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ jest.mock('react-dom', () => ({

describe('components/WarnMetricAckModal', () => {
const serverError = 'some error';
const gettingTrialError = 'some error';

const baseProps = {
stats: {
Expand All @@ -36,10 +37,16 @@ describe('components/WarnMetricAckModal', () => {
limit: 500,
acked: false,
},
license: {
IsLicensed: 'false',
},
enterpriseReady: false,
actions: {
closeModal: jest.fn(),
getStandardAnalytics: jest.fn(),
sendWarnMetricAck: jest.fn(),
requestTrialLicenseAndAckWarnMetric: jest.fn(),
getLicenseConfig: jest.fn(),
},
};

Expand Down Expand Up @@ -81,14 +88,30 @@ describe('components/WarnMetricAckModal', () => {
expect(wrapper.state('saving')).toEqual(false);
});

test('send ack on start trial button click', () => {
const props = {...baseProps};
props.enterpriseReady = true;

const wrapper = shallow<WarnMetricAckModal>(
<WarnMetricAckModal {...props}/>,
);

wrapper.setState({gettingTrial: false});
wrapper.find('.save-button').simulate('click');
expect(props.actions.requestTrialLicenseAndAckWarnMetric).toHaveBeenCalledTimes(1);
});

test('send ack on acknowledge button click', () => {
const props = {...baseProps};
props.enterpriseReady = false;

const wrapper = shallow<WarnMetricAckModal>(
<WarnMetricAckModal {...baseProps}/>,
<WarnMetricAckModal {...props}/>,
);

wrapper.setState({saving: false});
wrapper.find('.save-button').simulate('click');
expect(baseProps.actions.sendWarnMetricAck).toHaveBeenCalledTimes(1);
expect(props.actions.sendWarnMetricAck).toHaveBeenCalledTimes(1);
});

test('should have called props.onHide when Modal.onExited is called', () => {
Expand Down
Loading