diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeSegmentName.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeSegmentName.tsx deleted file mode 100644 index dcb1ea979c..0000000000 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeSegmentName.tsx +++ /dev/null @@ -1,18 +0,0 @@ -import type { FC } from 'react'; -import { NameWithChangeInfo } from './NameWithChangeInfo/NameWithChangeInfo.tsx'; - -type ChangeSegmentNameProps = { - name: string; - previousName?: string; -}; - -export const ChangeSegmentName: FC = ({ - name, - previousName, -}) => { - return ( - - - - ); -}; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeStrategyName.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeStrategyName.tsx deleted file mode 100644 index 20eb1fca20..0000000000 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ChangeStrategyName.tsx +++ /dev/null @@ -1,30 +0,0 @@ -import type { FC } from 'react'; -import { formatStrategyName } from 'utils/strategyNames'; -import { styled } from '@mui/material'; -import { textTruncated } from 'themes/themeStyles'; -import { NameWithChangeInfo } from './NameWithChangeInfo/NameWithChangeInfo.tsx'; -import { Truncator } from 'component/common/Truncator/Truncator.tsx'; - -type ChangeStrategyNameProps = { - name: string; - title?: string; - previousTitle?: string; -}; - -const Truncated = styled('span')(() => ({ - ...textTruncated, - maxWidth: 500, -})); - -export const ChangeStrategyName: FC = ({ - name, - title, - previousTitle, -}) => { - return ( - - {formatStrategyName(name)} - - - ); -}; diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyExecutionOrder.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyExecutionOrder.tsx index 1733e428c9..ea1b6de57d 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyExecutionOrder.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/EnvironmentStrategyExecutionOrder/EnvironmentStrategyExecutionOrder.tsx @@ -109,7 +109,7 @@ export const EnvironmentStrategyExecutionOrder = ({
- Change + View change View diff {actions} diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/NameWithChangeInfo/NameWithChangeInfo.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/NameWithChangeInfo/NameWithChangeInfo.tsx index d840ed899e..9fddf0a210 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/NameWithChangeInfo/NameWithChangeInfo.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/NameWithChangeInfo/NameWithChangeInfo.tsx @@ -6,7 +6,6 @@ import { textTruncated } from 'themes/themeStyles'; const Truncated = styled('span')(() => ({ ...textTruncated, maxWidth: 500, - display: 'block', })); const NewName = styled(Typography)({ diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ReleasePlanChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ReleasePlanChange.tsx index 965aaed6f7..49d4c34daf 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ReleasePlanChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/ReleasePlanChange.tsx @@ -89,7 +89,7 @@ const StartMilestone: FC<{
- Change + View change View diff {actions} @@ -202,7 +202,7 @@ const AddReleasePlan: FC<{
- Changes + View change View diff {actions} diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx index 08bd89552f..d87f17c24a 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/SegmentChangeDetails.tsx @@ -17,7 +17,7 @@ import { Deleted, } from './Change.styles.tsx'; import { SegmentDiff } from './SegmentDiff.tsx'; -import { ChangeSegmentName } from './ChangeSegmentName.tsx'; +import { NameWithChangeInfo } from './NameWithChangeInfo/NameWithChangeInfo.tsx'; const ActionsContainer = styled('div')(({ theme }) => ({ display: 'flex', @@ -57,7 +57,7 @@ export const SegmentChangeDetails: FC<{ const actionsWithTabs = ( - Change + View change View diff {actions} @@ -72,8 +72,8 @@ export const SegmentChangeDetails: FC<{ Deleting segment - @@ -102,8 +102,8 @@ export const SegmentChangeDetails: FC<{ Editing segment - diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx index 01c18edbdf..8121ec7f28 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.test.tsx @@ -243,7 +243,6 @@ test('Deleting strategy before change request is applied diffs against current s ); await screen.findByText('Deleting strategy'); - await screen.findByText('Gradual rollout'); await screen.findByText('current_title'); await screen.findByText('current_variant'); @@ -299,7 +298,6 @@ test('Deleting strategy after change request is applied diffs against the snapsh ); await screen.findByText('Deleting strategy'); - await screen.findByText('Gradual rollout'); await screen.findByText('snapshot_title'); expect(screen.queryByText('current_title')).not.toBeInTheDocument(); diff --git a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx index 9a51ddae1a..47d128ec2f 100644 --- a/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx +++ b/frontend/src/component/changeRequest/ChangeRequest/Changes/Change/StrategyChange.tsx @@ -18,7 +18,6 @@ import { EnvironmentVariantsTable } from 'component/feature/FeatureView/FeatureV import { ChangeOverwriteWarning } from './ChangeOverwriteWarning/ChangeOverwriteWarning.tsx'; import type { IFeatureStrategy } from 'interfaces/strategy'; import { Tab, TabList, TabPanel, Tabs } from './ChangeTabComponents.tsx'; -import { ChangeStrategyName } from './ChangeStrategyName.tsx'; import { StrategyDiff } from './StrategyDiff.tsx'; import { Action, @@ -27,6 +26,7 @@ import { ChangeItemWrapper, Deleted, } from './Change.styles.tsx'; +import { NameWithChangeInfo } from './NameWithChangeInfo/NameWithChangeInfo.tsx'; const StyledBox: FC<{ children?: React.ReactNode }> = styled(Box)( ({ theme }) => ({ @@ -103,10 +103,6 @@ const DeleteStrategy: FC<{ currentStrategy: IFeatureStrategy | undefined; actions?: ReactNode; }> = ({ change, changeRequestState, currentStrategy, actions }) => { - const name = - changeRequestState === 'Applied' - ? change.payload?.snapshot?.name - : currentStrategy?.name; const title = changeRequestState === 'Applied' ? change.payload?.snapshot?.title @@ -121,7 +117,10 @@ const DeleteStrategy: FC<{ Deleting strategy - + {actions} @@ -175,10 +174,9 @@ const UpdateStrategy: FC<{ wasDisabled={currentStrategy?.disabled} willBeDisabled={change.payload?.disabled} /> - {actions} @@ -248,10 +246,7 @@ const AddStrategy: FC<{ Adding {isDefaultChange && 'default'} strategy - + - Change + View change View diff {actions} diff --git a/frontend/src/component/changeRequest/ChangeRequestSidebar/ChangeRequestSidebar.tsx b/frontend/src/component/changeRequest/ChangeRequestSidebar/ChangeRequestSidebar.tsx index f855af363d..0baffc141e 100644 --- a/frontend/src/component/changeRequest/ChangeRequestSidebar/ChangeRequestSidebar.tsx +++ b/frontend/src/component/changeRequest/ChangeRequestSidebar/ChangeRequestSidebar.tsx @@ -1,6 +1,6 @@ import { useState, type VFC } from 'react'; import { Box, Button, styled, Typography } from '@mui/material'; -import { DynamicSidebarModal } from 'component/common/SidebarModal/SidebarModal'; +import { SidebarModal } from 'component/common/SidebarModal/SidebarModal'; import { PageContent } from 'component/common/PageContent/PageContent'; import { PageHeader } from 'component/common/PageHeader/PageHeader'; import CheckCircle from '@mui/icons-material/CheckCircle'; @@ -22,7 +22,6 @@ interface IChangeRequestSidebarProps { const StyledPageContent = styled(PageContent)(({ theme }) => ({ height: '100vh', overflow: 'auto', - minWidth: '50vw', padding: theme.spacing(6), [theme.breakpoints.down('md')]: { padding: theme.spacing(4, 2), @@ -106,11 +105,7 @@ export const ChangeRequestSidebar: VFC = ({ if (!loading && !data) { return ( - + } @@ -119,16 +114,12 @@ export const ChangeRequestSidebar: VFC = ({ {/* FIXME: empty state */} Close - + ); } return ( - + } @@ -159,6 +150,6 @@ export const ChangeRequestSidebar: VFC = ({ ))} - + ); }; diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.test.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.test.tsx index bdc671ea45..ff63dae6c8 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.test.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.test.tsx @@ -154,6 +154,27 @@ describe('validators', () => { ]); }, ); + + test.each(multipleValueOperators)( + 'multi-value operator %s should reject fully duplicate inputs and accept new values', + (operator) => { + const initial: IConstraint = { + contextName: 'context-field', + operator: operator, + values: ['a', 'b'], + }; + + const { result } = renderHook(() => + useEditableConstraint(initial, () => {}), + ); + + checkValidator(result.current.validator, [ + ['a', false], + [['a', 'c'], true], + [['a', 'b'], false], + ]); + }, + ); }); describe('legal values', () => { diff --git a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.tsx b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.tsx index 701f003458..91f9ce0212 100644 --- a/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.tsx +++ b/frontend/src/component/feature/FeatureStrategy/FeatureStrategyConstraints/EditableConstraint/useEditableConstraint/useEditableConstraint.tsx @@ -4,6 +4,7 @@ import type { IConstraint } from 'interfaces/strategy'; import { type EditableConstraint, fromIConstraint, + isMultiValueConstraint, isSingleValueConstraint, toIConstraint, } from './editable-constraint-type.ts'; @@ -16,8 +17,8 @@ import { type ConstraintUpdateAction, } from './constraint-reducer.ts'; import { - type ConstraintValidationResult, constraintValidator, + type ConstraintValidationResult, } from './constraint-validator.ts'; import { getDeletedLegalValues, @@ -76,7 +77,20 @@ export const useEditableConstraint = ( [JSON.stringify(context), localConstraint.contextName], ); - const validator = constraintValidator(localConstraint.operator); + const baseValidator = constraintValidator(localConstraint.operator); + + const validator = (...values: string[]) => { + if ( + isMultiValueConstraint(localConstraint) && + values.every((value) => localConstraint.values.has(value)) + ) { + if (values.length === 1) { + return [false, `${values[0]} is already added.`]; + } + return [false, `All the values are already added`]; + } + return baseValidator(...values); + }; useEffect(() => { if ( @@ -104,7 +118,7 @@ export const useEditableConstraint = ( isSingleValueConstraint(localConstraint) ) { return getInvalidLegalValues( - (value) => validator(value)[0], + (value) => baseValidator(value)[0], contextDefinition.legalValues, ); } diff --git a/src/lib/app.ts b/src/lib/app.ts index 4b09a14a70..0037c0d88b 100644 --- a/src/lib/app.ts +++ b/src/lib/app.ts @@ -103,10 +103,7 @@ export default async function getApp( if (config.enableOAS && services.openApiService) { services.openApiService.useDocs(app); } - // Support CORS preflight requests for the frontend endpoints. - // Preflight requests should not have Authorization headers, - // so this must be handled before the API token middleware. - app.options( + app.use( `${baseUriPath}/api/frontend*`, corsOriginMiddleware(services, config), ); diff --git a/src/lib/features/events/event-store.ts b/src/lib/features/events/event-store.ts index 0d8a8f0ca6..43d86b43b4 100644 --- a/src/lib/features/events/event-store.ts +++ b/src/lib/features/events/event-store.ts @@ -28,6 +28,8 @@ import type { ProjectActivitySchema } from '../../openapi/index.js'; import type { IQueryParam } from '../feature-toggle/types/feature-toggle-strategies-store-type.js'; import { applyGenericQueryParams } from '../feature-search/search-utils.js'; import type { ITag } from '../../tags/index.js'; +import metricsHelper from '../../util/metrics-helper.js'; +import { DB_TIME } from '../../metric-events.js'; const EVENT_COLUMNS = [ 'id', @@ -113,26 +115,38 @@ export class EventStore implements IEventStore { private logger: Logger; + private metricTimer: Function; + // a new DB has to be injected per transaction constructor(db: Db, getLogger: LogProvider) { this.db = db; this.logger = getLogger('event-store'); + this.metricTimer = (action) => + metricsHelper.wrapTimer(this.eventEmitter, DB_TIME, { + store: 'event', + action, + }); } async store(event: IBaseEvent): Promise { + const stopTimer = this.metricTimer('store'); try { await this.db(TABLE) .insert(this.eventToDbRow(event)) .returning(EVENT_COLUMNS); } catch (error: unknown) { this.logger.warn(`Failed to store "${event.type}" event: ${error}`); + } finally { + stopTimer(); } } async count(): Promise { + const stopTimer = this.metricTimer('count'); const count = await this.db(TABLE) .count>() .first(); + stopTimer(); if (!count) { return 0; } @@ -147,8 +161,10 @@ export class EventStore implements IEventStore { queryParams: IQueryParam[], query?: IEventSearchParams['query'], ): Promise { + const stopTimer = this.metricTimer('searchEventsCount'); const searchQuery = this.buildSearchQuery(queryParams, query); const count = await searchQuery.count().first(); + stopTimer(); if (!count) { return 0; } @@ -160,6 +176,7 @@ export class EventStore implements IEventStore { } async batchStore(events: IBaseEvent[]): Promise { + const stopTimer = this.metricTimer('batchStore'); try { await this.db(TABLE).insert( events.map((event) => this.eventToDbRow(event)), @@ -169,10 +186,13 @@ export class EventStore implements IEventStore { `Failed to store events: ${JSON.stringify(events)}`, error, ); + } finally { + stopTimer(); } } async getMaxRevisionId(largerThan: number = 0): Promise { + const stopTimer = this.metricTimer('getMaxRevisionId'); const row = await this.db(TABLE) .max('id') .where((builder) => @@ -193,10 +213,12 @@ export class EventStore implements IEventStore { ) .andWhere('id', '>=', largerThan) .first(); + stopTimer(); return row?.max ?? 0; } async getRevisionRange(start: number, end: number): Promise { + const stopTimer = this.metricTimer('getRevisionRange'); const query = this.db .select(EVENT_COLUMNS) .from(TABLE) @@ -246,6 +268,7 @@ export class EventStore implements IEventStore { } async query(operations: IQueryOperations[]): Promise { + const stopTimer = this.metricTimer('query'); try { let query: Knex.QueryBuilder = this.select(); @@ -271,10 +294,13 @@ export class EventStore implements IEventStore { return rows.map(this.rowToEvent); } catch (e) { return []; + } finally { + stopTimer(); } } async queryCount(operations: IQueryOperations[]): Promise { + const stopTimer = this.metricTimer('queryCount'); try { let query: Knex.QueryBuilder = this.db.count().from(TABLE); @@ -300,6 +326,8 @@ export class EventStore implements IEventStore { return Number.parseInt(queryResult.count || 0); } catch (e) { return 0; + } finally { + stopTimer(); } } @@ -355,6 +383,7 @@ export class EventStore implements IEventStore { } async getEvents(query?: Object): Promise { + const stopTimer = this.metricTimer('getEvents'); try { let qB = this.db .select(EVENT_COLUMNS) @@ -371,6 +400,8 @@ export class EventStore implements IEventStore { return rows.map(this.rowToEvent); } catch (err) { return []; + } finally { + stopTimer(); } } @@ -379,6 +410,7 @@ export class EventStore implements IEventStore { queryParams: IQueryParam[], options?: { withIp?: boolean }, ): Promise { + const stopTimer = this.metricTimer('searchEvents'); const query = this.buildSearchQuery(queryParams, params.query) .select(options?.withIp ? [...EVENT_COLUMNS, 'ip'] : EVENT_COLUMNS) .orderBy([ @@ -396,6 +428,8 @@ export class EventStore implements IEventStore { ); } catch (err) { return []; + } finally { + stopTimer(); } } @@ -420,6 +454,7 @@ export class EventStore implements IEventStore { } async getEventCreators(): Promise> { + const stopTimer = this.metricTimer('getEventCreators'); const query = this.db('events') .distinctOn('events.created_by_user_id') .leftJoin('users', 'users.id', '=', 'events.created_by_user_id') @@ -437,6 +472,7 @@ export class EventStore implements IEventStore { ]); const result = await query; + stopTimer(); return result .filter((row: any) => row.name || row.username || row.email) .map((row: any) => ({ @@ -448,6 +484,7 @@ export class EventStore implements IEventStore { async getProjectRecentEventActivity( project: string, ): Promise { + const stopTimer = this.metricTimer('getProjectRecentEventActivity'); const result = await this.db('events') .select( this.db.raw("TO_CHAR(created_at::date, 'YYYY-MM-DD') AS date"), @@ -462,6 +499,7 @@ export class EventStore implements IEventStore { .groupBy(this.db.raw("TO_CHAR(created_at::date, 'YYYY-MM-DD')")) .orderBy('date', 'asc'); + stopTimer(); return result.map((row) => ({ date: row.date, count: Number(row.count), @@ -531,10 +569,12 @@ export class EventStore implements IEventStore { } async setUnannouncedToAnnounced(): Promise { + const stopTimer = this.metricTimer('setUnannouncedToAnnounced'); const rows = await this.db(TABLE) .update({ announced: true }) .where('announced', false) .returning(EVENT_COLUMNS); + stopTimer(); return rows.map(this.rowToEvent); } @@ -545,6 +585,7 @@ export class EventStore implements IEventStore { } async setCreatedByUserId(batchSize: number): Promise { + const stopTimer = this.metricTimer('setCreatedByUserId'); const API_TOKEN_TABLE = 'api_tokens'; const toUpdate = await this.db(`${TABLE} as e`) @@ -592,6 +633,7 @@ export class EventStore implements IEventStore { }); await Promise.all(updatePromises); + stopTimer(); return toUpdate.length; } } diff --git a/src/lib/features/frontend-api/frontend-api-controller.ts b/src/lib/features/frontend-api/frontend-api-controller.ts index 85ce27fe2e..648868e04a 100644 --- a/src/lib/features/frontend-api/frontend-api-controller.ts +++ b/src/lib/features/frontend-api/frontend-api-controller.ts @@ -19,7 +19,6 @@ import { } from '../../openapi/index.js'; import type { Context } from 'unleash-client'; import { enrichContextWithIp } from './index.js'; -import { corsOriginMiddleware } from '../../middleware/index.js'; import NotImplementedError from '../../error/not-implemented-error.js'; import rateLimit from 'express-rate-limit'; import { minutesToMilliseconds } from 'date-fns'; @@ -65,10 +64,6 @@ export default class FrontendAPIController extends Controller { functionName, }); - // Support CORS requests for the frontend endpoints. - // Preflight requests are handled in `app.ts`. - this.app.use(corsOriginMiddleware(services, config)); - this.route({ method: 'get', path: '', diff --git a/src/lib/middleware/request-logger.ts b/src/lib/middleware/request-logger.ts index 91468b3ed9..c9c49ac895 100644 --- a/src/lib/middleware/request-logger.ts +++ b/src/lib/middleware/request-logger.ts @@ -8,21 +8,27 @@ import { const requestLogger: (config: IUnleashConfig) => RequestHandler = (config) => { const logger = config.getLogger('HTTP'); - const enable = config.server.enableRequestLogger; + const requestLoggerEnabled = config.server.enableRequestLogger; const impactMetrics = config.flagResolver.impactMetrics; return (req, res, next) => { - if (enable) { - res.on('finish', () => { - const { pathname } = url.parse(req.originalUrl); + const impactMetricsEnabled = + config.flagResolver.isEnabled('impactMetrics'); + + res.on('finish', () => { + if (impactMetricsEnabled && impactMetrics) { if (res.statusCode >= 400 && res.statusCode < 500) { - impactMetrics?.incrementCounter(CLIENT_ERROR_COUNT); + impactMetrics.incrementCounter(CLIENT_ERROR_COUNT); } if (res.statusCode >= 500) { - impactMetrics?.incrementCounter(SERVER_ERROR_COUNT); + impactMetrics.incrementCounter(SERVER_ERROR_COUNT); } + } + + if (requestLoggerEnabled) { + const { pathname } = url.parse(req.originalUrl); logger.info(`${res.statusCode} ${req.method} ${pathname}`); - }); - } + } + }); next(); }; };