Skip to content

Commit

Permalink
feat: use audit info in events (#6872)
Browse files Browse the repository at this point in the history
I've tried to use/add the audit info to all events I could see/find.
This makes this PR necessarily huge, because we do store quite a few
events. 

I realise it might not be complete yet, but tests
run green, and I think we now have a pattern to follow for other events.
  • Loading branch information
chriswk committed Apr 18, 2024
1 parent bf4c29b commit cf2bd28
Show file tree
Hide file tree
Showing 107 changed files with 2,877 additions and 2,473 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
} from '../../../../test/e2e/helpers/test-helper';
import getLogger from '../../../../test/fixtures/no-logger';
import { DEFAULT_ENV } from '../../../util/constants';
import type { IUserWithRootRole } from '../../../types';
import { type IUserWithRootRole, TEST_AUDIT_USER } from '../../../types';

let app: IUnleashTest;
let db: ITestDb;
Expand Down Expand Up @@ -73,11 +73,14 @@ beforeAll(async () => {
db.rawDatabase,
);

dummyAdmin = await app.services.userService.createUser({
name: 'Some Name',
email: 'test@getunleash.io',
rootRole: RoleName.ADMIN,
});
dummyAdmin = await app.services.userService.createUser(
{
name: 'Some Name',
email: 'test@getunleash.io',
rootRole: RoleName.ADMIN,
},
TEST_AUDIT_USER,
);
});

afterEach(async () => {
Expand Down Expand Up @@ -137,10 +140,12 @@ test('should support filtering on project', async () => {
await app.services.projectService.createProject(
{ name: 'projectA', id: 'projecta' },
dummyAdmin,
TEST_AUDIT_USER,
);
await app.services.projectService.createProject(
{ name: 'projectB', id: 'projectb' },
dummyAdmin,
TEST_AUDIT_USER,
);
await app.createFeature('ab_test1', 'projecta');
await app.createFeature('bd_test2', 'projectb');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -218,6 +218,7 @@ export default class DependentFeaturesController extends Controller {
feature,
},
req.user,
req.audit,
),
);

Expand All @@ -238,6 +239,7 @@ export default class DependentFeaturesController extends Controller {
},
projectId,
req.user,
req.audit,
),
);
res.status(200).end();
Expand All @@ -250,7 +252,12 @@ export default class DependentFeaturesController extends Controller {
const { child, projectId } = req.params;

await this.dependentFeaturesService.transactional((service) =>
service.deleteFeaturesDependencies([child], projectId, req.user),
service.deleteFeaturesDependencies(
[child],
projectId,
req.user,
req.audit,
),
);
res.status(200).end();
}
Expand Down
92 changes: 46 additions & 46 deletions src/lib/features/dependent-features/dependent-features-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,14 @@ import type {
} from './dependent-features';
import type { IDependentFeaturesReadModel } from './dependent-features-read-model-type';
import type { EventService } from '../../services';
import type { IUser } from '../../server-impl';
import { SKIP_CHANGE_REQUEST } from '../../types';
import type { IAuditUser, IUser } from '../../server-impl';
import {
FeatureDependenciesRemovedEvent,
FeatureDependencyAddedEvent,
FeatureDependencyRemovedEvent,
SKIP_CHANGE_REQUEST,
} from '../../types';
import type { IChangeRequestAccessReadModel } from '../change-request-access-service/change-request-access-read-model';
import { extractUsernameFromUser } from '../../util';
import type { IFeaturesReadModel } from '../feature-toggle/types/features-read-model-type';

interface IDependentFeaturesServiceDeps {
Expand Down Expand Up @@ -52,8 +56,7 @@ export class DependentFeaturesService {
newFeatureName,
projectId,
}: { featureName: string; newFeatureName: string; projectId: string },
user: string,
userId: number,
auditUser: IAuditUser,
) {
const parents =
await this.dependentFeaturesReadModel.getParents(featureName);
Expand All @@ -66,8 +69,7 @@ export class DependentFeaturesService {
enabled: parent.enabled,
variants: parent.variants,
},
user,
userId,
auditUser,
),
),
);
Expand All @@ -77,22 +79,21 @@ export class DependentFeaturesService {
{ child, projectId }: { child: string; projectId: string },
dependentFeature: CreateDependentFeatureSchema,
user: IUser,
auditUser: IAuditUser,
): Promise<void> {
await this.stopWhenChangeRequestsEnabled(projectId, user);

return this.unprotectedUpsertFeatureDependency(
{ child, projectId },
dependentFeature,
extractUsernameFromUser(user),
user.id,
auditUser,
);
}

async unprotectedUpsertFeatureDependency(
{ child, projectId }: { child: string; projectId: string },
dependentFeature: CreateDependentFeatureSchema,
user: string,
userId: number,
auditUser: IAuditUser,
): Promise<void> {
const { enabled, feature: parent, variants } = dependentFeature;

Expand Down Expand Up @@ -148,82 +149,81 @@ export class DependentFeaturesService {
variants,
};
await this.dependentFeaturesStore.upsert(featureDependency);
await this.eventService.storeEvent({
type: 'feature-dependency-added',
project: projectId,
featureName: child,
createdBy: user,
createdByUserId: userId,
data: {
feature: parent,
enabled: featureDependency.enabled,
...(variants !== undefined && { variants }),
},
});
await this.eventService.storeEvent(
new FeatureDependencyAddedEvent({
project: projectId,
featureName: child,
auditUser,
data: {
feature: parent,
enabled: featureDependency.enabled,
...(variants !== undefined && { variants }),
},
}),
);
}

async deleteFeatureDependency(
dependency: FeatureDependencyId,
projectId: string,
user: IUser,
auditUser: IAuditUser,
): Promise<void> {
await this.stopWhenChangeRequestsEnabled(projectId, user);

return this.unprotectedDeleteFeatureDependency(
dependency,
projectId,
extractUsernameFromUser(user),
user.id,
auditUser,
);
}

async unprotectedDeleteFeatureDependency(
dependency: FeatureDependencyId,
projectId: string,
user: string,
userId: number,
auditUser: IAuditUser,
): Promise<void> {
await this.dependentFeaturesStore.delete(dependency);
await this.eventService.storeEvent({
type: 'feature-dependency-removed',
project: projectId,
featureName: dependency.child,
createdBy: user,
createdByUserId: userId,
data: { feature: dependency.parent },
});
await this.eventService.storeEvent(
new FeatureDependencyRemovedEvent({
project: projectId,
featureName: dependency.child,
auditUser,
data: { feature: dependency.parent },
}),
);
}

async deleteFeaturesDependencies(
features: string[],
projectId: string,
user: IUser,
auditUser: IAuditUser,
): Promise<void> {
await this.stopWhenChangeRequestsEnabled(projectId, user);

return this.unprotectedDeleteFeaturesDependencies(
features,
projectId,
extractUsernameFromUser(user),
user.id,
auditUser,
);
}

async unprotectedDeleteFeaturesDependencies(
features: string[],
projectId: string,
user: string,
userId: number,
auditUser: IAuditUser,
): Promise<void> {
await this.dependentFeaturesStore.deleteAll(features);
await this.eventService.storeEvents(
features.map((feature) => ({
type: 'feature-dependencies-removed',
project: projectId,
featureName: feature,
createdBy: user,
createdByUserId: userId,
})),
features.map(
(feature) =>
new FeatureDependenciesRemovedEvent({
project: projectId,
featureName: feature,
auditUser,
}),
),
);
}

Expand Down
9 changes: 5 additions & 4 deletions src/lib/features/events/event-store.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import {
type IEvent,
type IBaseEvent,
SEGMENT_UPDATED,
FEATURE_IMPORT,
FEATURES_IMPORTED,
type IBaseEvent,
type IEvent,
type IEventType,
SEGMENT_UPDATED,
} from '../../types/events';
import type { LogProvider, Logger } from '../../logger';
import type { Logger, LogProvider } from '../../logger';
import type { IEventStore } from '../../types/stores/event-store';
import type { ITag } from '../../types/model';
import type { SearchEventsSchema } from '../../openapi/spec/search-events-schema';
Expand Down Expand Up @@ -394,6 +394,7 @@ class EventStore implements IEventStore {
feature_name: e.featureName,
project: e.project,
environment: e.environment,
ip: e.ip,
};
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,11 +120,7 @@ class ExportImportController extends Controller {
const query = req.body;
const userName = extractUsername(req);

const data = await this.exportService.export(
query,
userName,
req.user.id,
);
const data = await this.exportService.export(query, req.audit);

this.openApiService.respondWithValidation(
200,
Expand Down Expand Up @@ -159,7 +155,7 @@ class ExportImportController extends Controller {
res: Response,
): Promise<void> {
this.verifyExportImportEnabled();
const { user } = req;
const { user, audit } = req;

if (user instanceof ApiUser && user.type === 'admin') {
throw new BadDataError(
Expand All @@ -170,7 +166,7 @@ class ExportImportController extends Controller {
const dto = req.body;

await this.importService.transactional((service) =>
service.import(dto, user),
service.import(dto, user, audit),
);

res.status(200).end();
Expand Down

0 comments on commit cf2bd28

Please sign in to comment.