From 030161746a0587aa482fa35eef449a74db435119 Mon Sep 17 00:00:00 2001 From: Jamie Howard <48524071+jhoward1994@users.noreply.github.com> Date: Thu, 29 Feb 2024 08:51:17 +0000 Subject: [PATCH 1/7] fix(content-manager): send locale when deleting i18n single type (#19629) --- .../components/ContentTypeFormWrapper.tsx | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/packages/core/admin/admin/src/content-manager/components/ContentTypeFormWrapper.tsx b/packages/core/admin/admin/src/content-manager/components/ContentTypeFormWrapper.tsx index 2455790cd0..8ee5ffd459 100644 --- a/packages/core/admin/admin/src/content-manager/components/ContentTypeFormWrapper.tsx +++ b/packages/core/admin/admin/src/content-manager/components/ContentTypeFormWrapper.tsx @@ -88,7 +88,13 @@ const ContentTypeFormWrapper = ({ const { setCurrentStep } = useGuidedTour(); const { trackUsage } = useTracking(); const { push, replace } = useHistory(); - const [{ query, rawQuery }] = useQueryParams(); + const [{ query, rawQuery }] = useQueryParams<{ + plugins?: { + i18n?: { + locale?: string; + }; + }; + }>(); const dispatch = useTypedDispatch(); const { componentsDataStructure, contentTypeDataStructure, data, isLoading, status } = useTypedSelector((state) => state['content-manager_editViewCrudReducer']); @@ -252,8 +258,12 @@ const ContentTypeFormWrapper = ({ try { trackUsage('willDeleteEntry', trackerProperty); + const locale = query?.plugins?.i18n?.locale; + const params = isSingleType && locale ? { locale } : {}; + const { data } = await del( - `/content-manager/${collectionType}/${slug}/${id}` + `/content-manager/${collectionType}/${slug}/${id}`, + { params } ); toggleNotification({ From 80a18ebd59f47cacc8e68984612e16b49689283f Mon Sep 17 00:00:00 2001 From: Madhuri Sandbhor Date: Thu, 29 Feb 2024 10:13:49 +0100 Subject: [PATCH 2/7] fix: delete selected timezone value (#19628) --- .../content-releases/admin/src/components/ReleaseModal.tsx | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/core/content-releases/admin/src/components/ReleaseModal.tsx b/packages/core/content-releases/admin/src/components/ReleaseModal.tsx index 9b44347fe8..29e3e69633 100644 --- a/packages/core/content-releases/admin/src/components/ReleaseModal.tsx +++ b/packages/core/content-releases/admin/src/components/ReleaseModal.tsx @@ -298,6 +298,9 @@ const TimezoneComponent = ({ timezoneOptions }: { timezoneOptions: ITimezoneOpti onChange={(timezone) => { setFieldValue('timezone', timezone); }} + onTextValueChange={(timezone) => { + setFieldValue('timezone', timezone); + }} onClear={() => { setFieldValue('timezone', ''); }} From 710a9e5658e886ed0bfe2c2751d695381e3b2a89 Mon Sep 17 00:00:00 2001 From: Madhuri Sandbhor Date: Thu, 29 Feb 2024 10:14:07 +0100 Subject: [PATCH 3/7] fix: extra padding removed, background color added for delete action (#19627) --- .../admin/src/pages/ReleaseDetailsPage.tsx | 33 +++++++++---------- 1 file changed, 15 insertions(+), 18 deletions(-) diff --git a/packages/core/content-releases/admin/src/pages/ReleaseDetailsPage.tsx b/packages/core/content-releases/admin/src/pages/ReleaseDetailsPage.tsx index 0c06ec7e07..4a7ca2b70f 100644 --- a/packages/core/content-releases/admin/src/pages/ReleaseDetailsPage.tsx +++ b/packages/core/content-releases/admin/src/pages/ReleaseDetailsPage.tsx @@ -78,13 +78,20 @@ const ReleaseInfoWrapper = styled(Flex)` border-top: 1px solid ${({ theme }) => theme.colors.neutral150}; `; -const StyledMenuItem = styled(Menu.Item)<{ disabled?: boolean }>` +const StyledMenuItem = styled(Menu.Item)<{ + disabled?: boolean; + variant?: 'neutral' | 'danger'; +}>` svg path { fill: ${({ theme, disabled }) => disabled && theme.colors.neutral500}; } span { color: ${({ theme, disabled }) => disabled && theme.colors.neutral500}; } + + &:hover { + background: ${({ theme, variant = 'neutral' }) => theme.colors[`${variant}100`]}; + } `; const PencilIcon = styled(Pencil)` @@ -387,14 +394,7 @@ export const ReleaseDetailsLayout = ({ width="100%" > - + {formatMessage({ @@ -404,15 +404,12 @@ export const ReleaseDetailsLayout = ({ - - + + {formatMessage({ From d10040847b91742ccb8083938399b63ffa289c7a Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Thu, 29 Feb 2024 14:32:37 +0100 Subject: [PATCH 4/7] fix: api and transfer token lifespan select lists work with all durations --- .../src/api/config/controllers/config.js | 7 ++ .../template/src/api/config/routes/config.js | 8 +++ .../template/src/create-transfer-token.js | 27 ++++++++ e2e/app-template/template/src/index.js | 24 +------ e2e/constants.js | 2 + e2e/scripts/dts-import.js | 13 ++++ e2e/tests/admin/transfer/tokens.spec.ts | 65 +++++++++++++++++++ e2e/utils/shared.ts | 20 +++++- .../pages/ApiTokens/EditView/EditViewPage.tsx | 6 +- .../pages/TransferTokens/EditView.tsx | 4 +- .../admin/server/src/services/api-token.ts | 25 +++++-- .../server/src/services/transfer/token.ts | 34 ++++++---- .../core/admin/shared/contracts/api-token.ts | 2 +- .../core/admin/shared/contracts/transfer.ts | 4 +- 14 files changed, 192 insertions(+), 49 deletions(-) create mode 100644 e2e/app-template/template/src/create-transfer-token.js create mode 100644 e2e/tests/admin/transfer/tokens.spec.ts diff --git a/e2e/app-template/template/src/api/config/controllers/config.js b/e2e/app-template/template/src/api/config/controllers/config.js index 25d061fd2f..97301592ed 100644 --- a/e2e/app-template/template/src/api/config/controllers/config.js +++ b/e2e/app-template/template/src/api/config/controllers/config.js @@ -1,3 +1,5 @@ +const { createTestTransferToken } = require('../../../create-transfer-token'); + module.exports = { rateLimitEnable(ctx) { const { value } = ctx.request.body; @@ -6,6 +8,11 @@ module.exports = { configService.rateLimitEnable(value); + ctx.send(200); + }, + async resetTransferToken(ctx) { + await createTestTransferToken(strapi); + ctx.send(200); }, }; diff --git a/e2e/app-template/template/src/api/config/routes/config.js b/e2e/app-template/template/src/api/config/routes/config.js index 3653f076f1..3aee096f85 100644 --- a/e2e/app-template/template/src/api/config/routes/config.js +++ b/e2e/app-template/template/src/api/config/routes/config.js @@ -8,5 +8,13 @@ module.exports = { auth: false, }, }, + { + method: 'POST', + path: '/config/resettransfertoken', + handler: 'config.resetTransferToken', + config: { + auth: false, + }, + }, ], }; diff --git a/e2e/app-template/template/src/create-transfer-token.js b/e2e/app-template/template/src/create-transfer-token.js new file mode 100644 index 0000000000..9a1a815335 --- /dev/null +++ b/e2e/app-template/template/src/create-transfer-token.js @@ -0,0 +1,27 @@ +const { CUSTOM_TRANSFER_TOKEN_ACCESS_KEY } = require('./constants'); + +/** + * Make sure the test transfer token exists in the database + * @param {Strapi.Strapi} strapi + * @returns {Promise} + */ +const createTestTransferToken = async (strapi) => { + const { token: transferTokenService } = strapi.admin.services.transfer; + + const accessKeyHash = transferTokenService.hash(CUSTOM_TRANSFER_TOKEN_ACCESS_KEY); + const exists = await transferTokenService.exists({ accessKey: accessKeyHash }); + + if (!exists) { + await transferTokenService.create({ + name: 'TestToken', + description: 'Transfer token used to seed the e2e database', + lifespan: null, + permissions: ['push'], + accessKey: CUSTOM_TRANSFER_TOKEN_ACCESS_KEY, + }); + } +}; + +module.exports = { + createTestTransferToken, +}; diff --git a/e2e/app-template/template/src/index.js b/e2e/app-template/template/src/index.js index 8320d5ff43..4d9eec9a85 100644 --- a/e2e/app-template/template/src/index.js +++ b/e2e/app-template/template/src/index.js @@ -1,4 +1,4 @@ -const { CUSTOM_TRANSFER_TOKEN_ACCESS_KEY } = require('./constants'); +const { createTestTransferToken } = require('./create-transfer-token'); module.exports = { /** @@ -23,25 +23,3 @@ module.exports = { await createTestTransferToken(strapi); }, }; - -/** - * Make sure the test transfer token exists in the database - * @param {Strapi.Strapi} strapi - * @returns {Promise} - */ -const createTestTransferToken = async (strapi) => { - const { token: transferTokenService } = strapi.admin.services.transfer; - - const accessKeyHash = transferTokenService.hash(CUSTOM_TRANSFER_TOKEN_ACCESS_KEY); - const exists = await transferTokenService.exists({ accessKey: accessKeyHash }); - - if (!exists) { - await transferTokenService.create({ - name: 'TestToken', - description: 'Transfer token used to seed the e2e database', - lifespan: null, - permissions: ['push'], - accessKey: CUSTOM_TRANSFER_TOKEN_ACCESS_KEY, - }); - } -}; diff --git a/e2e/constants.js b/e2e/constants.js index 714f8e747a..0ea248aa05 100644 --- a/e2e/constants.js +++ b/e2e/constants.js @@ -4,6 +4,8 @@ const ALLOWED_CONTENT_TYPES = [ 'admin::user', 'admin::role', 'admin::permission', + 'admin::api-token', + 'admin::transfer-token', 'api::article.article', 'api::author.author', 'api::homepage.homepage', diff --git a/e2e/scripts/dts-import.js b/e2e/scripts/dts-import.js index 91bca192c3..66e923a828 100644 --- a/e2e/scripts/dts-import.js +++ b/e2e/scripts/dts-import.js @@ -48,6 +48,19 @@ export const resetDatabaseAndImportDataFromPath = async (filePath) => { engine.diagnostics.onDiagnostic(console.log); + try { + // reset the transfer token to allow the transfer if it's been wiped (that is, not included in previous import data) + const res = await fetch( + `http://127.0.0.1:${process.env.PORT ?? 1337}/api/config/resettransfertoken`, + { + method: 'POST', + } + ); + } catch (err) { + console.error('Token reset failed.' + JSON.stringify(err, null, 2)); + process.exit(1); + } + try { await engine.transfer(); } catch { diff --git a/e2e/tests/admin/transfer/tokens.spec.ts b/e2e/tests/admin/transfer/tokens.spec.ts new file mode 100644 index 0000000000..0fdc05e241 --- /dev/null +++ b/e2e/tests/admin/transfer/tokens.spec.ts @@ -0,0 +1,65 @@ +import { test, expect } from '@playwright/test'; +import { login } from '../../../utils/login'; +import { resetDatabaseAndImportDataFromPath } from '../../../scripts/dts-import'; +import { navToHeader, delay } from '../../../utils/shared'; + +const createTransferToken = async (page, tokenName, duration, type) => { + await navToHeader( + page, + ['Settings', 'Transfer Tokens', 'Create new Transfer Token'], + 'Create Transfer Token' + ); + + await page.getByLabel('Name*').click(); + await page.getByLabel('Name*').fill(tokenName); + + await page.getByLabel('Token duration').click(); + await page.getByRole('option', { name: duration }).click(); + + await page.getByLabel('Token type').click(); + await page.getByRole('option', { name: type }).click(); + + await page.getByRole('button', { name: 'Save' }).click(); + + await expect(page.getByText(/copy this token/)).toBeVisible(); + await expect(page.getByText('Expiration date:')).toBeVisible(); +}; + +test.describe('Transfer Tokens', () => { + test.beforeEach(async ({ page }) => { + await resetDatabaseAndImportDataFromPath('./e2e/data/with-admin.tar'); + await page.goto('/admin'); + await login({ page }); + }); + + // Test token creation + const testCases = [ + ['30-day push token', '30 days', 'Push'], + ['30-day pull token', '30 days', 'Pull'], + ['30-day full-access token', '30 days', 'Full access'], + // if push+pull work generally that's good enough for e2e + ['7-day token', '7 days', 'Full access'], + ['90-day token', '90 days', 'Full access'], + ['unlimited token', 'Unlimited', 'Full access'], + ]; + for (const [name, duration, type] of testCases) { + test(`A user should be able to create a ${name}`, async ({ page }) => { + await createTransferToken(page, name, duration, type); + }); + } + + test('Created tokens list page should be correct', async ({ page }) => { + await createTransferToken(page, 'my test token', 'unlimited', 'Full access'); + + // if we don't wait until createdAt is at least 1s, we see "NaN" for the timestamp + // TODO: fix the bug and remove this + await page.waitForTimeout(1100); + + await navToHeader(page, ['Settings', 'Transfer Tokens'], 'Transfer Tokens'); + + const row = page.getByRole('gridcell', { name: 'my test token', exact: true }); + await expect(row).toBeVisible(); + await expect(page.getByText(/\d+ (second|minute)s? ago/)).toBeVisible(); + // TODO: expand on this test, it could check edit and delete icons + }); +}); diff --git a/e2e/utils/shared.ts b/e2e/utils/shared.ts index 99cbbdc302..05e00d5b24 100644 --- a/e2e/utils/shared.ts +++ b/e2e/utils/shared.ts @@ -1,7 +1,25 @@ -import { test } from '@playwright/test'; +import { test, Page, expect } from '@playwright/test'; /** * Execute a test suite only if the condition is true */ export const describeOnCondition = (shouldDescribe: boolean) => shouldDescribe ? test.describe : test.describe.skip; + +/** + * Navigate to a page and confirm the header, awaiting each step + */ +export const navToHeader = async (page: Page, navItems: string[], headerText: string) => { + for (const navItem of navItems) { + // This does not use getByRole because sometimes "Settings" is "Settings 1" if there's a badge notification + // BUT if we don't match exact it conflicts with "Advanceed Settings" + // As a workaround, we implement our own startsWith with page.locator + const item = page.locator(`role=link[name^="${navItem}"]`); + await expect(item).toBeVisible(); + await item.click(); + } + + const header = page.getByRole('heading', { name: headerText, exact: true }); + await expect(header).toBeVisible(); + return header; +}; diff --git a/packages/core/admin/admin/src/pages/Settings/pages/ApiTokens/EditView/EditViewPage.tsx b/packages/core/admin/admin/src/pages/Settings/pages/ApiTokens/EditView/EditViewPage.tsx index 9a8c8e669e..b18563e8f9 100644 --- a/packages/core/admin/admin/src/pages/Settings/pages/ApiTokens/EditView/EditViewPage.tsx +++ b/packages/core/admin/admin/src/pages/Settings/pages/ApiTokens/EditView/EditViewPage.tsx @@ -201,8 +201,8 @@ export const EditView = () => { if (isCreating) { const res = await createToken({ ...body, - // in case a token has a lifespan of "unlimited" the API only accepts zero as a number - lifespan: body.lifespan === '0' ? parseInt(body.lifespan) : null, + // lifespan must be "null" for unlimited (0 would mean instantly expired and isn't accepted) + lifespan: body?.lifespan || null, permissions: body.type === 'custom' ? state.selectedActions : null, }); @@ -338,7 +338,7 @@ export const EditView = () => { name: apiToken?.name || '', description: apiToken?.description || '', type: apiToken?.type, - lifespan: apiToken?.lifespan ? apiToken.lifespan.toString() : apiToken?.lifespan, + lifespan: apiToken?.lifespan, }} enableReinitialize onSubmit={(body, actions) => handleSubmit(body, actions)} diff --git a/packages/core/admin/admin/src/pages/Settings/pages/TransferTokens/EditView.tsx b/packages/core/admin/admin/src/pages/Settings/pages/TransferTokens/EditView.tsx index 2e9aa94d87..403dd3b6b8 100644 --- a/packages/core/admin/admin/src/pages/Settings/pages/TransferTokens/EditView.tsx +++ b/packages/core/admin/admin/src/pages/Settings/pages/TransferTokens/EditView.tsx @@ -150,6 +150,8 @@ const EditView = () => { if (isCreating) { const res = await createToken({ ...body, + // lifespan must be "null" for unlimited (0 would mean instantly expired and isn't accepted) + lifespan: body?.lifespan || null, permissions, }); @@ -251,7 +253,7 @@ const EditView = () => { { name: transferToken?.name || '', description: transferToken?.description || '', - lifespan: transferToken?.lifespan ?? null, + lifespan: transferToken?.lifespan || null, /** * We need to cast the permissions to satisfy the type for `permissions` * in the request body incase we don't have a transferToken and instead diff --git a/packages/core/admin/server/src/services/api-token.ts b/packages/core/admin/server/src/services/api-token.ts index 9f85b46ae9..eaff6a7895 100644 --- a/packages/core/admin/server/src/services/api-token.ts +++ b/packages/core/admin/server/src/services/api-token.ts @@ -1,5 +1,5 @@ import crypto from 'crypto'; -import { omit, difference, isNil, isEmpty, map, isArray, uniq } from 'lodash/fp'; +import { omit, difference, isNil, isEmpty, map, isArray, uniq, isNumber } from 'lodash/fp'; import { errors } from '@strapi/utils'; import type { Update, ApiToken, ApiTokenBody } from '../../../shared/contracts/api-token'; import constants from './constants'; @@ -61,14 +61,25 @@ const assertCustomTokenPermissionsValidity = ( }; /** - * Assert that a token's lifespan is valid + * Check if a token's lifespan is valid */ -const assertValidLifespan = (lifespan: ApiTokenBody['lifespan']) => { +const isValidLifespan = (lifespan: unknown) => { if (isNil(lifespan)) { - return; + return true; } - if (!Object.values(constants.API_TOKEN_LIFESPANS).includes(lifespan as number)) { + if (!isNumber(lifespan) || !Object.values(constants.API_TOKEN_LIFESPANS).includes(lifespan)) { + return false; + } + + return true; +}; + +/** + * Assert that a token's lifespan is valid + */ +const assertValidLifespan = (lifespan: unknown) => { + if (!isValidLifespan(lifespan)) { throw new ValidationError( `lifespan must be one of the following values: ${Object.values(constants.API_TOKEN_LIFESPANS).join(', ')}` @@ -138,14 +149,14 @@ const hash = (accessKey: string) => { const getExpirationFields = (lifespan: ApiTokenBody['lifespan']) => { // it must be nil or a finite number >= 0 - const isValidNumber = Number.isFinite(lifespan) && (lifespan as number) > 0; + const isValidNumber = isNumber(lifespan) && Number.isFinite(lifespan) && lifespan > 0; if (!isValidNumber && !isNil(lifespan)) { throw new ValidationError('lifespan must be a positive number or null'); } return { lifespan: lifespan || null, - expiresAt: lifespan ? Date.now() + (lifespan as number) : null, + expiresAt: lifespan ? Date.now() + lifespan : null, }; }; diff --git a/packages/core/admin/server/src/services/transfer/token.ts b/packages/core/admin/server/src/services/transfer/token.ts index 1268d5eeef..b9b96cffc4 100644 --- a/packages/core/admin/server/src/services/transfer/token.ts +++ b/packages/core/admin/server/src/services/transfer/token.ts @@ -1,6 +1,6 @@ import crypto from 'crypto'; import assert from 'assert'; -import { map, isArray, omit, uniq, isNil, difference, isEmpty } from 'lodash/fp'; +import { map, isArray, omit, uniq, isNil, difference, isEmpty, isNumber } from 'lodash/fp'; import { errors } from '@strapi/utils'; import '@strapi/types'; import constants from '../constants'; @@ -79,7 +79,7 @@ const create = async (attributes: TokenCreatePayload): Promise => delete attributes.accessKey; assertTokenPermissionsValidity(attributes); - assertValidLifespan(attributes); + assertValidLifespan(attributes.lifespan); const result = (await strapi.db.transaction(async () => { const transferToken = await strapi.query(TRANSFER_TOKEN_UID).create({ @@ -131,7 +131,7 @@ const update = async ( } assertTokenPermissionsValidity(attributes); - assertValidLifespan(attributes); + assertValidLifespan(attributes.lifespan); return strapi.db.transaction(async () => { const updatedToken = await strapi.query(TRANSFER_TOKEN_UID).update({ @@ -281,11 +281,9 @@ const regenerate = async (id: string | number): Promise => { }; }; -const getExpirationFields = ( - lifespan: number | null -): { lifespan: null | number; expiresAt: null | number } => { +const getExpirationFields = (lifespan: TransferToken['lifespan']) => { // it must be nil or a finite number >= 0 - const isValidNumber = Number.isFinite(lifespan) && lifespan !== null && lifespan > 0; + const isValidNumber = isNumber(lifespan) && Number.isFinite(lifespan) && lifespan > 0; if (!isValidNumber && !isNil(lifespan)) { throw new ValidationError('lifespan must be a positive number or null'); } @@ -359,14 +357,28 @@ const assertTokenPermissionsValidity = (attributes: TokenUpdatePayload) => { }; /** - * Assert that a token's lifespan is valid + * Check if a token's lifespan is valid */ -const assertValidLifespan = ({ lifespan }: { lifespan?: TransferToken['lifespan'] }) => { +const isValidLifespan = (lifespan: unknown) => { if (isNil(lifespan)) { - return; + return true; } - if (!Object.values(constants.TRANSFER_TOKEN_LIFESPANS).includes(lifespan)) { + if ( + !isNumber(lifespan) || + !Object.values(constants.TRANSFER_TOKEN_LIFESPANS).includes(lifespan) + ) { + return false; + } + + return true; +}; + +/** + * Assert that a token's lifespan is valid + */ +const assertValidLifespan = (lifespan: unknown) => { + if (!isValidLifespan(lifespan)) { throw new ValidationError( `lifespan must be one of the following values: ${Object.values(constants.TRANSFER_TOKEN_LIFESPANS).join(', ')}` diff --git a/packages/core/admin/shared/contracts/api-token.ts b/packages/core/admin/shared/contracts/api-token.ts index 4d4da32500..a7dc08f218 100644 --- a/packages/core/admin/shared/contracts/api-token.ts +++ b/packages/core/admin/shared/contracts/api-token.ts @@ -8,7 +8,7 @@ export type ApiToken = { expiresAt: string; id: Entity.ID; lastUsedAt: string | null; - lifespan: string | number; + lifespan: string | number | null; name: string; permissions: string[]; type: 'custom' | 'full-access' | 'read-only'; diff --git a/packages/core/admin/shared/contracts/transfer.ts b/packages/core/admin/shared/contracts/transfer.ts index 2362d200ad..1eb1312401 100644 --- a/packages/core/admin/shared/contracts/transfer.ts +++ b/packages/core/admin/shared/contracts/transfer.ts @@ -1,7 +1,7 @@ import { errors } from '@strapi/utils'; export interface TransferTokenPermission { - id: number | string; + id: number | `${number}`; action: 'push' | 'pull' | 'push-pull'; token: TransferToken | number; } @@ -12,7 +12,7 @@ export interface DatabaseTransferToken { description: string; accessKey: string; lastUsedAt?: number; - lifespan: number | null; + lifespan: string | number | null; expiresAt: number; permissions: TransferTokenPermission[]; } From 73143c28059b343ba62d98c29672ab114562fbbc Mon Sep 17 00:00:00 2001 From: Ben Irvin Date: Fri, 1 Mar 2024 12:31:17 +0100 Subject: [PATCH 5/7] test: improve e2e playwright config --- .github/workflows/tests.yml | 4 ++-- docs/docs/guides/e2e/00-setup.md | 40 ++++++++++++++++++++++++++++++++ e2e/README.md | 3 +++ playwright.base.config.js | 34 +++++++++++++++++++++++---- 4 files changed, 75 insertions(+), 6 deletions(-) create mode 100644 e2e/README.md diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ab13ff42ec..201b9ac4ac 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -189,7 +189,7 @@ jobs: if: failure() with: name: ce-playwright-trace - path: test-apps/e2e/**/test-results/**/trace.zip + path: test-apps/e2e/test-results/**/trace.zip retention-days: 1 e2e_ee: @@ -231,7 +231,7 @@ jobs: if: failure() with: name: ee-playwright-trace - path: test-apps/e2e/**/test-results/**/trace.zip + path: test-apps/e2e/test-results/**/trace.zip retention-days: 1 api_ce_pg: diff --git a/docs/docs/guides/e2e/00-setup.md b/docs/docs/guides/e2e/00-setup.md index ca77a6d7db..36e11e7862 100644 --- a/docs/docs/guides/e2e/00-setup.md +++ b/docs/docs/guides/e2e/00-setup.md @@ -29,6 +29,46 @@ This will spawn by default a Strapi instance per testing domain (e.g. content-ma If you need to clean the test-apps folder because they are not working as expected, you can run `yarn test:e2e clean` which will clean said directory. +### Running specific tests + +To run only one domain, meaning a top-level directory in e2e/tests such as "admin" or "content-manager", use the `--domains` option. + +```shell +yarn test:e2e --domains admin +yarn test:e2e --domain admin +``` + +To run a specific file, you can pass arguments and options to playwright using `--` between the test:e2e options and the playwright options, such as: + +```shell +# to run just the login.spec.ts file in the admin domain +yarn test:e2e --domains admin -- login.spec.ts +``` + +### Concurrency / parallellization + +By default, every domain is run with its own test app in parallel with the other domains. The tests within a domain are run in series, one at a time. + +If you need an easier way to view the output, or have problems running multiple apps at once on your system, you can use the `-c` option + +```shell +# only run one domain at a time +yarn test:e2e -c 1 +``` + +### Env Variables to Control Test Config + +Some helpers have been added to allow you to modify the playwright configuration on your own system without touching the playwright config file used by the test runner. + +| env var | Description | Default | +| ---------------------------- | -------------------------------------------- | ------------------ | +| PLAYWRIGHT_WEBSERVER_TIMEOUT | timeout for starting the Strapi server | 16000 (160s) | +| PLAYWRIGHT_ACTION_TIMEOUT | playwright action timeout (ie, click()) | 15000 (15s) | +| PLAYWRIGHT_EXPECT_TIMEOUT | playwright expect waitFor timeout | 10000 (10s) | +| PLAYWRIGHT_TIMEOUT | playwright timeout, for each individual test | 30000 (30s) | +| PLAYWRIGHT_OUTPUT_DIR | playwright output dir, such as trace files | '../test-results/' | +| PLAYWRIGHT_VIDEO | set 'true' to save videos on failed tests | false | + ## Strapi Templates The test-app you create uses a [template](https://docs.strapi.io/developer-docs/latest/setup-deployment-guides/installation/templates.html) found at `e2e/app-template` in this folder we can store our premade content schemas & any customisations we may need such as other plugins / custom fields / endpoints etc. diff --git a/e2e/README.md b/e2e/README.md new file mode 100644 index 0000000000..b9f6b59e50 --- /dev/null +++ b/e2e/README.md @@ -0,0 +1,3 @@ +## End-to-end Playwright Tests + +See contributor docs in docs/docs/guides/e2e for more info diff --git a/playwright.base.config.js b/playwright.base.config.js index fa0a6f6099..4a79bb3168 100644 --- a/playwright.base.config.js +++ b/playwright.base.config.js @@ -1,5 +1,6 @@ // @ts-check const { devices } = require('@playwright/test'); +const { parseType } = require('@strapi/utils'); const getEnvNum = (envVar, defaultValue) => { if (envVar !== undefined && envVar !== null) { @@ -8,6 +9,22 @@ const getEnvNum = (envVar, defaultValue) => { return defaultValue; }; +const getEnvString = (envVar, defaultValue) => { + if (envVar?.trim().length) { + return envVar; + } + + return defaultValue; +}; + +const getEnvBool = (envVar, defaultValue) => { + if (!envVar || envVar === '') { + return defaultValue; + } + + return parseType({ type: 'boolean', value: envVar.toLowerCase() }); +}; + /** * @typedef ConfigOptions * @type {{ port: number; testDir: string; appDir: string }} @@ -28,7 +45,7 @@ const createConfig = ({ port, testDir, appDir }) => ({ * Maximum time expect() should wait for the condition to be met. * For example in `await expect(locator).toHaveText();` */ - timeout: getEnvNum(process.env.PLAYWRIGHT_EXPECT_TIMEOUT, 30 * 1000), + timeout: getEnvNum(process.env.PLAYWRIGHT_EXPECT_TIMEOUT, 10 * 1000), }, /* Run tests in files in parallel */ fullyParallel: false, @@ -46,13 +63,22 @@ const createConfig = ({ port, testDir, appDir }) => ({ baseURL: `http://127.0.0.1:${port}`, /* Default time each action such as `click()` can take to 20s */ - actionTimeout: getEnvNum(process.env.PLAYWRIGHT_ACTION_TIMEOUT, 20 * 1000), + actionTimeout: getEnvNum(process.env.PLAYWRIGHT_ACTION_TIMEOUT, 15 * 1000), /* Collect trace when a test failed on the CI. See https://playwright.dev/docs/trace-viewer Until https://github.com/strapi/strapi/issues/18196 is fixed we can't enable this locally, because the Strapi server restarts every time a new file (trace) is created. */ - trace: process.env.CI ? 'retain-on-failure' : 'off', + trace: 'retain-on-failure', + video: getEnvBool(process.env.PLAYWRIGHT_VIDEO, false) + ? { + mode: 'retain-on-failure', // 'retain-on-failure' to save videos only for failed tests + size: { + width: 1280, + height: 720, + }, + } + : 'off', }, /* Configure projects for major browsers */ @@ -80,7 +106,7 @@ const createConfig = ({ port, testDir, appDir }) => ({ ], /* Folder for test artifacts such as screenshots, videos, traces, etc. */ - outputDir: 'test-results/', + outputDir: getEnvString(process.env.PLAYWRIGHT_OUTPUT_DIR, '../test-results/'), // in the test-apps/e2e dir, to avoid writing files to the running Strapi project dir /* Run your local dev server before starting the tests */ webServer: { From ad6d81cb6ca7fdf563554fdbed649a990b51f21c Mon Sep 17 00:00:00 2001 From: Madhuri Sandbhor Date: Mon, 4 Mar 2024 11:50:38 +0100 Subject: [PATCH 6/7] Feat(content-releases): release status badge (#19611) * feat(content-releases): add status to releases * add docs and fix e2e error * draft: added basic badge with default value * Update docs/docs/docs/01-core/content-releases/00-intro.md Co-authored-by: Simone * Update docs/docs/docs/01-core/content-releases/00-intro.md Co-authored-by: Simone * Update docs/docs/docs/01-core/content-releases/00-intro.md Co-authored-by: Simone * apply marks feedback * don't throw error on lifecycle hooks inside releases * handle when actions are not valid anymore * await for entry validation on releases edit entry * check if are changes in content types attributes to revalidate * fix e2e test * apply marks feedback * fix: removed default status value * fix: release card design updated, capitalize scheduled period * fix: e2e test updated to select always a current date --------- Co-authored-by: Fernando Chavez Co-authored-by: Simone --- .../content-releases/releases-page.spec.ts | 10 ++- .../admin/src/pages/ReleaseDetailsPage.tsx | 10 ++- .../admin/src/pages/ReleasesPage.tsx | 89 +++++++++++++------ .../pages/tests/ReleaseDetailsPage.test.tsx | 38 +++++++- .../pages/tests/mockReleaseDetailsPageData.ts | 5 +- .../src/components/RelativeTime.tsx | 4 +- 6 files changed, 123 insertions(+), 33 deletions(-) diff --git a/e2e/tests/content-releases/releases-page.spec.ts b/e2e/tests/content-releases/releases-page.spec.ts index 2f57cc6c1c..a70bdbd375 100644 --- a/e2e/tests/content-releases/releases-page.spec.ts +++ b/e2e/tests/content-releases/releases-page.spec.ts @@ -70,7 +70,15 @@ describeOnCondition(edition === 'EE')('Releases page', () => { name: 'Date', }) .click(); - await page.getByRole('gridcell', { name: 'Sunday, March 3, 2024' }).click(); + + const formattedDate = new Date().toLocaleDateString('en-US', { + weekday: 'long', + month: 'long', + day: 'numeric', + year: 'numeric', + }); + + await page.getByRole('gridcell', { name: formattedDate }).click(); await page .getByRole('combobox', { diff --git a/packages/core/content-releases/admin/src/pages/ReleaseDetailsPage.tsx b/packages/core/content-releases/admin/src/pages/ReleaseDetailsPage.tsx index 4a7ca2b70f..a7805ec82e 100644 --- a/packages/core/content-releases/admin/src/pages/ReleaseDetailsPage.tsx +++ b/packages/core/content-releases/admin/src/pages/ReleaseDetailsPage.tsx @@ -60,6 +60,8 @@ import { import { useTypedDispatch } from '../store/hooks'; import { getTimezoneOffset } from '../utils/time'; +import { getBadgeProps } from './ReleasesPage'; + import type { ReleaseAction, ReleaseActionGroupBy, @@ -350,7 +352,13 @@ export const ReleaseDetailsLayout = ({ + + {numberOfEntriesText + + (IsSchedulingEnabled && isScheduled ? ` - ${scheduledText}` : '')} + + {release.status} + } navigationAction={ } to="/plugins/content-releases"> diff --git a/packages/core/content-releases/admin/src/pages/ReleasesPage.tsx b/packages/core/content-releases/admin/src/pages/ReleasesPage.tsx index bb0a625a05..97bf508db6 100644 --- a/packages/core/content-releases/admin/src/pages/ReleasesPage.tsx +++ b/packages/core/content-releases/admin/src/pages/ReleasesPage.tsx @@ -4,6 +4,7 @@ import * as React from 'react'; import { useLicenseLimits } from '@strapi/admin/strapi-admin'; import { Alert, + Badge, Box, Button, ContentLayout, @@ -39,7 +40,7 @@ import { useIntl } from 'react-intl'; import { useHistory, useLocation } from 'react-router-dom'; import styled from 'styled-components'; -import { GetReleases } from '../../../shared/contracts/releases'; +import { GetReleases, type Release } from '../../../shared/contracts/releases'; import { ReleaseModal, FormValues } from '../components/ReleaseModal'; import { PERMISSIONS } from '../constants'; import { isAxiosError } from '../services/axios'; @@ -62,6 +63,37 @@ const LinkCard = styled(Link)` display: block; `; +const CapitalizeRelativeTime = styled(RelativeTime)` + text-transform: capitalize; +`; + +const getBadgeProps = (status: Release['status']) => { + let color; + switch (status) { + case 'ready': + color = 'success'; + break; + case 'blocked': + color = 'warning'; + break; + case 'failed': + color = 'danger'; + break; + case 'done': + color = 'primary'; + break; + case 'empty': + default: + color = 'neutral'; + } + + return { + textColor: `${color}600`, + backgroundColor: `${color}100`, + borderColor: `${color}200`, + }; +}; + const ReleasesGrid = ({ sectionTitle, releases = [], isError = false }: ReleasesGridProps) => { const { formatMessage } = useIntl(); const IsSchedulingEnabled = window.strapi.future.isEnabled('contentReleasesScheduling'); @@ -89,7 +121,7 @@ const ReleasesGrid = ({ sectionTitle, releases = [], isError = false }: Releases return ( - {releases.map(({ id, name, actions, scheduledAt }) => ( + {releases.map(({ id, name, actions, scheduledAt, status }) => ( - - {name} - - - {IsSchedulingEnabled ? ( - scheduledAt ? ( - + + + {name} + + + {IsSchedulingEnabled ? ( + scheduledAt ? ( + + ) : ( + formatMessage({ + id: 'content-releases.pages.Releases.not-scheduled', + defaultMessage: 'Not scheduled', + }) + ) ) : ( - formatMessage({ - id: 'content-releases.pages.Releases.not-scheduled', - defaultMessage: 'Not scheduled', - }) - ) - ) : ( - formatMessage( - { - id: 'content-releases.page.Releases.release-item.entries', - defaultMessage: - '{number, plural, =0 {No entries} one {# entry} other {# entries}}', - }, - { number: actions.meta.count } - ) - )} - + formatMessage( + { + id: 'content-releases.page.Releases.release-item.entries', + defaultMessage: + '{number, plural, =0 {No entries} one {# entry} other {# entries}}', + }, + { number: actions.meta.count } + ) + )} + + + {status} @@ -405,4 +440,4 @@ const ReleasesPage = () => { ); }; -export { ReleasesPage }; +export { ReleasesPage, getBadgeProps }; diff --git a/packages/core/content-releases/admin/src/pages/tests/ReleaseDetailsPage.test.tsx b/packages/core/content-releases/admin/src/pages/tests/ReleaseDetailsPage.test.tsx index e97d905dbb..0edc7893b3 100644 --- a/packages/core/content-releases/admin/src/pages/tests/ReleaseDetailsPage.test.tsx +++ b/packages/core/content-releases/admin/src/pages/tests/ReleaseDetailsPage.test.tsx @@ -52,6 +52,9 @@ describe('Releases details page', () => { const releaseSubtitle = await screen.findAllByText('No entries'); expect(releaseSubtitle[0]).toBeInTheDocument(); + const releaseStatus = screen.getByText('empty'); + expect(releaseStatus).toBeInTheDocument(); + const moreButton = screen.getByRole('button', { name: 'Release edit and delete menu' }); expect(moreButton).toBeInTheDocument(); @@ -146,7 +149,7 @@ describe('Releases details page', () => { expect(tables).toHaveLength(2); }); - it('shows the right status', async () => { + it('shows the right status for unpublished release', async () => { server.use( rest.get('/content-releases/:releaseId', (req, res, ctx) => res(ctx.json(mockReleaseDetailsPageData.withActionsHeaderData)) @@ -160,7 +163,7 @@ describe('Releases details page', () => { ); render(, { - initialEntries: [{ pathname: `/content-releases/1` }], + initialEntries: [{ pathname: `/content-releases/2` }], }); const releaseTitle = await screen.findByText( @@ -168,6 +171,10 @@ describe('Releases details page', () => { ); expect(releaseTitle).toBeInTheDocument(); + const releaseStatus = screen.getByText('ready'); + expect(releaseStatus).toBeInTheDocument(); + expect(releaseStatus).toHaveStyle(`color: #328048`); + const cat1Row = screen.getByRole('row', { name: /cat1/i }); expect(within(cat1Row).getByRole('gridcell', { name: 'Ready to publish' })).toBeInTheDocument(); @@ -181,4 +188,31 @@ describe('Releases details page', () => { within(add1Row).getByRole('gridcell', { name: 'Already published' }) ).toBeInTheDocument(); }); + + it('shows the right release status for published release', async () => { + server.use( + rest.get('/content-releases/:releaseId', (req, res, ctx) => + res(ctx.json(mockReleaseDetailsPageData.withActionsAndPublishedHeaderData)) + ) + ); + + server.use( + rest.get('/content-releases/:releaseId/actions', (req, res, ctx) => + res(ctx.json(mockReleaseDetailsPageData.withMultipleActionsBodyData)) + ) + ); + + render(, { + initialEntries: [{ pathname: `/content-releases/3` }], + }); + + const releaseTitle = await screen.findByText( + mockReleaseDetailsPageData.withActionsAndPublishedHeaderData.data.name + ); + expect(releaseTitle).toBeInTheDocument(); + + const releaseStatus = screen.getByText('done'); + expect(releaseStatus).toBeInTheDocument(); + expect(releaseStatus).toHaveStyle(`color: #4945ff`); + }); }); diff --git a/packages/core/content-releases/admin/src/pages/tests/mockReleaseDetailsPageData.ts b/packages/core/content-releases/admin/src/pages/tests/mockReleaseDetailsPageData.ts index 4a0c9d3bb0..7a61e30695 100644 --- a/packages/core/content-releases/admin/src/pages/tests/mockReleaseDetailsPageData.ts +++ b/packages/core/content-releases/admin/src/pages/tests/mockReleaseDetailsPageData.ts @@ -9,6 +9,7 @@ const RELEASE_NO_ACTIONS_HEADER_MOCK_DATA = { createdAt: '2023-11-16T15:18:32.560Z', updatedAt: '2023-11-16T15:18:32.560Z', releasedAt: null, + status: 'empty', createdBy: { id: 1, firstname: 'Admin', @@ -50,6 +51,7 @@ const RELEASE_WITH_ACTIONS_HEADER_MOCK_DATA = { createdAt: '2023-11-16T15:18:32.560Z', updatedAt: '2023-11-16T15:18:32.560Z', releasedAt: null, + status: 'ready', createdBy: { id: 1, firstname: 'Admin', @@ -70,11 +72,12 @@ const RELEASE_WITH_ACTIONS_HEADER_MOCK_DATA = { const PUBLISHED_RELEASE_WITH_ACTIONS_HEADER_MOCK_DATA = { data: { - id: 2, + id: 3, name: 'release with actions', createdAt: '2023-11-16T15:18:32.560Z', updatedAt: '2023-11-16T15:18:32.560Z', releasedAt: '2023-11-16T15:18:32.560Z', + status: 'done', createdBy: { id: 1, firstname: 'Admin', diff --git a/packages/core/helper-plugin/src/components/RelativeTime.tsx b/packages/core/helper-plugin/src/components/RelativeTime.tsx index 1f54585ac4..b57ecc74c8 100644 --- a/packages/core/helper-plugin/src/components/RelativeTime.tsx +++ b/packages/core/helper-plugin/src/components/RelativeTime.tsx @@ -12,6 +12,7 @@ interface CustomInterval { export interface RelativeTimeProps { timestamp: Date; customIntervals?: CustomInterval[]; + className?: string; } /** @@ -28,7 +29,7 @@ export interface RelativeTimeProps { * ]} * ``` */ -const RelativeTime = ({ timestamp, customIntervals = [] }: RelativeTimeProps) => { +const RelativeTime = ({ timestamp, customIntervals = [], className }: RelativeTimeProps) => { const { formatRelativeTime, formatDate, formatTime } = useIntl(); const interval = intervalToDuration({ @@ -54,6 +55,7 @@ const RelativeTime = ({ timestamp, customIntervals = [] }: RelativeTimeProps) => From 20c4c0d0016306a8264891064bd3f51f7dc58640 Mon Sep 17 00:00:00 2001 From: Josh <37798644+joshuaellis@users.noreply.github.com> Date: Mon, 4 Mar 2024 15:14:54 +0000 Subject: [PATCH 7/7] test(e2e): fix content-releases bug by advancing time by a day (#19670) --- e2e/tests/content-releases/releases-page.spec.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/e2e/tests/content-releases/releases-page.spec.ts b/e2e/tests/content-releases/releases-page.spec.ts index a70bdbd375..95bc23b823 100644 --- a/e2e/tests/content-releases/releases-page.spec.ts +++ b/e2e/tests/content-releases/releases-page.spec.ts @@ -71,7 +71,9 @@ describeOnCondition(edition === 'EE')('Releases page', () => { }) .click(); - const formattedDate = new Date().toLocaleDateString('en-US', { + const date = new Date(); + date.setDate(date.getDate() + 1); + const formattedDate = date.toLocaleDateString('en-US', { weekday: 'long', month: 'long', day: 'numeric',