Skip to content

Commit

Permalink
feat: add new ActionItemExporter functionality
Browse files Browse the repository at this point in the history
Allows for action items to be exported when a review is approved. Attached via the new optional configuration of `Providers.actionItemExporters`. Fixes #61
  • Loading branch information
Tethik committed Dec 13, 2023
1 parent d585e25 commit 1f87897
Show file tree
Hide file tree
Showing 12 changed files with 333 additions and 19 deletions.
21 changes: 21 additions & 0 deletions core/src/action-items/ActionItemExporter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { DataAccessLayer } from "../data/dal.js";
import { ActionItem } from "../data/threats/ActionItem.js";

export interface ExportResult {
Key: string;

ThreatId: string;
/**
* The URL to the linked issue.
*/
LinkedURL?: string;
}

export interface ActionItemExporter {
key: string;

onReviewApproved(
dal: DataAccessLayer,
actionItems: ActionItem[]
): Promise<ExportResult[]>;
}
141 changes: 141 additions & 0 deletions core/src/action-items/ActionItemHandler.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,141 @@
import { randomUUID } from "crypto";
import { DataAccessLayer } from "../data/dal.js";
import Model from "../data/models/Model.js";
import { createPostgresPool } from "../data/postgres.js";
import { Review } from "../data/reviews/Review.js";
import Threat, { ThreatSeverity } from "../data/threats/Threat.js";
import { _deleteAllTheThings } from "../data/utils.js";
import { ActionItemHandler } from "./ActionItemHandler.js";
import { DummyActionItemExporter } from "./DummyActionItemExporter.js";
import { ActionItem } from "../data/threats/ActionItem.js";
import { ExportResult } from "./ActionItemExporter.js";

class OtherDummyActionItemExporter extends DummyActionItemExporter {
key = "other-dummy";
url = "other";

setUrl(url: string) {
this.url = url;
}

async onReviewApproved(
dal: DataAccessLayer,
actionItems: ActionItem[]
): Promise<ExportResult[]> {
return actionItems.map((actionItem) => ({
Key: this.key,
ThreatId: actionItem.threat.id!,
LinkedURL: this.url,
}));
}
}

describe("ActionItemHandler implementation", () => {
let dal: DataAccessLayer;
let model: Model;
let threatId: string;
let review: Review;

beforeAll(async () => {
const pool = await createPostgresPool();
dal = new DataAccessLayer(pool);
});

beforeEach(async () => {
await _deleteAllTheThings(dal);
model = new Model("some-system-id", "some-version", "root");
model.data = { components: [], dataFlows: [] };
model.id = await dal.modelService.create(model);
threatId = await dal.threatService.create(
new Threat("title", "desc", model.id!, randomUUID(), "root")
);
await dal.threatService.update(model.id, threatId, {
severity: ThreatSeverity.High,
isActionItem: true,
});
review = new Review(model.id!, "root");
await dal.reviewService.create(review);
});

it("should handle no exporters without issue", async () => {
const handler = new ActionItemHandler(dal);
await handler.onReviewApproved(review);
});

it("should create an action item export", async () => {
const handler = new ActionItemHandler(dal);
handler.attachExporter(new DummyActionItemExporter());
await handler.onReviewApproved(review);
const actionItems = await dal.threatService.listActionItems(model.id!);
expect(actionItems).toHaveLength(1);
expect(actionItems[0].threat.id).toEqual(threatId);
expect(actionItems[0].threat.severity).toEqual(ThreatSeverity.High);
expect(actionItems[0].exports).toHaveLength(1);
expect(actionItems[0].exports[0].exporterKey).toBe("dummy");
});

it("should replace existing action item exports", async () => {
const handler = new ActionItemHandler(dal);
handler.attachExporter(new DummyActionItemExporter());
await handler.onReviewApproved(review);
const actionItems = await dal.threatService.listActionItems(model.id!);
expect(actionItems).toHaveLength(1);
expect(actionItems[0].threat.id).toEqual(threatId);
expect(actionItems[0].threat.severity).toEqual(ThreatSeverity.High);
expect(actionItems[0].exports).toHaveLength(1);
expect(actionItems[0].exports[0].exporterKey).toBe("dummy");

// trigger again
await handler.onReviewApproved(review);
expect(actionItems).toHaveLength(1);
expect(actionItems[0].threat.id).toEqual(threatId);
expect(actionItems[0].threat.severity).toEqual(ThreatSeverity.High);
expect(actionItems[0].exports).toHaveLength(1);
expect(actionItems[0].exports[0].exporterKey).toBe("dummy");
});

it("should handle multiple exporters", async () => {
const handler = new ActionItemHandler(dal);
handler.attachExporter(new DummyActionItemExporter());
handler.attachExporter(new OtherDummyActionItemExporter());
await handler.onReviewApproved(review);
const actionItems = await dal.threatService.listActionItems(model.id!);
expect(actionItems).toHaveLength(1);
expect(actionItems[0].threat.id).toEqual(threatId);
expect(actionItems[0].threat.severity).toEqual(ThreatSeverity.High);
expect(actionItems[0].exports).toHaveLength(2);
expect(actionItems[0].exports[0].exporterKey).toBe("dummy");
expect(actionItems[0].exports[1].exporterKey).toBe("other-dummy");
});

it("should handle multiple exporters + changed url", async () => {
const handler = new ActionItemHandler(dal);
handler.attachExporter(new DummyActionItemExporter());
const other = new OtherDummyActionItemExporter();
handler.attachExporter(other);
await handler.onReviewApproved(review);
const actionItems = await dal.threatService.listActionItems(model.id!);
expect(actionItems).toHaveLength(1);
expect(actionItems[0].threat.id).toEqual(threatId);
expect(actionItems[0].threat.severity).toEqual(ThreatSeverity.High);
expect(actionItems[0].exports).toHaveLength(2);
expect(actionItems[0].exports[0].exporterKey).toBe("dummy");
expect(actionItems[0].exports[1].exporterKey).toBe("other-dummy");

// change the url
other.setUrl("changed");
await handler.onReviewApproved(review);
const actionItems2 = await dal.threatService.listActionItems(model.id!);
expect(actionItems2).toHaveLength(1);
expect(actionItems2[0].threat.id).toEqual(threatId);
expect(actionItems2[0].threat.severity).toEqual(ThreatSeverity.High);
expect(actionItems2[0].exports).toHaveLength(2);
expect(actionItems2[0].exports[0].exporterKey).toBe("dummy");
expect(actionItems2[0].exports[1].exporterKey).toBe("other-dummy");
expect(actionItems2[0].exports[1].linkedURL).toBe("changed");
});

afterAll(async () => {
await dal.pool.end();
});
});
52 changes: 52 additions & 0 deletions core/src/action-items/ActionItemHandler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
import log4js from "log4js";
import { DataAccessLayer } from "../data/dal.js";
import { Review } from "../data/reviews/Review.js";
import { ActionItemExport } from "../data/threats/ActionItem.js";
import { ActionItemExporter, ExportResult } from "./ActionItemExporter.js";

const log = log4js.getLogger("ActionItemHandler");

export class ActionItemHandler {
private exporters: ActionItemExporter[] = [];

constructor(private dal: DataAccessLayer) {
this.dal.reviewService.on("approved", ({ review }) => {
this.onReviewApproved(review);
});
}

attachExporter(exporter: ActionItemExporter) {
this.exporters.push(exporter);
log.info("Attached action item exporter ", exporter.key);
}

async onReviewApproved(review: Review) {
if (this.exporters.length === 0) {
log.debug("No exporters for action items. Skipping export");
}

return this.exportForModel(review.modelId);
}

// TODO: route to trigger this.
async exportForModel(modelId: string) {
const actionItems = await this.dal.threatService.listActionItems(modelId);
log.info(`Found ${actionItems.length} action items to export`);

const results: ExportResult[] = [];
for (const exporter of this.exporters) {
const result = await exporter.onReviewApproved(this.dal, actionItems);
results.push(...result);
}

for (const result of results) {
await this.dal.threatService.insertActionItemExport(
new ActionItemExport(result.Key, result.ThreatId, result.LinkedURL!)
);
// What if exporters are removed (1) or the link is removed, i.e. not returned by the exporter (2)?
// 1. Could be handled manually in the database (query for exporter_key)
// 2. May need extra funcionality to remove the link. For now I'll leave it as is.
}
log.info(`Exported ${results.length} action items`);
}
}
18 changes: 18 additions & 0 deletions core/src/action-items/DummyActionItemExporter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { DataAccessLayer } from "../data/dal.js";
import { ActionItem } from "../data/threats/ActionItem.js";
import { ActionItemExporter, ExportResult } from "./ActionItemExporter.js";

export class DummyActionItemExporter implements ActionItemExporter {
key: string = "dummy";

async onReviewApproved(
dal: DataAccessLayer,
actionItems: ActionItem[]
): Promise<ExportResult[]> {
return actionItems.map((actionItem) => ({
Key: this.key,
ThreatId: actionItem.threat.id!,
LinkedURL: `dummy:${actionItem.threat.id!}`,
}));
}
}
3 changes: 3 additions & 0 deletions core/src/bootstrap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@ export async function bootstrap(): Promise<DataAccessLayer> {
bt.setReviewerProvider(providers.reviewerProvider);
bt.setUserProvider(providers.userProvider);
bt.setSystemProvider(providers.systemProvider);
providers.actionItemExporters?.forEach((exporter) =>
dal.actionItemHandler.attachExporter(exporter)
);

bt.registerComponentClasses(providers.componentClasses || []);
bt.registerNotificationTemplates(providers.notificationTemplates || []);
Expand Down
2 changes: 2 additions & 0 deletions core/src/config/GramConfiguration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type { AssetFolder } from "./AssetFolder.js";
import type { Secret } from "./Secret.js";
import type { Migration } from "../data/Migration.js";
import type { TeamProvider } from "../auth/TeamProvider.js";
import type { ActionItemExporter } from "../action-items/ActionItemExporter.js";

export interface Providers {
/**
Expand All @@ -32,6 +33,7 @@ export interface Providers {
notificationTemplates?: NotificationTemplate[];
suggestionSources?: SuggestionSource[];
teamProvider?: TeamProvider;
actionItemExporters?: ActionItemExporter[];
}

export interface GramConfiguration {
Expand Down
5 changes: 5 additions & 0 deletions core/src/data/dal.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
createPostgresPool,
getDatabaseName,
} from "./postgres.js";
import { ActionItemHandler } from "../action-items/ActionItemHandler.js";

/**
* Class that carries access to all DataServices, useful for passing dependencies.
Expand Down Expand Up @@ -51,6 +52,7 @@ export class DataAccessLayer {
userHandler: UserHandler;
reviewerHandler: ReviewerHandler;
teamHandler: TeamHandler;
actionItemHandler: ActionItemHandler;

get authzProvider(): AuthzProvider {
return authzProvider;
Expand Down Expand Up @@ -85,5 +87,8 @@ export class DataAccessLayer {
this.suggestionEngine = new SuggestionEngine(this);
this.reportService = new ReportDataService(this);
this.bannerService = new BannerDataService(this);

// Initialize Action Item Handler. Needs to happen after Data Services are initialized.
this.actionItemHandler = new ActionItemHandler(this);
}
}
8 changes: 8 additions & 0 deletions core/src/data/migrations/25_exported_action_items.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
CREATE TABLE if not exists exported_action_items (
exporter_key varchar(255) NOT NULL,
threat_id uuid REFERENCES threats(id) ON DELETE CASCADE,
url text NOT NULL,
created_at TIMESTAMP WITH TIME ZONE DEFAULT current_timestamp,
updated_at TIMESTAMP WITH TIME ZONE DEFAULT current_timestamp,
PRIMARY KEY (exporter_key, threat_id)
);
28 changes: 28 additions & 0 deletions core/src/data/threats/ActionItem.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import Threat from "./Threat.js";

export class ActionItemExport {
constructor(
public exporterKey: string,
public threatId: string,
public linkedURL: string
) {}

toJSON() {
return {
exporterKey: this.exporterKey,
threatId: this.threatId,
linkedURL: this.linkedURL,
};
}
}

export class ActionItem {
constructor(public threat: Threat, public exports: ActionItemExport[]) {}

toJSON() {
return {
threat: this.threat.toJSON(),
exports: this.exports.map((e) => e.toJSON()),
};
}
}
2 changes: 1 addition & 1 deletion core/src/data/threats/ThreatDataService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,7 @@ describe("ThreatDataService implementation", () => {
expect(threats.length).toEqual(1);

expect(threats[0].toJSON()).toBeDefined();
expect(threats[0].id).toBe(threat2.id);
expect(threats[0].threat.id).toBe(threat2.id);
});

it("should return null value by default", async () => {
Expand Down
Loading

0 comments on commit 1f87897

Please sign in to comment.