From 99a0d5277e2d6fbdd976faea69a77f063d4adc37 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Fri, 25 Oct 2024 17:53:08 +0200 Subject: [PATCH 1/6] Expose whether a version is soft or hard-blocked in the blocked page --- src/amo/pages/Block/index.js | 105 ++++++++++++++++++----------------- 1 file changed, 54 insertions(+), 51 deletions(-) diff --git a/src/amo/pages/Block/index.js b/src/amo/pages/Block/index.js index 360efdc657a..08de120cd20 100644 --- a/src/amo/pages/Block/index.js +++ b/src/amo/pages/Block/index.js @@ -83,52 +83,26 @@ export class BlockBase extends React.Component { ); } - renderDateAndURL(): React.Node | Array { + renderURL(): React.Node | Array { const { block, i18n } = this.props; if (!block) { return ; } - const content: Array = [ - i18n.sprintf(i18n.gettext('Blocked on %(date)s.'), { - date: i18n.moment(block.created).format('ll'), - }), - ]; - if (block.url) { - content.push( - ' ', + return ( {i18n.gettext('View block request')} - , - '.', + ); } - return content; - } - - renderVersions(): React.Node | string { - const { block, i18n } = this.props; - - if (!block) { - return ; - } - - if (block.is_all_versions) { - return i18n.gettext('Versions blocked: all versions.'); - } - - const versions = block.versions.join(', '); - - return i18n.sprintf(i18n.gettext('Versions blocked: %(versions)s.'), { - versions, - }); + return null; } render(): React.Node { - const { block, errorHandler, i18n } = this.props; + const { block, errorHandler, i18n, match } = this.props; if (errorHandler.hasError()) { log.warn(`Captured API Error: ${errorHandler.capturedError.messages}`); @@ -140,15 +114,39 @@ export class BlockBase extends React.Component { return ; } - const title = - block && block.name - ? i18n.sprintf( - i18n.gettext(`%(addonName)s has been blocked for your protection.`), - { - addonName: block.name, - }, - ) - : i18n.gettext(`This add-on has been blocked for your protection.`); + const isSoftBlocked = + (block?.soft_blocked.includes(match.params.versionId)) || + (block?.soft_blocked.length && !block.blocked.length); + let title; + if (isSoftBlocked) { + title = + block && block.name + ? i18n.sprintf( + i18n.gettext( + `%(addonName)s is restricted for violating Mozilla policies.`, + ), + { + addonName: block.name, + }, + ) + : i18n.gettext( + `This add-on is restricted for violating Mozilla policies.`, + ); + } else { + title = + block && block.name + ? i18n.sprintf( + i18n.gettext( + `%(addonName)s is blocked for violating Mozilla policies.`, + ), + { + addonName: block.name, + }, + ) + : i18n.gettext( + `This add-on is blocked for violating Mozilla policies.`, + ); + } return ( @@ -159,7 +157,7 @@ export class BlockBase extends React.Component { -

{i18n.gettext('Why was it blocked?')}

+

{i18n.gettext('Why did this happen?')}

{

{i18n.gettext('What does this mean?')}

- {i18n.gettext(`The problematic add-on or plugin will be - automatically disabled and no longer usable.`)} + {isSoftBlocked + ? i18n.gettext(`Until the violation is resolved, this add-on + won't be available for download from addons.mozilla.org. + If it’s already installed, it will be disabled and users + will be informed about the violation. They may choose to + enable the add-on again at their own risk.`) + : i18n.gettext(`Until the violation is resolved, this add-on + won't be available for download from addons.mozilla.org. + It will be automatically disabled and no longer usable in + Firefox.`)}

{ or other third-party software that seriously compromises Firefox security, stability, or performance and meets %(criteriaStartLink)scertain criteria%(criteriaEndLink)s, - the software may be blocked from general use. For more - information, please read %(supportStartLink)sthis support - article%(supportEndLink)s.`), + the software may be blocked or restricted from general + use. For more information, please read + %(supportStartLink)sthis support article%(supportEndLink)s. + `), { criteriaStartLink: ``, criteriaEndLink: '', @@ -202,11 +209,7 @@ export class BlockBase extends React.Component { ['a'], )} /> -

- {this.renderVersions()} -
- {this.renderDateAndURL()} -

+

{this.renderURL()}

 From 76f49a0619b7bc54deed3172317ea58568450a11 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Fri, 25 Oct 2024 17:57:34 +0200 Subject: [PATCH 2/6] Useless parenthesis --- src/amo/pages/Block/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/amo/pages/Block/index.js b/src/amo/pages/Block/index.js index 08de120cd20..1a43c62a386 100644 --- a/src/amo/pages/Block/index.js +++ b/src/amo/pages/Block/index.js @@ -115,7 +115,7 @@ export class BlockBase extends React.Component { } const isSoftBlocked = - (block?.soft_blocked.includes(match.params.versionId)) || + block?.soft_blocked.includes(match.params.versionId) || (block?.soft_blocked.length && !block.blocked.length); let title; if (isSoftBlocked) { From 4b19ec533d6e6125aad7e66b4b55cb2c91aeee15 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Fri, 25 Oct 2024 18:00:22 +0200 Subject: [PATCH 3/6] Fix type --- src/amo/reducers/blocks.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/amo/reducers/blocks.js b/src/amo/reducers/blocks.js index eef7a820f0e..24f84dd90be 100644 --- a/src/amo/reducers/blocks.js +++ b/src/amo/reducers/blocks.js @@ -15,6 +15,8 @@ export type ExternalBlockType = {| guid: string, id: number, versions: [string], + soft_blocked: [string], + blocked: [string], is_all_versions?: boolean, modified: string, reason: string | null, From 0392b3122a5e9bad97871849836b16abf17bae5c Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Fri, 25 Oct 2024 19:10:48 +0200 Subject: [PATCH 4/6] Change this string too --- src/amo/pages/Block/index.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/amo/pages/Block/index.js b/src/amo/pages/Block/index.js index 1a43c62a386..7700c812f1f 100644 --- a/src/amo/pages/Block/index.js +++ b/src/amo/pages/Block/index.js @@ -162,8 +162,8 @@ export class BlockBase extends React.Component { // eslint-disable-next-line react/no-danger dangerouslySetInnerHTML={sanitizeHTML( i18n.sprintf( - i18n.gettext(`This add-on violates %(startLink)sMozilla's - Add-on Policies%(endLink)s.`), + i18n.gettext(`This extension, theme, or plugin violates + %(startLink)sMozilla's Add-on Policies%(endLink)s.`), { startLink: ``, endLink: '', From c575f51510b9f58d52002da7cc2ea36afa6028e0 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Mon, 28 Oct 2024 12:22:32 +0100 Subject: [PATCH 5/6] Tests --- src/amo/pages/Block/index.js | 5 +- tests/unit/amo/pages/TestBlock.js | 141 +++++++++++++++--------------- tests/unit/helpers.js | 2 + 3 files changed, 76 insertions(+), 72 deletions(-) diff --git a/src/amo/pages/Block/index.js b/src/amo/pages/Block/index.js index 7700c812f1f..b6883f39900 100644 --- a/src/amo/pages/Block/index.js +++ b/src/amo/pages/Block/index.js @@ -115,8 +115,9 @@ export class BlockBase extends React.Component { } const isSoftBlocked = - block?.soft_blocked.includes(match.params.versionId) || - (block?.soft_blocked.length && !block.blocked.length); + block?.soft_blocked.length && + (block?.soft_blocked.includes(match.params.versionId) || + !block?.blocked.length); let title; if (isSoftBlocked) { title = diff --git a/tests/unit/amo/pages/TestBlock.js b/tests/unit/amo/pages/TestBlock.js index 530fcc5d5a0..a39dc03e26e 100644 --- a/tests/unit/amo/pages/TestBlock.js +++ b/tests/unit/amo/pages/TestBlock.js @@ -15,7 +15,6 @@ import { createFakeBlockResult, createLocalizedString, dispatchClientMetadata, - fakeI18n, getElement, renderPage as defaultRender, screen, @@ -129,16 +128,14 @@ describe(__filename, () => { expect( within(screen.getByClassName('Block-reason')).getAllByRole('alert'), ).toHaveLength(1); - // 1. versions blocked - // 2. date and URL expect( within(screen.getByClassName('Block-metadata')).getAllByRole('alert'), - ).toHaveLength(2); + ).toHaveLength(1); }); it('renders a generic header/title when the block has no add-on name', async () => { const block = _createFakeBlockResult({ addonName: null }); - const title = 'This add-on has been blocked for your protection.'; + const title = 'This add-on is blocked for violating Mozilla policies.'; store.dispatch(loadBlock({ block })); render(); @@ -155,7 +152,7 @@ describe(__filename, () => { const block = _createFakeBlockResult({ addonName: createLocalizedString(name), }); - const title = `${name} has been blocked for your protection.`; + const title = `${name} is blocked for violating Mozilla policies.`; store.dispatch(loadBlock({ block })); render(); @@ -168,85 +165,40 @@ describe(__filename, () => { expect(screen.getByText(title)).toBeInTheDocument(); }); - it('renders a paragraph with the reason when the block has one', () => { - const reason = 'this is a reason for a block'; - const block = _createFakeBlockResult({ reason }); - store.dispatch(loadBlock({ block })); - render(); - - expect(screen.getByText(reason)).toBeInTheDocument(); - expect(screen.getByText(reason)).toHaveAttribute('lang', 'en-US'); - }); - - it('does not render a reason if the block does not have one', () => { - const block = _createFakeBlockResult({ reason: null }); - store.dispatch(loadBlock({ block })); - render(); - - expect(screen.queryByClassName('Block-reason')).not.toBeInTheDocument(); - }); - - it('renders "all versions" when "is_all_versions" is true', () => { + it('renders a generic soft-block header/title when the block has no add-on name', async () => { const block = _createFakeBlockResult({ - is_all_versions: true, + addonName: null, + soft_blocked: ['42.0'], + blocked: [], }); - const i18n = fakeI18n(); + const title = 'This add-on is restricted for violating Mozilla policies.'; store.dispatch(loadBlock({ block })); render(); - // The version info and the block date are inside the same tag, separated - // by a
. - expect( - screen.getByTextAcrossTags( - `Versions blocked: all versions.Blocked on ${i18n - .moment(block.created) - .format('ll')}.`, + await waitFor(() => + expect(getElement('title')).toHaveTextContent( + `${title} – Add-ons for Firefox (${lang})`, ), - ).toBeInTheDocument(); + ); + expect(screen.getByText(title)).toBeInTheDocument(); }); - it('renders the versions if "is_all_versions" is false', () => { - const v1 = '12'; - const v2 = '34'; - const block = _createFakeBlockResult({ - versions: [v1, v2], - is_all_versions: false, - }); - const i18n = fakeI18n(); + it('renders a paragraph with the reason when the block has one', () => { + const reason = 'this is a reason for a block'; + const block = _createFakeBlockResult({ reason }); store.dispatch(loadBlock({ block })); render(); - // The version info and the block date are inside the same tag, separated - // by a
. - expect( - screen.getByTextAcrossTags( - `Versions blocked: ${v1}, ${v2}.Blocked on ${i18n - .moment(block.created) - .format('ll')}.`, - ), - ).toBeInTheDocument(); + expect(screen.getByText(reason)).toBeInTheDocument(); + expect(screen.getByText(reason)).toHaveAttribute('lang', 'en-US'); }); - it('renders the versions if "is_all_versions" is missing', () => { - const v1 = '12'; - const v2 = '34'; - const block = _createFakeBlockResult({ - versions: [v1, v2], - is_all_versions: undefined, - }); - const i18n = fakeI18n(); + it('does not render a reason if the block does not have one', () => { + const block = _createFakeBlockResult({ reason: null }); store.dispatch(loadBlock({ block })); render(); - // The version info and the block date are inside the same tag, separated - // by a
. - expect( - screen.getByTextAcrossTags( - `Versions blocked: ${v1}, ${v2}.Blocked on ${i18n - .moment(block.created) - .format('ll')}.`, - ), - ).toBeInTheDocument(); + expect(screen.queryByClassName('Block-reason')).not.toBeInTheDocument(); }); it('renders the reason with HTML tags removed', () => { @@ -307,7 +259,56 @@ describe(__filename, () => { }); expect( - screen.getByText(`${name} has been blocked for your protection.`), + screen.getByText(`${name} is blocked for violating Mozilla policies.`), + ).toBeInTheDocument(); + expect( + screen.getByText( + /It will be automatically disabled and no longer usable in Firefox./, + ), + ).toBeInTheDocument(); + }); + + it('renders a soft-block page when the block has only soft-blocks', () => { + const name = 'some-addon-name'; + const block = _createFakeBlockResult({ + addonName: createLocalizedString(name), + soft_blocked: ['42.0'], + blocked: [], + }); + store.dispatch(loadBlock({ block })); + + render(); + + expect( + screen.getByText(`${name} is restricted for violating Mozilla policies.`), + ).toBeInTheDocument(); + expect( + screen.getByText( + /They may choose to enable the add-on again at their own risk./, + ), + ).toBeInTheDocument(); + }); + + it('renders a soft-block page when a soft-blocked versionId is present in the URL', () => { + const name = 'some-addon-name'; + const block = _createFakeBlockResult({ + addonName: createLocalizedString(name), + soft_blocked: ['42.0'], + }); + store.dispatch(loadBlock({ block })); + + defaultRender({ + initialEntries: [`${getLocation()}42.0/`], + store, + }); + + expect( + screen.getByText(`${name} is restricted for violating Mozilla policies.`), + ).toBeInTheDocument(); + expect( + screen.getByText( + /They may choose to enable the add-on again at their own risk./, + ), ).toBeInTheDocument(); }); diff --git a/tests/unit/helpers.js b/tests/unit/helpers.js index 53e113ebec3..1d0eb040c09 100644 --- a/tests/unit/helpers.js +++ b/tests/unit/helpers.js @@ -1313,6 +1313,8 @@ export const createFakeBlockResult = ({ modified: '2020-01-22T10:09:01Z', guid, versions: ['0.1', '4.56'], + blocked: ['0.1', '4.56'], + soft_blocked: [], is_all_versions: false, addon_name: addonName, reason, From e2733411344fbe681659819b89d211f5e4542083 Mon Sep 17 00:00:00 2001 From: Mathieu Pillard Date: Tue, 29 Oct 2024 11:24:27 +0100 Subject: [PATCH 6/6] Drop versions from the reducer/fake block helper --- src/amo/reducers/blocks.js | 1 - tests/unit/helpers.js | 1 - 2 files changed, 2 deletions(-) diff --git a/src/amo/reducers/blocks.js b/src/amo/reducers/blocks.js index 24f84dd90be..dd9d0bf782d 100644 --- a/src/amo/reducers/blocks.js +++ b/src/amo/reducers/blocks.js @@ -14,7 +14,6 @@ export type ExternalBlockType = {| created: string, guid: string, id: number, - versions: [string], soft_blocked: [string], blocked: [string], is_all_versions?: boolean, diff --git a/tests/unit/helpers.js b/tests/unit/helpers.js index 1d0eb040c09..c6b109f2abf 100644 --- a/tests/unit/helpers.js +++ b/tests/unit/helpers.js @@ -1312,7 +1312,6 @@ export const createFakeBlockResult = ({ created: '2020-01-22T10:09:01Z', modified: '2020-01-22T10:09:01Z', guid, - versions: ['0.1', '4.56'], blocked: ['0.1', '4.56'], soft_blocked: [], is_all_versions: false,