Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(awards): add slack notifications #31

Merged
merged 25 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions packages/backend/src/plugins/awards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,5 +9,8 @@ export default async function createPlugin(
logger: env.logger,
database: env.database,
identity: env.identity,
config: env.config,
discovery: env.discovery,
tokenManager: env.tokenManager,
});
}
17 changes: 17 additions & 0 deletions plugins/awards-backend/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,23 @@ function makeCreateEnv(config: Config) {
}
```

## Configuration

### Slack notifications

To enable Slack notifications, add the following to your `app-config.yaml` file:

```yaml
awards:
notifications:
slack:
webhook:
# https://api.slack.com/messaging/webhooks
url: <my_slack_webhook_url>
Copy link
Contributor

@bbckr bbckr Feb 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't the webhook url contain a secret? i think in the documentation here we should advise that it be included as an env var and referenced as such in the config (from what I understand about dynamic includes in backstage)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done here 8d2d2b2

```

Users who have the `slack.com/user_id` annotation set (see [slack-catalog-backend](/plugins/slack-catalog-backend/README.md)) will be tagged in notifications that pertain to them.

## Developing this plugin

The plugin can be executed in isolation during development by running
Expand Down
3 changes: 3 additions & 0 deletions plugins/awards-backend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,13 @@
"dependencies": {
"@backstage/backend-common": "^0.20.1",
"@backstage/backend-plugin-api": "^0.6.9",
"@backstage/catalog-client": "^1.6.0",
"@backstage/catalog-model": "^1.4.4",
"@backstage/config": "^1.1.1",
"@backstage/errors": "^1.2.3",
"@backstage/plugin-auth-node": "^0.4.3",
"@seatgeek/backstage-plugin-awards-common": "link:*",
"@slack/webhook": "^7.0.2",
"@types/express": "*",
"express": "^4.17.1",
"express-promise-router": "^4.1.0",
Expand Down
158 changes: 158 additions & 0 deletions plugins/awards-backend/src/awards.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
/*
* Copyright SeatGeek
* Licensed under the terms of the Apache-2.0 license. See LICENSE file in project root for terms.
*/
import { TokenManager } from '@backstage/backend-common';
import {
CatalogClient,
CatalogRequestOptions,
GetEntitiesByRefsRequest,
} from '@backstage/catalog-client';
import { UserEntity } from '@backstage/catalog-model';
import { Award } from '@seatgeek/backstage-plugin-awards-common';
import * as winston from 'winston';
import { Awards } from './awards';
import { AwardsStore } from './database/awards';
import { NotificationsGateway } from './notifications/notifications';

const frank = 'user:default/frank-ocean';

function makeAward(): Award {
return {
uid: '123456',
name: 'Test Award',
description: 'This is a test award',
image: 'image_data',
owners: [frank],
recipients: ['user:default/peyton-manning', 'user:default/serena-williams'],
};
}

function makeUser(ref: string): UserEntity {
return {
apiVersion: 'backstage.io/v1alpha1',
kind: 'User',
metadata: {
name: ref,
},
spec: {},
};
}

describe('Awards', () => {
let db: jest.Mocked<AwardsStore>;
let notifications: jest.Mocked<NotificationsGateway>;
let catalogClient: jest.Mocked<CatalogClient>;
let tokenManager: jest.Mocked<TokenManager>;
let awards: Awards;

beforeEach(() => {
db = {
search: jest.fn(),
add: jest.fn(),
update: jest.fn(),
delete: jest.fn(),
};
notifications = {
notifyNewRecipientsAdded: jest.fn(),
};
tokenManager = {
authenticate: jest.fn(),
getToken: jest.fn().mockReturnValue({ token: 'mocked-token' }),
};
catalogClient = {
getEntitiesByRefs: jest
.fn()
.mockImplementation(
async (
request: GetEntitiesByRefsRequest,
_?: CatalogRequestOptions,
) => {
return {
items: request.entityRefs.map(makeUser),
};
},
),
} as unknown as jest.Mocked<CatalogClient>;
const logger = winston.createLogger({
transports: [new winston.transports.Console({ silent: true })],
});
awards = new Awards(db, notifications, catalogClient, tokenManager, logger);
});

afterEach(() => {
jest.resetAllMocks();
});

describe('create', () => {
it('should notify new recipients', async () => {
const award = makeAward();
db.add = jest.fn().mockResolvedValue(award);
const result = await awards.create(frank, {
name: award.name,
description: award.description,
image: award.image,
owners: award.owners,
recipients: award.recipients,
});

// wait for the afterCreate promises to complete
await new Promise(process.nextTick);

expect(result).toEqual(award);
expect(db.add).toHaveBeenCalledWith(
award.name,
award.description,
award.image,
award.owners,
award.recipients,
);
expect(notifications.notifyNewRecipientsAdded).toHaveBeenCalledWith(
frank,
award,
[
makeUser('user:default/peyton-manning'),
makeUser('user:default/serena-williams'),
],
);
});
});

describe('update', () => {
it('should notify new recipients', async () => {
const award = makeAward();
db.search = jest.fn().mockResolvedValue([award]);
const updated = {
...award,
recipients: [
...award.recipients,
'user:default/megan-rapinoe',
'user:default/adrianne-lenker',
],
};
db.update = jest.fn().mockResolvedValue(updated);
const result = await awards.update(frank, award.uid, updated);

// wait for the afterUpdate promises to complete
await new Promise(process.nextTick);

expect(result).toEqual(updated);
expect(db.update).toHaveBeenCalledWith(
updated.uid,
updated.name,
updated.description,
updated.image,
updated.owners,
updated.recipients,
);
expect(notifications.notifyNewRecipientsAdded).toHaveBeenCalledWith(
frank,
updated,
[
makeUser('user:default/megan-rapinoe'),
makeUser('user:default/adrianne-lenker'),
],
);
});
});
});
76 changes: 73 additions & 3 deletions plugins/awards-backend/src/awards.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,33 +2,99 @@
* Copyright SeatGeek
* Licensed under the terms of the Apache-2.0 license. See LICENSE file in project root for terms.
*/
import { TokenManager } from '@backstage/backend-common';
import { CatalogClient } from '@backstage/catalog-client';
import { isUserEntity } from '@backstage/catalog-model';
import { NotFoundError } from '@backstage/errors';
import { Award, AwardInput } from '@seatgeek/backstage-plugin-awards-common';
import { Logger } from 'winston';
import { AwardsStore } from './database/awards';
import { NotificationsGateway } from './notifications/notifications';

function nonNullable<T>(value: T): value is NonNullable<T> {
return value !== null && value !== undefined;
}

export class Awards {
private readonly db: AwardsStore;
private readonly logger: Logger;
private readonly notifications: NotificationsGateway;
private readonly catalogClient: CatalogClient;
private readonly tokenManager: TokenManager;

constructor(db: AwardsStore, logger: Logger) {
constructor(
db: AwardsStore,
notifications: NotificationsGateway,
catalogClient: CatalogClient,
tokenManager: TokenManager,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three new dependencies are only used by the notifyNewRecipients function, but now the entire class is untestable unless these are all injected/mocked. Should we decouple this with one additional abstraction layer (perhaps called AwardsNotifier or something) between the awards and the gateway so we can inject a single AwardNotifier and keep that logic isolated from awards management? WDYT?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yep i think that makes sense. AwardsNotifier could also have some smarter logic that i kept out from bloating the Awards class like fanning out to multiple notification gateways and skipping catalog fetching if there are no notification gateways (rather than the nullnotificationgateway)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done in fb4f4f0.. how's that look?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it!

like fanning out to multiple notification gateways and skipping catalog fetching if there are no notification gateways (rather than the nullnotificationgateway)

I often like to use this design pattern for those types of things (I don't know what it's called):

export class MultipleGateways implements NotificationsGateway {
  constructor(private readonly gateways: NotificationsGateway[]) {}

  async notifyNewRecipientsAdded(award: Award, newRecipients: UserEntity[]): Promise<void> {
    this.gateways.forEach((gateway) => {
      gateway.notifyNewRecipientsAdded(award, newRecipients);
    });
  }
}

But because you're also skipping the catalog fetching when no gateways are added I think it totally makes sense to implement AwardsNotifier exactly how you did - nicely done!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lol yeah i love that. i think it's the composite pattern?

logger: Logger,
) {
this.db = db;
this.notifications = notifications;
this.logger = logger.child({ class: 'Awards' });
this.catalogClient = catalogClient;
this.tokenManager = tokenManager;
this.logger.debug('Constructed');
}

async get(uid: string): Promise<Award> {
return await this.getAwardByUid(uid);
}

async create(input: AwardInput): Promise<Award> {
return await this.db.add(
private async notifyNewRecipients(
identityRef: string,
zhammer marked this conversation as resolved.
Show resolved Hide resolved
award: Award,
newRecipients: string[],
): Promise<void> {
const token = await this.tokenManager.getToken();
const resp = await this.catalogClient.getEntitiesByRefs(
{
entityRefs: newRecipients,
},
token,
);
const users = resp.items.filter(nonNullable).filter(isUserEntity);
await this.notifications.notifyNewRecipientsAdded(
identityRef,
award,
users,
);
}

private async afterCreate(identityRef: string, award: Award): Promise<void> {
if (award.recipients.length > 0) {
await this.notifyNewRecipients(identityRef, award, award.recipients);
}
}

async create(identityRef: string, input: AwardInput): Promise<Award> {
const award = await this.db.add(
input.name,
input.description,
input.image,
input.owners,
input.recipients,
);

this.afterCreate(identityRef, award).catch(e => {
this.logger.error('Error running afterCreate action', e);
});

return award;
}

private async afterUpdate(
identityRef: string,
curr: Award,
previous: Award,
): Promise<void> {
const newRecipients = curr.recipients.filter(
recipient => !previous.recipients.includes(recipient),
);

if (newRecipients.length > 0) {
await this.notifyNewRecipients(identityRef, curr, newRecipients);
}
}

async update(
Expand All @@ -51,6 +117,10 @@ export class Awards {
input.recipients,
);

this.afterUpdate(identityRef, updated, award).catch(e => {
this.logger.error('Error running afterUpdate action', e);
});

return updated;
}

Expand Down
Loading
Loading