Always show `InstallWarning` when we could (#8649)

* Always show `InstallWarning` when we could

* Address review comments

src/amo/components/InstallWarning/index.js

@@ -5,13 +5,7 @@ import { connect } from 'react-redux';
 import { compose } from 'redux';
 
 import type { AppState } from 'amo/store';
-import { hasAddonManager } from 'core/addonManager';
-import {
-  ADDON_TYPE_EXTENSION,
-  CLIENT_APP_FIREFOX,
-  UNINSTALLED,
-  UNKNOWN,
-} from 'core/constants';
+import { ADDON_TYPE_EXTENSION, CLIENT_APP_FIREFOX } from 'core/constants';
 import translate from 'core/i18n/translate';
 import log from 'core/logger';
 import tracking from 'core/tracking';
@@ -19,7 +13,6 @@ import { isFirefox } from 'core/utils/compatibility';
 import { withExperiment } from 'core/withExperiment';
 import Notice from 'ui/components/Notice';
 import type { UserAgentInfoType } from 'core/reducers/api';
-import type { InstalledAddon } from 'core/reducers/installations';
 import type { AddonType } from 'core/types/addons';
 import type { I18nType } from 'core/types/i18n';
 import type { WithExperimentInjectedProps } from 'core/withExperiment';
@@ -34,13 +27,11 @@ type InternalProps = {|
   ...Props,
   ...WithExperimentInjectedProps,
   _couldShowWarning?: () => boolean,
-  _hasAddonManager: typeof hasAddonManager,
   _log: typeof log,
   _tracking: typeof tracking,
   clientApp: string,
   className?: string,
   i18n: I18nType,
-  installStatus: $PropertyType<InstalledAddon, 'status'>,
   userAgentInfo: UserAgentInfoType,
 |};
 
@@ -59,7 +50,6 @@ const WARNING_LINK_DESTINATION =
 
 export class InstallWarningBase extends React.Component<InternalProps> {
   static defaultProps = {
-    _hasAddonManager: hasAddonManager,
     _log: log,
     _tracking: tracking,
   };
@@ -67,11 +57,9 @@ export class InstallWarningBase extends React.Component<InternalProps> {
   couldShowWarning = () => {
     const {
       _couldShowWarning,
-      _hasAddonManager,
       addon,
       clientApp,
       isExperimentEnabled,
-      installStatus,
       userAgentInfo,
     } = this.props;
     return _couldShowWarning
@@ -80,9 +68,7 @@ export class InstallWarningBase extends React.Component<InternalProps> {
           clientApp === CLIENT_APP_FIREFOX &&
           addon.type === ADDON_TYPE_EXTENSION &&
           !addon.is_recommended &&
-          isExperimentEnabled &&
-          (!_hasAddonManager() ||
-            [UNINSTALLED, UNKNOWN].includes(installStatus));
+          isExperimentEnabled;
   };
 
   maybeSendDisplayTrackingEvent = () => {
@@ -115,14 +101,6 @@ export class InstallWarningBase extends React.Component<InternalProps> {
     this.maybeSendDisplayTrackingEvent();
   }
 
-  componentDidUpdate({ installStatus: oldInstallStatus }: InternalProps) {
-    const { installStatus: newInstallStatus } = this.props;
-
-    if (newInstallStatus !== oldInstallStatus) {
-      this.maybeSendDisplayTrackingEvent();
-    }
-  }
-
   render() {
     const { className, i18n, variant } = this.props;
 
@@ -144,13 +122,9 @@ export class InstallWarningBase extends React.Component<InternalProps> {
   }
 }
 
-export const mapStateToProps = (state: AppState, ownProps: InternalProps) => {
-  const { addon } = ownProps;
-  const installedAddon = (addon && state.installations[addon.guid]) || {};
-
+export const mapStateToProps = (state: AppState) => {
   return {
     clientApp: state.api.clientApp,
-    installStatus: installedAddon.status,
     userAgentInfo: state.api.userAgentInfo,
   };
 };

tests/unit/amo/components/TestInstallWarning.js

@@ -8,15 +8,11 @@ import InstallWarning, {
   VARIANT_EXCLUDE_WARNING,
   InstallWarningBase,
 } from 'amo/components/InstallWarning';
-import { setInstallState } from 'core/actions/installations';
 import {
   ADDON_TYPE_EXTENSION,
   ADDON_TYPE_STATIC_THEME,
   CLIENT_APP_ANDROID,
   CLIENT_APP_FIREFOX,
-  INSTALLED,
-  UNINSTALLED,
-  UNKNOWN,
 } from 'core/constants';
 import { createInternalAddon } from 'core/reducers/addons';
 import {
@@ -24,7 +20,6 @@ import {
   dispatchClientMetadata,
   fakeAddon,
   fakeI18n,
-  fakeInstalledAddon,
   getFakeLogger,
   shallowUntilTarget,
   userAgentsByPlatform,
@@ -55,16 +50,6 @@ describe(__filename, () => {
     );
   };
 
-  const _setInstallStatus = ({ addon, status }) => {
-    store.dispatch(
-      setInstallState({
-        ...fakeInstalledAddon,
-        guid: addon.guid,
-        status,
-      }),
-    );
-  };
-
   describe('couldShowWarning', () => {
     const addonThatWouldShowWarning = {
       ...fakeAddon,
@@ -76,7 +61,6 @@ describe(__filename, () => {
     // couldShowWarning to be true.
     const renderWithWarning = (props = {}) => {
       return render({
-        _hasAddonManager: sinon.stub().returns(false),
         addon: createInternalAddon(addonThatWouldShowWarning),
         isExperimentEnabled: true,
         ...props,
@@ -98,29 +82,6 @@ describe(__filename, () => {
       expect(component.instance().couldShowWarning()).toEqual(true);
     });
 
-    it.each([UNINSTALLED, UNKNOWN])(
-      `returns true if mozAddonManager exists and the add-on has a status of %s`,
-      (installStatus) => {
-        const addon = addonThatWouldShowWarning;
-        _setInstallStatus({ addon, status: installStatus });
-
-        const component = renderWithWarning({
-          _hasAddonManager: sinon.stub().returns(true),
-          addon: createInternalAddon(addon),
-        });
-
-        expect(component.instance().couldShowWarning()).toEqual(true);
-      },
-    );
-
-    it('returns true regardless of status if there is no mozAddonManager', () => {
-      const component = renderWithWarning({
-        _hasAddonManager: sinon.stub().returns(false),
-      });
-
-      expect(component.instance().couldShowWarning()).toEqual(true);
-    });
-
     it('returns false if the add-on is not an extension', () => {
       const component = renderWithWarning({
         addon: createInternalAddon({
@@ -143,18 +104,6 @@ describe(__filename, () => {
       expect(component.instance().couldShowWarning()).toEqual(false);
     });
 
-    it('returns false if mozAddonManager exists and the addon does not have a status of UNINSTALLED or UNKNOWN', () => {
-      const addon = addonThatWouldShowWarning;
-      _setInstallStatus({ addon, status: INSTALLED });
-
-      const component = renderWithWarning({
-        _hasAddonManager: sinon.stub().returns(true),
-        addon: createInternalAddon(addon),
-      });
-
-      expect(component.instance().couldShowWarning()).toEqual(false);
-    });
-
     it('returns false if the experiment is disabled', () => {
       const component = renderWithWarning({
         isExperimentEnabled: false,
@@ -225,18 +174,6 @@ describe(__filename, () => {
     sinon.assert.called(_couldShowWarning);
   });
 
-  it('calls couldShowWarning on update', () => {
-    const _couldShowWarning = sinon.spy();
-
-    const root = render({ _couldShowWarning });
-
-    _couldShowWarning.resetHistory();
-
-    root.setProps({ installStatus: INSTALLED });
-
-    sinon.assert.called(_couldShowWarning);
-  });
-
   describe('display tracking event', () => {
     const _tracking = createFakeTracking();
 
@@ -280,49 +217,6 @@ describe(__filename, () => {
 
         sinon.assert.notCalled(_tracking.sendEvent);
       });
-
-      it('sends the event on update if installStatus has changed', () => {
-        const _couldShowWarning = sinon.stub().returns(true);
-        const addon = createInternalAddon(fakeAddon);
-        const installStatus = UNINSTALLED;
-
-        const root = render({
-          _couldShowWarning,
-          _tracking,
-          addon,
-          installStatus: undefined,
-          variant,
-        });
-
-        _tracking.sendEvent.resetHistory();
-
-        root.setProps({ installStatus });
-
-        sinon.assert.calledWith(_tracking.sendEvent, {
-          action: variant,
-          category: EXPERIMENT_CATEGORY_DISPLAY,
-          label: addon.name,
-        });
-      });
-
-      it('does not send the event on update if installStatus has not changed', () => {
-        const _couldShowWarning = sinon.stub().returns(true);
-        const installStatus = UNINSTALLED;
-        const addon = fakeAddon;
-        _setInstallStatus({ addon, status: installStatus });
-
-        const root = render({
-          _couldShowWarning,
-          _tracking,
-          variant,
-        });
-
-        _tracking.sendEvent.resetHistory();
-
-        root.setProps({ installStatus });
-
-        sinon.assert.notCalled(_tracking.sendEvent);
-      });
     });
   });