From d64926020ffe3dbfc0bd2f99944f66bae90a6120 Mon Sep 17 00:00:00 2001 From: greg-adams Date: Mon, 14 Oct 2024 11:15:54 -0700 Subject: [PATCH 1/7] add digest queries --- packages/server/__tests__/db/db.test.js | 6 +- .../server/__tests__/db/seeds/fixtures.js | 8 + .../lib/grants-collaboration.test.js | 163 ++++++++++++++++++ .../lib/grantsCollaboration/grantActivity.js | 124 +++++++++++++ 4 files changed, 298 insertions(+), 3 deletions(-) create mode 100644 packages/server/src/lib/grantsCollaboration/grantActivity.js diff --git a/packages/server/__tests__/db/db.test.js b/packages/server/__tests__/db/db.test.js index fedb7b15b..54c38e324 100644 --- a/packages/server/__tests__/db/db.test.js +++ b/packages/server/__tests__/db/db.test.js @@ -44,7 +44,7 @@ describe('db', () => { 'DRY RUN :: Migrating agency criteria for agency 4', 'DRY RUN :: Migrating agency criteria for users 1,2 belonging to agency 0', 'DRY RUN :: No agency criteria to migrate for users 3 belonging to agency 1', - 'DRY RUN :: No agency criteria to migrate for users 4 belonging to agency 2', + 'DRY RUN :: No agency criteria to migrate for users 4,5 belonging to agency 2', 'DRY RUN :: No users to migrate for agency 4', 'DRY RUN :: Would have inserted approximately 2 saved searches. Note: there may be duplicates.', 'DRY RUN :: Done migrating legacy agency criteria to saved searches', @@ -70,7 +70,7 @@ describe('db', () => { 'Migrating agency criteria for agency 4', 'Migrating agency criteria for users 1,2 belonging to agency 0', 'No agency criteria to migrate for users 3 belonging to agency 1', - 'No agency criteria to migrate for users 4 belonging to agency 2', + 'No agency criteria to migrate for users 4,5 belonging to agency 2', 'No users to migrate for agency 4', 'Inserted 2 saved searches', 'Done migrating legacy agency criteria to saved searches', @@ -117,7 +117,7 @@ describe('db', () => { 'Migrating agency criteria for agency 4', 'Migrating agency criteria for users 1,2 belonging to agency 0', 'No agency criteria to migrate for users 3 belonging to agency 1', - 'No agency criteria to migrate for users 4 belonging to agency 2', + 'No agency criteria to migrate for users 4,5 belonging to agency 2', 'No users to migrate for agency 4', 'Inserted 1 saved searches', // This would have been 2 if not for the duplication mechanism. 'Done migrating legacy agency criteria to saved searches', diff --git a/packages/server/__tests__/db/seeds/fixtures.js b/packages/server/__tests__/db/seeds/fixtures.js index ed2a65fee..d1fbe9e14 100644 --- a/packages/server/__tests__/db/seeds/fixtures.js +++ b/packages/server/__tests__/db/seeds/fixtures.js @@ -89,6 +89,14 @@ const users = { id: 4, tenant_id: agencies.usdr.tenant_id, }, + usdrAdmin: { + email: 'usdr.admin@test.com', + name: 'USDR admin', + agency_id: agencies.usdr.id, + role_id: roles.adminRole.id, + id: 5, + tenant_id: agencies.usdr.tenant_id, + }, }; const keywords = { diff --git a/packages/server/__tests__/lib/grants-collaboration.test.js b/packages/server/__tests__/lib/grants-collaboration.test.js index 53435e81c..1ff603e51 100644 --- a/packages/server/__tests__/lib/grants-collaboration.test.js +++ b/packages/server/__tests__/lib/grants-collaboration.test.js @@ -1,12 +1,16 @@ const { expect, use } = require('chai'); const chaiAsPromised = require('chai-as-promised'); const { DateTime } = require('luxon'); +const _ = require('lodash'); const knex = require('../../src/db/connection'); const fixtures = require('../db/seeds/fixtures'); const { saveNoteRevision, getOrganizationNotesForGrant, getOrganizationNotesForGrantByUser } = require('../../src/lib/grantsCollaboration/notes'); const { followGrant, unfollowGrant, getFollowerForGrant, getFollowersForGrant, } = require('../../src/lib/grantsCollaboration/followers'); +const { + getGrantActivity, getGrantActivityEmailRecipients, +} = require('../../src/lib/grantsCollaboration/grantActivity'); use(chaiAsPromised); @@ -243,4 +247,163 @@ describe('Grants Collaboration', () => { expect(result.pagination.next).to.equal(follower2.id); }); }); + + context('Grant Activity', () => { + const { adminUser, staffUser } = fixtures.users; + const grant1 = fixtures.grants.earFellowship; + const grant2 = fixtures.grants.healthAide; + + let periodStart; + let periodEnd; + + let grant1NoteAdmin; + let grant1NoteStaff; + + beforeEach(async () => { + periodStart = DateTime.now().minus({ days: 1 }).toJSDate(); + + // Grant 1 Follows + await knex('grant_followers') + .insert({ grant_id: grant1.grant_id, user_id: adminUser.id }, 'id'); + + await knex('grant_followers') + .insert({ grant_id: grant1.grant_id, user_id: staffUser.id }, 'id'); + + // Grant 1 Notes + [grant1NoteAdmin] = await knex('grant_notes') + .insert({ grant_id: grant1.grant_id, user_id: adminUser.id }, 'id'); + + await knex('grant_notes_revisions') + .insert({ grant_note_id: grant1NoteAdmin.id, text: 'Admin note' }, 'id'); + + [grant1NoteStaff] = await knex('grant_notes') + .insert({ grant_id: grant1.grant_id, user_id: staffUser.id }, 'id'); + + await knex('grant_notes_revisions') + .insert({ grant_note_id: grant1NoteStaff.id, text: 'Staff note' }, 'id'); + + // Grant 2 Follows + await knex('grant_followers') + .insert({ grant_id: grant2.grant_id, user_id: staffUser.id }, 'id'); + + // Grant 2 Notes + const [grant2NoteStaff] = await knex('grant_notes') + .insert({ grant_id: grant2.grant_id, user_id: staffUser.id }, 'id'); + + await knex('grant_notes_revisions') + .insert({ grant_note_id: grant2NoteStaff.id, text: 'Staff note' }, 'id'); + + periodEnd = new Date(); + }); + + it('retrieves all email recipients for note/follow activity by period', async () => { + const recipients = await getGrantActivityEmailRecipients(knex, periodStart, periodEnd); + + expect(recipients).to.have.length(2); + + expect(recipients).to.have.members([ + adminUser.id, + staffUser.id, + ]); + }); + + it('retrieves all note/follow activity by period', async () => { + const grants = await getGrantActivity(knex, adminUser.id, periodStart, periodEnd); + + expect(grants).to.have.lengthOf(1); + expect(grants[0].grantId).to.equal(grant1.grant_id); + expect(grants[0].notes).to.have.lengthOf(2); + expect(grants[0].follows).to.have.lengthOf(2); + + // Sorted oldest first + expect(grants[0].notes[0].noteText).to.equal('Admin note'); + expect(grants[0].follows[0].userId).to.equal(adminUser.id); + }); + + it('retrieves email recipients only if OTHER users took action', async () => { + periodStart = new Date(); + + // Admin edits note + await knex('grant_notes_revisions') + .insert({ grant_note_id: grant1NoteAdmin.id, text: 'Edit for Admin note' }, 'id'); + + const recipients1 = await getGrantActivityEmailRecipients(knex, periodStart, new Date()); + expect(recipients1).to.have.members([staffUser.id]); + + // Staff edits note + await knex('grant_notes_revisions') + .insert({ grant_note_id: grant1NoteStaff.id, text: 'Edit for Staff note' }, 'id'); + + const recipients2 = await getGrantActivityEmailRecipients(knex, periodStart, new Date()); + expect(recipients2).to.have.members([staffUser.id, adminUser.id]); + }); + + it('retrieves no note/follow activity when window is outside time period of activity', async () => { + periodStart = DateTime.fromJSDate(periodStart).minus({ days: 1 }).toJSDate(); + periodEnd = DateTime.fromJSDate(periodEnd).minus({ days: 1 }).toJSDate(); + + const recipients = await getGrantActivityEmailRecipients(knex, periodStart, periodEnd); + expect(recipients).to.have.lengthOf(0); + + await Promise.all( + [adminUser.id, staffUser.id].map(async (userId) => { + const grants = await getGrantActivity(knex, userId, periodStart, periodEnd); + expect(grants).to.have.lengthOf(0); + }), + ); + }); + + it('Grant activity results should ignore activity by other org/tenants', async () => { + const { usdrUser: otherUser1, usdrAdmin: otherUser2 } = fixtures.users; + + await knex('grant_followers') + .insert([ + { grant_id: grant1.grant_id, user_id: otherUser1.id }, + { grant_id: grant1.grant_id, user_id: otherUser2.id }, + ], 'id'); + + const [otherUser1Note, otherUser2Note] = await knex('grant_notes') + .insert([ + { grant_id: grant1.grant_id, user_id: otherUser1.id }, + { grant_id: grant1.grant_id, user_id: otherUser2.id }, + ], 'id'); + + await knex('grant_notes_revisions') + .insert([ + { grant_note_id: otherUser1Note.id, text: 'Other tenant note1' }, + { grant_note_id: otherUser2Note.id, text: 'Other tenant note2' }, + ], 'id'); + periodEnd = new Date(); + + // Email recipients INCLUDES multiple tenants + expect(await getGrantActivityEmailRecipients(knex, periodStart, periodEnd)).to.have.members([ + adminUser.id, + staffUser.id, + otherUser1.id, + otherUser2.id, + ]); + + const getActivityUserIds = (grants) => grants.reduce((userIds, grant) => { + grant.notes.forEach((act) => userIds.push(act.userId)); + grant.follows.forEach((act) => userIds.push(act.userId)); + return userIds; + }, []); + + const tenantOneUsers = [adminUser.id, staffUser.id]; + const tenantTwoUsers = [otherUser1.id, otherUser2.id]; + + // Digest activity does NOT include cross-over between tenants + const tenantOneGrants = await Promise.all( + tenantOneUsers.map((userId) => getGrantActivity(knex, userId, periodStart, periodEnd)), + ); + const tenantOneUserIds = getActivityUserIds(tenantOneGrants.flat()); + expect(tenantOneUserIds).not.to.include.members(tenantTwoUsers); + + const tenantTwoGrants = await Promise.all( + tenantTwoUsers.map((userId) => getGrantActivity(knex, userId, periodStart, periodEnd)), + ); + const tenantTwoUserIds = getActivityUserIds(tenantTwoGrants.flat()); + expect(tenantTwoUserIds).not.to.include.members(tenantOneUsers); + }); + }); }); diff --git a/packages/server/src/lib/grantsCollaboration/grantActivity.js b/packages/server/src/lib/grantsCollaboration/grantActivity.js new file mode 100644 index 000000000..9566710a2 --- /dev/null +++ b/packages/server/src/lib/grantsCollaboration/grantActivity.js @@ -0,0 +1,124 @@ +const _ = require('lodash'); + +const getActivitiesQuery = (knex) => { + const noteRevisionsSub = knex + .select() + .from({ r: 'grant_notes_revisions' }) + .whereRaw('r.grant_note_id = gn.id') + .orderBy('r.created_at', 'desc') + .limit(1); + + return knex.unionAll([ + knex + .select([ + 'gf.id', + 'gf.grant_id', + 'gf.user_id', + 'gf.created_at AS activity_at', + knex.raw(`'follow' AS activity_type`), + knex.raw('null AS text_content'), + ]) + .from({ gf: 'grant_followers' }), + knex + .select([ + 'rev.id', + 'gn.grant_id', + 'gn.user_id', + 'rev.created_at AS activity_at', + knex.raw(`'note' AS activity_type`), + 'rev.text AS text_content', + ]) + .from({ gn: 'grant_notes' }) + .joinRaw(`LEFT JOIN LATERAL (${noteRevisionsSub.toString()}) AS rev ON rev.grant_note_id = gn.id`), + ]) + .as('activity'); +}; + +async function getGrantActivityEmailRecipients(knex, periodStart, periodEnd) { + const query = knex + .distinct('recipient_followers.user_id AS recipient_user_id') + .from( + getActivitiesQuery(knex), + ) + .join({ recipient_followers: 'grant_followers' }, 'recipient_followers.grant_id', 'activity.grant_id') + .join({ activity_users: 'users' }, 'activity_users.id', 'activity.user_id') + .join({ recipient_users: 'users' }, 'recipient_users.id', 'recipient_followers.user_id') + .where('activity.activity_at', '>', periodStart) + .andWhere('activity.activity_at', '<', periodEnd) + // only consider actions taken by users in the same organization as the recipient: + .andWhereRaw('recipient_users.tenant_id = activity_users.tenant_id') + // exclude rows where the recipient user is the one taking the action, + // to ensure that users only receive a digest if OTHER users took action: + .andWhereRaw('recipient_followers.user_id != activity.user_id'); + + const results = await query; + + return _.map(results, 'recipient_user_id'); +} + +async function getGrantActivity(knex, userId, periodStart, periodEnd) { + const query = knex.select([ + 'g.grant_id AS grant_id', + 'g.title AS grant_title', + 'activity_users.id AS user_id', + 'activity_users.name AS user_name', + 'activity_users.email AS user_email', + 'activity_users_agencies.name AS agency_name', + 'activity.activity_at', + 'activity.activity_type', + 'activity.text_content AS note_text', + ]) + .from( + getActivitiesQuery(knex), + ) + .join({ recipient_followers: 'grant_followers' }, 'recipient_followers.grant_id', 'activity.grant_id') + // incorporate users table for users responsible for the activity: + .join({ activity_users: 'users' }, 'activity_users.id', 'activity.user_id') + // incorporate users table for the recipient follower: + .join({ recipient_users: 'users' }, 'recipient_users.id', 'recipient_followers.user_id') + // Additional JOINs for data selected for use in the email's content: + .join({ g: 'grants' }, 'g.grant_id', 'activity.grant_id') + .join({ activity_users_agencies: 'agencies' }, 'activity_users_agencies.id', 'activity_users.agency_id') + .where('activity.activity_at', '>', periodStart) + .andWhere('activity.activity_at', '<', periodEnd) + // Limit to activity where the user performing the activity belongs to the same organization: + .andWhereRaw('activity_users.tenant_id = recipient_users.tenant_id') + // limit activity to grants for which the recipient user is a follower + .andWhere('recipient_followers.user_id', userId) + .orderBy([ + { column: 'g.grant_id', order: 'desc' }, + { column: 'activity.activity_at', order: 'asc' }, + ]); + + const results = await query; + + // Group by grant + const resultsByGrant = _.groupBy(results, 'grant_id'); + + // Grant IDs distinct + const grantIds = [...new Set(_.map(results, 'grant_id'))]; + + return grantIds.map((grantId) => { + const activities = resultsByGrant[grantId].map((act) => ({ + userId: act.user_id, + userName: act.user_name, + agencyName: act.agency_name, + activityAt: act.activity_at, + activityType: act.activity_type, + noteText: act.note_text, + })); + const activitiesByType = _.groupBy(activities, 'activityType'); + + return { + grantId, + grantTitle: resultsByGrant[grantId][0].grant_title, + notes: activitiesByType.note || [], + follows: activitiesByType.follow || [], + }; + }); +} + +module.exports = { + getGrantActivity, + getGrantActivityEmailRecipients, +}; From 3534522fa71d505568920596436de2104bab633e Mon Sep 17 00:00:00 2001 From: greg-adams Date: Tue, 15 Oct 2024 10:32:12 -0700 Subject: [PATCH 2/7] add email trigger --- packages/server/__tests__/email/email.test.js | 86 ++++++++++++++++- .../lib/grants-collaboration.test.js | 67 ++++++++----- .../sendGrantActivityDigestEmail.test.js | 30 ++++++ packages/server/src/lib/email.js | 94 +++++++++++++++++++ packages/server/src/lib/email/constants.js | 1 + .../lib/grantsCollaboration/grantActivity.js | 47 ++++++---- .../scripts/sendGrantActivityDigestEmail.js | 27 ++++++ terraform/main.tf | 17 ++-- .../modules/gost_api/grant_activity_digest.tf | 46 +++++++++ terraform/modules/gost_api/task.tf | 25 ++--- terraform/modules/gost_api/variables.tf | 6 ++ terraform/prod.tfvars | 17 ++-- terraform/sandbox.tfvars | 19 ++-- terraform/staging.tfvars | 17 ++-- terraform/variables.tf | 4 + 15 files changed, 413 insertions(+), 90 deletions(-) create mode 100644 packages/server/__tests__/scripts/sendGrantActivityDigestEmail.test.js create mode 100644 packages/server/src/scripts/sendGrantActivityDigestEmail.js create mode 100644 terraform/modules/gost_api/grant_activity_digest.tf diff --git a/packages/server/__tests__/email/email.test.js b/packages/server/__tests__/email/email.test.js index ff1d96110..367743d76 100644 --- a/packages/server/__tests__/email/email.test.js +++ b/packages/server/__tests__/email/email.test.js @@ -2,6 +2,7 @@ const { expect } = require('chai'); const moment = require('moment'); +const { DateTime } = require('luxon'); const sinon = require('sinon'); const _ = require('lodash'); require('dotenv').config(); @@ -9,6 +10,9 @@ const emailService = require('../../src/lib/email/service-email'); const email = require('../../src/lib/email'); const fixtures = require('../db/seeds/fixtures'); const db = require('../../src/db'); +const { tags } = require('../../src/lib/email/constants'); + +const { knex } = db; const awsTransport = require('../../src/lib/gost-aws'); const { @@ -216,20 +220,21 @@ describe('Email module', () => { describe('Email sender', () => { const sandbox = sinon.createSandbox(); before(async () => { - await fixtures.seed(db.knex); process.env.DD_SERVICE = 'test-dd-service'; process.env.DD_ENV = 'test-dd-env'; process.env.DD_VERSION = 'test-dd-version'; + sandbox.spy(emailService); }); + after(async () => { - await db.knex.destroy(); + await knex.destroy(); process.env.DD_SERVICE = DD_SERVICE; process.env.DD_ENV = DD_ENV; process.env.DD_VERSION = DD_VERSION; }); - beforeEach(() => { - sandbox.spy(emailService); + beforeEach(async () => { + await fixtures.seed(knex); }); afterEach(() => { @@ -571,4 +576,77 @@ describe('Email sender', () => { expect(sendFake.calledOnce).to.equal(true); }); }); + + context('buildAndSendGrantActivityDigestEmails', () => { + const { adminUser, staffUser } = fixtures.users; + const grant1 = fixtures.grants.earFellowship; + + let periodStart; + let periodEnd; + + let sendFake; + + beforeEach(async () => { + sendFake = sinon.fake.returns('foo'); + sinon.replace(emailService, 'getTransport', sinon.fake.returns({ sendEmail: sendFake })); + + periodStart = new Date(); + + // Grant 1 Follows + await knex('grant_followers') + .insert([ + { grant_id: grant1.grant_id, user_id: adminUser.id }, + { grant_id: grant1.grant_id, user_id: staffUser.id }, + ], 'id'); + + // Grant 1 Notes + const [adminNote, staffNote] = await knex('grant_notes') + .insert([ + { grant_id: grant1.grant_id, user_id: adminUser.id }, + { grant_id: grant1.grant_id, user_id: staffUser.id }, + ], 'id'); + + await knex('grant_notes_revisions') + .insert([ + { grant_note_id: adminNote.id, text: 'Admin note' }, + { grant_note_id: staffNote.id, text: 'Staff note' }, + ], 'id'); + + periodEnd = new Date(); + }); + + it('Sends an email for all users with activity', async () => { + // Send digest email for users following grants + await email.buildAndSendGrantActivityDigestEmails(null, periodStart, periodEnd); + expect(sendFake.callCount).to.equal(2); + + const [firstEmail, secondEmail] = sendFake.getCalls(); + + expect([firstEmail.args[0].toAddress, secondEmail.args[0].toAddress]).to.be.members([adminUser.email, staffUser.email]); + }); + + it('Sends an email for a user\'s saved search', async () => { + // Build digest for user following grants + await email.buildAndSendGrantActivityDigestEmails(adminUser.id, periodStart, periodEnd); + expect(sendFake.calledOnce).to.equal(true); + + const { body, text, ...rest } = sendFake.firstCall.args[0]; + + expect(rest).to.deep.equal({ + fromName: 'USDR Federal Grant Finder', + ccAddress: undefined, + toAddress: adminUser.email, + subject: 'New activity in your grants', + tags: [ + `notification_type=${tags.emailTypes.grantActivityDigest}`, + 'user_role=admin', + `organization_id=${adminUser.tenant_id}`, + `team_id=${adminUser.agency_id}`, + 'service=test-dd-service', + 'env=test-dd-env', + 'version=test-dd-version', + ], + }); + }); + }); }); diff --git a/packages/server/__tests__/lib/grants-collaboration.test.js b/packages/server/__tests__/lib/grants-collaboration.test.js index 1ff603e51..e2d35e121 100644 --- a/packages/server/__tests__/lib/grants-collaboration.test.js +++ b/packages/server/__tests__/lib/grants-collaboration.test.js @@ -9,7 +9,7 @@ const { followGrant, unfollowGrant, getFollowerForGrant, getFollowersForGrant, } = require('../../src/lib/grantsCollaboration/followers'); const { - getGrantActivity, getGrantActivityEmailRecipients, + getGrantActivityByUserId, getGrantActivityEmailRecipients, } = require('../../src/lib/grantsCollaboration/grantActivity'); use(chaiAsPromised); @@ -249,6 +249,17 @@ describe('Grants Collaboration', () => { }); context('Grant Activity', () => { + // Helper functions + const getUserIdsForActivities = (grants) => { + const ids = grants.reduce((userIds, grant) => { + grant.notes.forEach((act) => userIds.add(act.userId)); + grant.follows.forEach((act) => userIds.add(act.userId)); + return userIds; + }, new Set()); + + return Array.from(ids); + }; + const { adminUser, staffUser } = fixtures.users; const grant1 = fixtures.grants.earFellowship; const grant2 = fixtures.grants.healthAide; @@ -286,6 +297,9 @@ describe('Grants Collaboration', () => { await knex('grant_followers') .insert({ grant_id: grant2.grant_id, user_id: staffUser.id }, 'id'); + await knex('grant_followers') + .insert({ grant_id: grant2.grant_id, user_id: adminUser.id }, 'id'); + // Grant 2 Notes const [grant2NoteStaff] = await knex('grant_notes') .insert({ grant_id: grant2.grant_id, user_id: staffUser.id }, 'id'); @@ -308,16 +322,16 @@ describe('Grants Collaboration', () => { }); it('retrieves all note/follow activity by period', async () => { - const grants = await getGrantActivity(knex, adminUser.id, periodStart, periodEnd); - - expect(grants).to.have.lengthOf(1); - expect(grants[0].grantId).to.equal(grant1.grant_id); - expect(grants[0].notes).to.have.lengthOf(2); - expect(grants[0].follows).to.have.lengthOf(2); - - // Sorted oldest first - expect(grants[0].notes[0].noteText).to.equal('Admin note'); - expect(grants[0].follows[0].userId).to.equal(adminUser.id); + const grantActivity = await getGrantActivityByUserId(knex, adminUser.id, periodStart, periodEnd); + + expect(grantActivity.grants).to.have.lengthOf(2); + expect(grantActivity.userEmail).to.equal(adminUser.email); + expect(grantActivity.userName).to.equal(adminUser.name); + expect(grantActivity.grants[0].grantId).to.equal(grant1.grant_id); + expect(grantActivity.grants[0].notes).to.have.lengthOf(1); + expect(grantActivity.grants[0].follows).to.have.lengthOf(1); + expect(grantActivity.grants[0].notes[0].noteText).to.equal('Staff note'); + expect(grantActivity.grants[0].follows[0].userId).to.equal(staffUser.id); }); it('retrieves email recipients only if OTHER users took action', async () => { @@ -338,6 +352,16 @@ describe('Grants Collaboration', () => { expect(recipients2).to.have.members([staffUser.id, adminUser.id]); }); + it('retrieves activity only for OTHER users action', async () => { + const adminGrantActivity = await getGrantActivityByUserId(knex, adminUser.id, periodStart, periodEnd); + const adminActivityIds = getUserIdsForActivities(adminGrantActivity.grants); + expect(adminActivityIds).not.to.include(adminUser.id); + + const staffGrantActivity = await getGrantActivityByUserId(knex, staffUser.id, periodStart, periodEnd); + const staffActivityIds = getUserIdsForActivities(staffGrantActivity.grants); + expect(staffActivityIds).not.to.include(staffUser.id); + }); + it('retrieves no note/follow activity when window is outside time period of activity', async () => { periodStart = DateTime.fromJSDate(periodStart).minus({ days: 1 }).toJSDate(); periodEnd = DateTime.fromJSDate(periodEnd).minus({ days: 1 }).toJSDate(); @@ -347,8 +371,8 @@ describe('Grants Collaboration', () => { await Promise.all( [adminUser.id, staffUser.id].map(async (userId) => { - const grants = await getGrantActivity(knex, userId, periodStart, periodEnd); - expect(grants).to.have.lengthOf(0); + const grantActivity = await getGrantActivityByUserId(knex, userId, periodStart, periodEnd); + expect(grantActivity.grants).to.have.lengthOf(0); }), ); }); @@ -383,26 +407,25 @@ describe('Grants Collaboration', () => { otherUser2.id, ]); - const getActivityUserIds = (grants) => grants.reduce((userIds, grant) => { - grant.notes.forEach((act) => userIds.push(act.userId)); - grant.follows.forEach((act) => userIds.push(act.userId)); - return userIds; - }, []); + const getGrantActivity = async (userId) => { + const grantActivity = await getGrantActivityByUserId(knex, userId, periodStart, periodEnd); + return grantActivity.grants; + }; const tenantOneUsers = [adminUser.id, staffUser.id]; const tenantTwoUsers = [otherUser1.id, otherUser2.id]; // Digest activity does NOT include cross-over between tenants const tenantOneGrants = await Promise.all( - tenantOneUsers.map((userId) => getGrantActivity(knex, userId, periodStart, periodEnd)), + tenantOneUsers.map((userId) => getGrantActivity(userId)), ); - const tenantOneUserIds = getActivityUserIds(tenantOneGrants.flat()); + const tenantOneUserIds = getUserIdsForActivities(tenantOneGrants.flat()); expect(tenantOneUserIds).not.to.include.members(tenantTwoUsers); const tenantTwoGrants = await Promise.all( - tenantTwoUsers.map((userId) => getGrantActivity(knex, userId, periodStart, periodEnd)), + tenantTwoUsers.map((userId) => getGrantActivity(userId)), ); - const tenantTwoUserIds = getActivityUserIds(tenantTwoGrants.flat()); + const tenantTwoUserIds = getUserIdsForActivities(tenantTwoGrants.flat()); expect(tenantTwoUserIds).not.to.include.members(tenantOneUsers); }); }); diff --git a/packages/server/__tests__/scripts/sendGrantActivityDigestEmail.test.js b/packages/server/__tests__/scripts/sendGrantActivityDigestEmail.test.js new file mode 100644 index 000000000..1a2c0e429 --- /dev/null +++ b/packages/server/__tests__/scripts/sendGrantActivityDigestEmail.test.js @@ -0,0 +1,30 @@ +const { expect } = require('chai'); +const sinon = require('sinon'); +const email = require('../../src/lib/email'); +const sendGrantActivityDigestEmail = require('../../src/scripts/sendGrantActivityDigestEmail').main; + +describe('sendGrantActivityDigestEmail script', () => { + const sandbox = sinon.createSandbox(); + let buildAndSendGrantActivityDigestEmailsFake; + + beforeEach(() => { + process.env.ENABLE_GRANT_ACTIVITY_DIGEST_SCHEDULED_TASK = 'true'; + buildAndSendGrantActivityDigestEmailsFake = sandbox.fake(); + sandbox.replace(email, 'buildAndSendGrantActivityDigestEmails', buildAndSendGrantActivityDigestEmailsFake); + }); + + afterEach(() => { + sandbox.restore(); + }); + + it('triggers sending digest emails when flags are on', async () => { + await sendGrantActivityDigestEmail(); + expect(buildAndSendGrantActivityDigestEmailsFake.called).to.equal(true); + }); + + it('triggers no emails when scheduled task flag is off', async () => { + process.env.ENABLE_GRANT_ACTIVITY_DIGEST_SCHEDULED_TASK = 'false'; + await sendGrantActivityDigestEmail(); + expect(buildAndSendGrantActivityDigestEmailsFake.called).to.equal(false); + }); +}); diff --git a/packages/server/src/lib/email.js b/packages/server/src/lib/email.js index 88331428c..7f5fbde16 100644 --- a/packages/server/src/lib/email.js +++ b/packages/server/src/lib/email.js @@ -2,6 +2,7 @@ const tracer = require('dd-trace').init(); const { URL } = require('url'); const moment = require('moment'); const { capitalize } = require('lodash'); +const { DateTime } = require('luxon'); // eslint-disable-next-line import/no-unresolved const asyncBatch = require('async-batch').default; const fileSystem = require('fs'); @@ -10,6 +11,9 @@ const mustache = require('mustache'); const { log } = require('./logging'); const emailService = require('./email/service-email'); const db = require('../db'); + +const { knex } = db; +const { getGrantActivityByUserId, getGrantActivityEmailRecipients } = require('./grantsCollaboration/grantActivity'); const { notificationType, tags } = require('./email/constants'); const { isUSDR, isUSDRSuperAdmin } = require('./access-helpers'); @@ -465,6 +469,20 @@ async function buildDigestBody({ name, openDate, matchedGrants }) { return formattedBody; } +async function buildGrantActivityDigestBody({ name, grants, periodEnd }) { + // Build Email Here + // + // + + const formattedBody = mustache.render(JSON.stringify(grants), { + body_title: `${name}: New activity in your grants`, + body_detail: DateTime.fromJSDate(periodEnd).toFormat('DDD'), + additional_body: '', + }); + + return formattedBody; +} + async function sendGrantDigestEmail({ name, matchedGrants, matchedGrantsTotal, recipient, openDate, }) { @@ -502,6 +520,41 @@ async function sendGrantDigestEmail({ ); } +async function sendGrantActivityDigestEmail({ + name, recipientEmail, grants, periodEnd, +}) { + console.log(`${name} is subscribed for digests on ${periodEnd}`); + + if (!grants || grants?.length === 0) { + console.error(`There was no grant note/follow activity available for ${name}`); + return; + } + + const formattedBody = await buildGrantActivityDigestBody({ name, grants, periodEnd }); + const preheader = 'See recent activity from grants you follow'; + + const emailHTML = addBaseBranding(formattedBody, { + tool_name: 'Federal Grant Finder', + title: 'New activity in your grants', + preheader, + includeNotificationsLink: true, + }); + + // TODO: add plain text version of the email + const emailPlain = emailHTML.replace(/<[^>]+>/g, ''); + + await deliverEmail( + { + fromName: GRANT_FINDER_EMAIL_FROM_NAME, + toAddress: recipientEmail, + emailHTML, + emailPlain, + subject: 'New activity in your grants', + emailType: tags.emailTypes.grantActivityDigest, + }, + ); +} + async function getAndSendGrantForSavedSearch({ userSavedSearch, openDate, }) { @@ -528,6 +581,21 @@ async function getAndSendGrantForSavedSearch({ }); } +async function getAndSendGrantActivityDigest({ + recipientId, + periodStart, + periodEnd, +}) { + const grantActivityResults = await getGrantActivityByUserId(knex, recipientId, periodStart, periodEnd); + + return sendGrantActivityDigestEmail({ + name: grantActivityResults.userName, + recipientEmail: grantActivityResults.userEmail, + grants: grantActivityResults.grants, + periodEnd, + }); +} + function yesterday() { return moment().subtract(1, 'day').format('YYYY-MM-DD'); } @@ -553,6 +621,31 @@ async function buildAndSendGrantDigestEmails(userId, openDate = yesterday()) { console.log(`Successfully built and sent grants digest emails for ${inputs.length} saved searches on ${openDate}`); } +async function buildAndSendGrantActivityDigestEmails(userId, periodStart, periodEnd) { + const userGroup = userId ? `user Id ${userId}` : 'all users'; + console.log(`Building and sending Grant Activity Digest email for ${userGroup} on ${periodEnd}`); + /* + 1. get all email recipients + 2. call getAndSendGrantActivityDigest to find activity for each user and send the digest + */ + let recipientIds = await getGrantActivityEmailRecipients(knex, periodStart, periodEnd); + + if (userId) { + // Send specific user only if activity exists + recipientIds = recipientIds.filter((recipientId) => recipientId === userId); + } + + const inputs = recipientIds.map((recipientId) => ({ + recipientId, + periodStart, + periodEnd, + })); + + await asyncBatch(inputs, getAndSendGrantActivityDigest, 2); + + console.log(`Successfully built and sent grants digest emails for ${inputs.length} users on ${periodEnd}`); +} + async function sendAsyncReportEmail(recipient, signedUrl, reportType) { const formattedBodyTemplate = fileSystem.readFileSync(path.join(__dirname, '../static/email_templates/_formatted_body.html')); @@ -592,6 +685,7 @@ module.exports = { * Send grant digest emails to all subscribed users. */ buildAndSendGrantDigestEmails, + buildAndSendGrantActivityDigestEmails, sendGrantDigestEmail, // Exposed for testing buildDigestBody, diff --git a/packages/server/src/lib/email/constants.js b/packages/server/src/lib/email/constants.js index 8466b708e..c03cfcaac 100644 --- a/packages/server/src/lib/email/constants.js +++ b/packages/server/src/lib/email/constants.js @@ -29,6 +29,7 @@ const tags = Object.freeze( treasuryReport: 'treasury_report', welcome: 'welcome', grantDigest: 'grant_digest', + grantActivityDigest: 'grant_activity_digest', treasuryReportError: 'treasury_report_error', auditReportError: 'audit_report_error', }, diff --git a/packages/server/src/lib/grantsCollaboration/grantActivity.js b/packages/server/src/lib/grantsCollaboration/grantActivity.js index 9566710a2..9f9a99218 100644 --- a/packages/server/src/lib/grantsCollaboration/grantActivity.js +++ b/packages/server/src/lib/grantsCollaboration/grantActivity.js @@ -56,10 +56,12 @@ async function getGrantActivityEmailRecipients(knex, periodStart, periodEnd) { return _.map(results, 'recipient_user_id'); } -async function getGrantActivity(knex, userId, periodStart, periodEnd) { +async function getGrantActivityByUserId(knex, userId, periodStart, periodEnd) { const query = knex.select([ 'g.grant_id AS grant_id', 'g.title AS grant_title', + 'recipient_users.name AS recipient_user_name', + 'recipient_users.email AS recipient_user_email', 'activity_users.id AS user_id', 'activity_users.name AS user_name', 'activity_users.email AS user_email', @@ -85,6 +87,7 @@ async function getGrantActivity(knex, userId, periodStart, periodEnd) { .andWhereRaw('activity_users.tenant_id = recipient_users.tenant_id') // limit activity to grants for which the recipient user is a follower .andWhere('recipient_followers.user_id', userId) + .andWhereNot('activity_users.id', userId) .orderBy([ { column: 'g.grant_id', order: 'desc' }, { column: 'activity.activity_at', order: 'asc' }, @@ -98,27 +101,33 @@ async function getGrantActivity(knex, userId, periodStart, periodEnd) { // Grant IDs distinct const grantIds = [...new Set(_.map(results, 'grant_id'))]; - return grantIds.map((grantId) => { - const activities = resultsByGrant[grantId].map((act) => ({ - userId: act.user_id, - userName: act.user_name, - agencyName: act.agency_name, - activityAt: act.activity_at, - activityType: act.activity_type, - noteText: act.note_text, - })); - const activitiesByType = _.groupBy(activities, 'activityType'); + const result = { + userEmail: results.length ? results[0].recipient_user_email : null, + userName: results.length ? results[0].recipient_user_name : null, + grants: grantIds.map((grantId) => { + const activities = resultsByGrant[grantId].map((act) => ({ + userId: act.user_id, + userName: act.user_name, + agencyName: act.agency_name, + activityAt: act.activity_at, + activityType: act.activity_type, + noteText: act.note_text, + })); + const activitiesByType = _.groupBy(activities, 'activityType'); - return { - grantId, - grantTitle: resultsByGrant[grantId][0].grant_title, - notes: activitiesByType.note || [], - follows: activitiesByType.follow || [], - }; - }); + return { + grantId, + grantTitle: resultsByGrant[grantId][0].grant_title, + notes: activitiesByType.note || [], + follows: activitiesByType.follow || [], + }; + }), + }; + + return result; } module.exports = { - getGrantActivity, + getGrantActivityByUserId, getGrantActivityEmailRecipients, }; diff --git a/packages/server/src/scripts/sendGrantActivityDigestEmail.js b/packages/server/src/scripts/sendGrantActivityDigestEmail.js new file mode 100644 index 000000000..f55e77372 --- /dev/null +++ b/packages/server/src/scripts/sendGrantActivityDigestEmail.js @@ -0,0 +1,27 @@ +#!/usr/bin/env node +const tracer = require('dd-trace').init(); +const { log } = require('../lib/logging'); +const email = require('../lib/email'); + +/** + * This script sends all enabled grant activity digest emails. It is triggered by a + * scheduled ECS task configured in terraform to run on a daily basis. + * + * This setup is intended to be a temporary fix until we've set up more + * robust email infrastructure. See here for context: + * https://github.com/usdigitalresponse/usdr-gost/issues/2133 + */ +exports.main = async function main() { + if (process.env.ENABLE_GRANT_ACTIVITY_DIGEST_SCHEDULED_TASK !== 'true') { + return; + } + + await tracer.trace('sendGrantActivityDigestEmail', async () => { + log.info('Sending grant activity digest emails'); + await email.buildAndSendGrantActivityDigestEmails(); + }); +}; + +if (require.main === module) { + exports.main().then(() => process.exit()); +} diff --git a/terraform/main.tf b/terraform/main.tf index 06526ae91..92aaf4f12 100644 --- a/terraform/main.tf +++ b/terraform/main.tf @@ -177,14 +177,15 @@ module "api" { ecs_cluster_name = join("", aws_ecs_cluster.default[*].name) # Task configuration - docker_tag = var.api_container_image_tag - default_desired_task_count = var.api_default_desired_task_count - autoscaling_desired_count_minimum = var.api_minumum_task_count - autoscaling_desired_count_maximum = var.api_maximum_task_count - enable_new_team_terminology = var.api_enable_new_team_terminology - enable_saved_search_grants_digest = var.api_enable_saved_search_grants_digest - enable_grant_digest_scheduled_task = var.api_enable_grant_digest_scheduled_task - unified_service_tags = local.unified_service_tags + docker_tag = var.api_container_image_tag + default_desired_task_count = var.api_default_desired_task_count + autoscaling_desired_count_minimum = var.api_minumum_task_count + autoscaling_desired_count_maximum = var.api_maximum_task_count + enable_new_team_terminology = var.api_enable_new_team_terminology + enable_saved_search_grants_digest = var.api_enable_saved_search_grants_digest + enable_grant_digest_scheduled_task = var.api_enable_grant_digest_scheduled_task + enable_grant_activity_digest_scheduled_task = var.api_enable_grant_activity_digest_scheduled_task + unified_service_tags = local.unified_service_tags datadog_environment_variables = merge( var.default_datadog_environment_variables, var.api_datadog_environment_variables, diff --git a/terraform/modules/gost_api/grant_activity_digest.tf b/terraform/modules/gost_api/grant_activity_digest.tf new file mode 100644 index 000000000..504f2d667 --- /dev/null +++ b/terraform/modules/gost_api/grant_activity_digest.tf @@ -0,0 +1,46 @@ +module "grant_activity_digest_scheduled_task" { + source = "../scheduled_ecs_task" + enabled = var.enabled && var.enable_grant_activity_digest_scheduled_task + + name_prefix = "${var.namespace}-grant-activity-digest-task" + description = "Executes an ECS task that sends grants activity digest emails at 8am ET" + + # Schedule + schedule_expression = "cron(0 8 * * *)" + schedule_expression_timezone = "America/New_York" + flexible_time_window = { hours = 1 } + # Until we have more robust email retry handling, we'll limit to 1 attempt to avoid sending multiple duplicate emails to a portion of the audience + retry_policy_max_attempts = 1 + retry_policy_max_event_age = { hours = 4 } + + // Permissions + task_role_arn = join("", aws_ecs_task_definition.default[*].task_role_arn) + task_execution_role_arn = join("", aws_ecs_task_definition.default[*].execution_role_arn) + permissions_boundary_arn = var.permissions_boundary_arn + + // Task settings + cluster_arn = join("", data.aws_ecs_cluster.default[*].arn) + task_definition_arn = join("", aws_ecs_task_definition.default[*].arn) + task_revision = "LATEST" + launch_type = "FARGATE" + enable_ecs_managed_tags = true + enable_execute_command = false + + task_override = jsonencode({ + containerOverrides = [ + { + name = "api" + command = [ + "node", + "./src/scripts/sendGrantActivityDigestEmail.js" + ] + }, + ] + }) + + network_configuration = { + assign_public_ip = false + security_groups = var.security_group_ids + subnets = var.subnet_ids + } +} diff --git a/terraform/modules/gost_api/task.tf b/terraform/modules/gost_api/task.tf index ef4c9cd9d..a07066e19 100644 --- a/terraform/modules/gost_api/task.tf +++ b/terraform/modules/gost_api/task.tf @@ -44,18 +44,19 @@ module "api_container_definition" { map_environment = merge( { - API_DOMAIN = "https://${var.domain_name}" - AUDIT_REPORT_BUCKET = module.arpa_audit_reports_bucket.bucket_id - DATA_DIR = "/var/data" - ENABLE_SAVED_SEARCH_GRANTS_DIGEST = var.enable_saved_search_grants_digest ? "true" : "false" - ENABLE_GRANT_DIGEST_SCHEDULED_TASK = var.enable_grant_digest_scheduled_task ? "true" : "false" - ENABLE_NEW_TEAM_TERMINOLOGY = var.enable_new_team_terminology ? "true" : "false" - NODE_OPTIONS = "--max_old_space_size=1024" - NOTIFICATIONS_EMAIL = var.notifications_email_address - PGSSLROOTCERT = "rds-global-bundle.pem" - SES_CONFIGURATION_SET_DEFAULT = var.ses_configuration_set_default - VUE_APP_GRANTS_API_URL = module.api_gateway.apigatewayv2_api_api_endpoint - WEBSITE_DOMAIN = "https://${var.website_domain_name}" + API_DOMAIN = "https://${var.domain_name}" + AUDIT_REPORT_BUCKET = module.arpa_audit_reports_bucket.bucket_id + DATA_DIR = "/var/data" + ENABLE_SAVED_SEARCH_GRANTS_DIGEST = var.enable_saved_search_grants_digest ? "true" : "false" + ENABLE_GRANT_DIGEST_SCHEDULED_TASK = var.enable_grant_digest_scheduled_task ? "true" : "false" + ENABLE_GRANT_ACTIVITY_DIGEST_SCHEDULED_TASK = var.enable_grant_activity_digest_scheduled_task ? "true" : "false" + ENABLE_NEW_TEAM_TERMINOLOGY = var.enable_new_team_terminology ? "true" : "false" + NODE_OPTIONS = "--max_old_space_size=1024" + NOTIFICATIONS_EMAIL = var.notifications_email_address + PGSSLROOTCERT = "rds-global-bundle.pem" + SES_CONFIGURATION_SET_DEFAULT = var.ses_configuration_set_default + VUE_APP_GRANTS_API_URL = module.api_gateway.apigatewayv2_api_api_endpoint + WEBSITE_DOMAIN = "https://${var.website_domain_name}" }, local.datadog_env_vars, var.api_container_environment, diff --git a/terraform/modules/gost_api/variables.tf b/terraform/modules/gost_api/variables.tf index a84066a23..1e32d284c 100644 --- a/terraform/modules/gost_api/variables.tf +++ b/terraform/modules/gost_api/variables.tf @@ -179,6 +179,12 @@ variable "enable_grant_digest_scheduled_task" { default = false } +variable "enable_grant_activity_digest_scheduled_task" { + description = "When true, sets the ENABLE_GRANT_ACTIVITY_DIGEST_SCHEDULED_TASK environment variable to true in the API container." + type = bool + default = false +} + variable "unified_service_tags" { description = "Datadog unified service tags to apply to runtime environments." type = object({ diff --git a/terraform/prod.tfvars b/terraform/prod.tfvars index c6255dc75..f309902e6 100644 --- a/terraform/prod.tfvars +++ b/terraform/prod.tfvars @@ -66,14 +66,15 @@ website_google_tag_id = "G-WCDTMFM6RG" cluster_container_insights_enabled = true // API / Backend -api_enabled = true -api_default_desired_task_count = 3 -api_minumum_task_count = 2 -api_maximum_task_count = 5 -api_enable_new_team_terminology = true -api_enable_saved_search_grants_digest = true -api_enable_grant_digest_scheduled_task = true -api_log_retention_in_days = 30 +api_enabled = true +api_default_desired_task_count = 3 +api_minumum_task_count = 2 +api_maximum_task_count = 5 +api_enable_new_team_terminology = true +api_enable_saved_search_grants_digest = true +api_enable_grant_digest_scheduled_task = true +api_enable_grant_activity_digest_scheduled_task = true +api_log_retention_in_days = 30 api_container_environment = { NEW_GRANT_DETAILS_PAGE_ENABLED = true SHARE_TERMINOLOGY_ENABLED = true diff --git a/terraform/sandbox.tfvars b/terraform/sandbox.tfvars index 904ffeb11..f757418ae 100644 --- a/terraform/sandbox.tfvars +++ b/terraform/sandbox.tfvars @@ -22,15 +22,16 @@ website_feature_flags = { cluster_container_insights_enabled = false // API / Backend -api_enabled = true -api_container_image_tag = "latest" -api_default_desired_task_count = 1 -api_minumum_task_count = 1 -api_maximum_task_count = 5 -api_enable_new_team_terminology = false -api_enable_saved_search_grants_digest = false -api_enable_grant_digest_scheduled_task = false -api_log_retention_in_days = 7 +api_enabled = true +api_container_image_tag = "latest" +api_default_desired_task_count = 1 +api_minumum_task_count = 1 +api_maximum_task_count = 5 +api_enable_new_team_terminology = false +api_enable_saved_search_grants_digest = false +api_enable_grant_digest_scheduled_task = false +api_enable_grant_activity_digest_scheduled_task = false +api_log_retention_in_days = 7 // Postgres postgres_enabled = true diff --git a/terraform/staging.tfvars b/terraform/staging.tfvars index 69284a8de..a2f3dd3b0 100644 --- a/terraform/staging.tfvars +++ b/terraform/staging.tfvars @@ -65,14 +65,15 @@ website_google_tag_id = "G-D5DFR7BN0N" cluster_container_insights_enabled = true // API / Backend -api_enabled = true -api_default_desired_task_count = 1 -api_minumum_task_count = 1 -api_maximum_task_count = 5 -api_enable_new_team_terminology = true -api_enable_saved_search_grants_digest = true -api_enable_grant_digest_scheduled_task = true -api_log_retention_in_days = 14 +api_enabled = true +api_default_desired_task_count = 1 +api_minumum_task_count = 1 +api_maximum_task_count = 5 +api_enable_new_team_terminology = true +api_enable_saved_search_grants_digest = true +api_enable_grant_digest_scheduled_task = true +api_enable_grant_activity_digest_scheduled_task = true +api_log_retention_in_days = 14 api_container_environment = { NEW_GRANT_DETAILS_PAGE_ENABLED = true, SHARE_TERMINOLOGY_ENABLED = true, diff --git a/terraform/variables.tf b/terraform/variables.tf index a80f975a0..e4feb914f 100644 --- a/terraform/variables.tf +++ b/terraform/variables.tf @@ -218,6 +218,10 @@ variable "api_enable_grant_digest_scheduled_task" { type = bool } +variable "api_enable_grant_activity_digest_scheduled_task" { + type = bool +} + variable "api_log_retention_in_days" { type = number } From 763023241855f552ab5ccac80eae325b787dad84 Mon Sep 17 00:00:00 2001 From: greg-adams Date: Tue, 15 Oct 2024 16:03:17 -0700 Subject: [PATCH 3/7] include email subscriptions --- packages/server/__tests__/email/email.test.js | 19 ++- .../lib/grants-collaboration.test.js | 128 +++++++++++------- .../sendGrantActivityDigestEmail.test.js | 2 +- packages/server/src/db/constants.js | 3 + packages/server/src/lib/email.js | 32 ++++- packages/server/src/lib/email/constants.js | 4 +- .../lib/grantsCollaboration/grantActivity.js | 27 ++-- .../scripts/sendGrantActivityDigestEmail.js | 8 +- 8 files changed, 157 insertions(+), 66 deletions(-) diff --git a/packages/server/__tests__/email/email.test.js b/packages/server/__tests__/email/email.test.js index 367743d76..4bd5b7655 100644 --- a/packages/server/__tests__/email/email.test.js +++ b/packages/server/__tests__/email/email.test.js @@ -10,7 +10,7 @@ const emailService = require('../../src/lib/email/service-email'); const email = require('../../src/lib/email'); const fixtures = require('../db/seeds/fixtures'); const db = require('../../src/db'); -const { tags } = require('../../src/lib/email/constants'); +const { tags, notificationType, emailSubscriptionStatus } = require('../../src/lib/email/constants'); const { knex } = db; const awsTransport = require('../../src/lib/gost-aws'); @@ -223,7 +223,6 @@ describe('Email sender', () => { process.env.DD_SERVICE = 'test-dd-service'; process.env.DD_ENV = 'test-dd-env'; process.env.DD_VERSION = 'test-dd-version'; - sandbox.spy(emailService); }); after(async () => { @@ -235,6 +234,7 @@ describe('Email sender', () => { beforeEach(async () => { await fixtures.seed(knex); + sandbox.spy(emailService); }); afterEach(() => { @@ -590,6 +590,21 @@ describe('Email sender', () => { sendFake = sinon.fake.returns('foo'); sinon.replace(emailService, 'getTransport', sinon.fake.returns({ sendEmail: sendFake })); + await knex('email_subscriptions').insert([ + { + user_id: adminUser.id, + agency_id: adminUser.agency_id, + notification_type: notificationType.grantActivity, + status: emailSubscriptionStatus.subscribed, + }, + { + user_id: staffUser.id, + agency_id: staffUser.agency_id, + notification_type: notificationType.grantActivity, + status: emailSubscriptionStatus.subscribed, + }, + ]); + periodStart = new Date(); // Grant 1 Follows diff --git a/packages/server/__tests__/lib/grants-collaboration.test.js b/packages/server/__tests__/lib/grants-collaboration.test.js index e2d35e121..5a76b30ab 100644 --- a/packages/server/__tests__/lib/grants-collaboration.test.js +++ b/packages/server/__tests__/lib/grants-collaboration.test.js @@ -1,9 +1,9 @@ const { expect, use } = require('chai'); const chaiAsPromised = require('chai-as-promised'); const { DateTime } = require('luxon'); -const _ = require('lodash'); const knex = require('../../src/db/connection'); const fixtures = require('../db/seeds/fixtures'); +const { seed } = require('../db/seeds/fixtures'); const { saveNoteRevision, getOrganizationNotesForGrant, getOrganizationNotesForGrantByUser } = require('../../src/lib/grantsCollaboration/notes'); const { followGrant, unfollowGrant, getFollowerForGrant, getFollowersForGrant, @@ -11,9 +11,14 @@ const { const { getGrantActivityByUserId, getGrantActivityEmailRecipients, } = require('../../src/lib/grantsCollaboration/grantActivity'); +const emailConstants = require('../../src/lib/email/constants'); use(chaiAsPromised); +afterEach(async () => { + await seed(knex); +}); + describe('Grants Collaboration', () => { context('saveNoteRevision', () => { it('creates new note', async () => { @@ -260,6 +265,15 @@ describe('Grants Collaboration', () => { return Array.from(ids); }; + const subscribeUser = async (user) => { + await knex('email_subscriptions').insert({ + user_id: user.id, + agency_id: user.agency_id, + notification_type: emailConstants.notificationType.grantActivity, + status: emailConstants.emailSubscriptionStatus.subscribed, + }); + }; + const { adminUser, staffUser } = fixtures.users; const grant1 = fixtures.grants.earFellowship; const grant2 = fixtures.grants.healthAide; @@ -271,6 +285,9 @@ describe('Grants Collaboration', () => { let grant1NoteStaff; beforeEach(async () => { + subscribeUser(adminUser); + subscribeUser(staffUser); + periodStart = DateTime.now().minus({ days: 1 }).toJSDate(); // Grant 1 Follows @@ -321,6 +338,15 @@ describe('Grants Collaboration', () => { ]); }); + it('does not return recipients if users are unsubscribed', async () => { + // Unsubscribe all users + await knex('email_subscriptions').update({ status: emailConstants.emailSubscriptionStatus.unsubscribed }); + + const recipients = await getGrantActivityEmailRecipients(knex, periodStart, periodEnd); + + expect(recipients).to.have.length(0); + }); + it('retrieves all note/follow activity by period', async () => { const grantActivity = await getGrantActivityByUserId(knex, adminUser.id, periodStart, periodEnd); @@ -377,56 +403,64 @@ describe('Grants Collaboration', () => { ); }); - it('Grant activity results should ignore activity by other org/tenants', async () => { + context('Grant activity by other org/tenants', async () => { const { usdrUser: otherUser1, usdrAdmin: otherUser2 } = fixtures.users; - await knex('grant_followers') - .insert([ - { grant_id: grant1.grant_id, user_id: otherUser1.id }, - { grant_id: grant1.grant_id, user_id: otherUser2.id }, - ], 'id'); - - const [otherUser1Note, otherUser2Note] = await knex('grant_notes') - .insert([ - { grant_id: grant1.grant_id, user_id: otherUser1.id }, - { grant_id: grant1.grant_id, user_id: otherUser2.id }, - ], 'id'); - - await knex('grant_notes_revisions') - .insert([ - { grant_note_id: otherUser1Note.id, text: 'Other tenant note1' }, - { grant_note_id: otherUser2Note.id, text: 'Other tenant note2' }, - ], 'id'); - periodEnd = new Date(); - - // Email recipients INCLUDES multiple tenants - expect(await getGrantActivityEmailRecipients(knex, periodStart, periodEnd)).to.have.members([ - adminUser.id, - staffUser.id, - otherUser1.id, - otherUser2.id, - ]); - - const getGrantActivity = async (userId) => { - const grantActivity = await getGrantActivityByUserId(knex, userId, periodStart, periodEnd); - return grantActivity.grants; - }; - - const tenantOneUsers = [adminUser.id, staffUser.id]; - const tenantTwoUsers = [otherUser1.id, otherUser2.id]; + beforeEach(async () => { + await subscribeUser(otherUser1); + await subscribeUser(otherUser2); + + await knex('grant_followers') + .insert([ + { grant_id: grant1.grant_id, user_id: otherUser1.id }, + { grant_id: grant1.grant_id, user_id: otherUser2.id }, + ], 'id'); + + const [otherUser1Note, otherUser2Note] = await knex('grant_notes') + .insert([ + { grant_id: grant1.grant_id, user_id: otherUser1.id }, + { grant_id: grant1.grant_id, user_id: otherUser2.id }, + ], 'id'); + + await knex('grant_notes_revisions') + .insert([ + { grant_note_id: otherUser1Note.id, text: 'Other tenant note1' }, + { grant_note_id: otherUser2Note.id, text: 'Other tenant note2' }, + ], 'id'); + periodEnd = new Date(); + }); - // Digest activity does NOT include cross-over between tenants - const tenantOneGrants = await Promise.all( - tenantOneUsers.map((userId) => getGrantActivity(userId)), - ); - const tenantOneUserIds = getUserIdsForActivities(tenantOneGrants.flat()); - expect(tenantOneUserIds).not.to.include.members(tenantTwoUsers); + it('Includes recipients from multiple tenants', async () => { + // Email recipients INCLUDES multiple tenants + expect(await getGrantActivityEmailRecipients(knex, periodStart, periodEnd)).to.have.members([ + adminUser.id, + staffUser.id, + otherUser1.id, + otherUser2.id, + ]); + }); - const tenantTwoGrants = await Promise.all( - tenantTwoUsers.map((userId) => getGrantActivity(userId)), - ); - const tenantTwoUserIds = getUserIdsForActivities(tenantTwoGrants.flat()); - expect(tenantTwoUserIds).not.to.include.members(tenantOneUsers); + it('Does NOT include cross-over activity events from multiple tenants', async () => { + const getGrantActivity = async (userId) => { + const grantActivity = await getGrantActivityByUserId(knex, userId, periodStart, periodEnd); + return grantActivity.grants; + }; + + const tenantOneUsers = [adminUser.id, staffUser.id]; + const tenantTwoUsers = [otherUser1.id, otherUser2.id]; + + const tenantOneGrants = await Promise.all( + tenantOneUsers.map((userId) => getGrantActivity(userId)), + ); + const tenantOneUserIds = getUserIdsForActivities(tenantOneGrants.flat()); + expect(tenantOneUserIds).not.to.include.members(tenantTwoUsers); + + const tenantTwoGrants = await Promise.all( + tenantTwoUsers.map((userId) => getGrantActivity(userId)), + ); + const tenantTwoUserIds = getUserIdsForActivities(tenantTwoGrants.flat()); + expect(tenantTwoUserIds).not.to.include.members(tenantOneUsers); + }); }); }); }); diff --git a/packages/server/__tests__/scripts/sendGrantActivityDigestEmail.test.js b/packages/server/__tests__/scripts/sendGrantActivityDigestEmail.test.js index 1a2c0e429..279008ef9 100644 --- a/packages/server/__tests__/scripts/sendGrantActivityDigestEmail.test.js +++ b/packages/server/__tests__/scripts/sendGrantActivityDigestEmail.test.js @@ -8,7 +8,6 @@ describe('sendGrantActivityDigestEmail script', () => { let buildAndSendGrantActivityDigestEmailsFake; beforeEach(() => { - process.env.ENABLE_GRANT_ACTIVITY_DIGEST_SCHEDULED_TASK = 'true'; buildAndSendGrantActivityDigestEmailsFake = sandbox.fake(); sandbox.replace(email, 'buildAndSendGrantActivityDigestEmails', buildAndSendGrantActivityDigestEmailsFake); }); @@ -18,6 +17,7 @@ describe('sendGrantActivityDigestEmail script', () => { }); it('triggers sending digest emails when flags are on', async () => { + process.env.ENABLE_GRANT_ACTIVITY_DIGEST_SCHEDULED_TASK = 'true'; await sendGrantActivityDigestEmail(); expect(buildAndSendGrantActivityDigestEmailsFake.called).to.equal(true); }); diff --git a/packages/server/src/db/constants.js b/packages/server/src/db/constants.js index f8014100b..1549cc8c7 100644 --- a/packages/server/src/db/constants.js +++ b/packages/server/src/db/constants.js @@ -14,5 +14,8 @@ module.exports = { keywords: 'keywords', email_subscriptions: 'email_subscriptions', grants_saved_searches: 'grants_saved_searches', + grant_notes: 'grant_notes', + grant_followers: 'grant_followers', + grant_notes_revisions: 'grant_notes_revisions', }, }; diff --git a/packages/server/src/lib/email.js b/packages/server/src/lib/email.js index 7f5fbde16..99400ffda 100644 --- a/packages/server/src/lib/email.js +++ b/packages/server/src/lib/email.js @@ -473,8 +473,31 @@ async function buildGrantActivityDigestBody({ name, grants, periodEnd }) { // Build Email Here // // + const formatActivities = (activities) => activities.reduce((output, activity) => `${output} +
  • + ${activity.userName} - ${activity.agencyName}
    + ${activity.userEmail}

    + ${activity.noteText || ''} +
    • + `, ''); + + let body = ''; + grants.forEach((grant) => { + body += `

    ${grant.grantTitle}

    `; + + body += `

    ${grant.notes.length} New Notes:

    `; + body += ` +
      ${formatActivities(grant.notes)}
    + `; + + body += `

    ${grant.follows.length} New Follows:

    `; + body += ` +
      ${formatActivities(grant.follows)}
    + `; + body += '
    '; + }); - const formattedBody = mustache.render(JSON.stringify(grants), { + const formattedBody = mustache.render(body, { body_title: `${name}: New activity in your grants`, body_detail: DateTime.fromJSDate(periodEnd).toFormat('DDD'), additional_body: '', @@ -523,12 +546,11 @@ async function sendGrantDigestEmail({ async function sendGrantActivityDigestEmail({ name, recipientEmail, grants, periodEnd, }) { - console.log(`${name} is subscribed for digests on ${periodEnd}`); - if (!grants || grants?.length === 0) { console.error(`There was no grant note/follow activity available for ${name}`); return; } + console.log(`${name} is will receive digests on ${DateTime.fromJSDate(periodEnd).toLocaleString(DateTime.DATE_FULL)}`); const formattedBody = await buildGrantActivityDigestBody({ name, grants, periodEnd }); const preheader = 'See recent activity from grants you follow'; @@ -623,7 +645,9 @@ async function buildAndSendGrantDigestEmails(userId, openDate = yesterday()) { async function buildAndSendGrantActivityDigestEmails(userId, periodStart, periodEnd) { const userGroup = userId ? `user Id ${userId}` : 'all users'; - console.log(`Building and sending Grant Activity Digest email for ${userGroup} on ${periodEnd}`); + console.log(` + Building and sending Grant Activity Digest email for ${userGroup} on ${DateTime.fromJSDate(periodEnd).toLocaleString(DateTime.DATE_FULL)} + `); /* 1. get all email recipients 2. call getAndSendGrantActivityDigest to find activity for each user and send the digest diff --git a/packages/server/src/lib/email/constants.js b/packages/server/src/lib/email/constants.js index c03cfcaac..dab205aaf 100644 --- a/packages/server/src/lib/email/constants.js +++ b/packages/server/src/lib/email/constants.js @@ -36,6 +36,8 @@ const tags = Object.freeze( }, ); +const TZ_NY = 'America/New_York'; + module.exports = { - notificationType, emailSubscriptionStatus, defaultSubscriptionPreference, tags, + notificationType, emailSubscriptionStatus, defaultSubscriptionPreference, tags, TZ_NY, }; diff --git a/packages/server/src/lib/grantsCollaboration/grantActivity.js b/packages/server/src/lib/grantsCollaboration/grantActivity.js index 9f9a99218..1520ccb98 100644 --- a/packages/server/src/lib/grantsCollaboration/grantActivity.js +++ b/packages/server/src/lib/grantsCollaboration/grantActivity.js @@ -1,9 +1,11 @@ const _ = require('lodash'); +const { TABLES } = require('../../db/constants'); +const emailConstants = require('../email/constants'); const getActivitiesQuery = (knex) => { const noteRevisionsSub = knex .select() - .from({ r: 'grant_notes_revisions' }) + .from({ r: TABLES.grant_notes_revisions }) .whereRaw('r.grant_note_id = gn.id') .orderBy('r.created_at', 'desc') .limit(1); @@ -40,11 +42,17 @@ async function getGrantActivityEmailRecipients(knex, periodStart, periodEnd) { .from( getActivitiesQuery(knex), ) - .join({ recipient_followers: 'grant_followers' }, 'recipient_followers.grant_id', 'activity.grant_id') - .join({ activity_users: 'users' }, 'activity_users.id', 'activity.user_id') - .join({ recipient_users: 'users' }, 'recipient_users.id', 'recipient_followers.user_id') + .join({ recipient_followers: TABLES.grant_followers }, 'recipient_followers.grant_id', 'activity.grant_id') + .join({ activity_users: TABLES.users }, 'activity_users.id', 'activity.user_id') + .join({ recipient_users: TABLES.users }, 'recipient_users.id', 'recipient_followers.user_id') + .leftJoin({ recipient_subscriptions: TABLES.email_subscriptions }, (builder) => { + builder + .on(`recipient_followers.user_id`, '=', `recipient_subscriptions.user_id`) + .on(`recipient_subscriptions.notification_type`, '=', knex.raw('?', [emailConstants.notificationType.grantActivity])); + }) .where('activity.activity_at', '>', periodStart) .andWhere('activity.activity_at', '<', periodEnd) + .andWhere('recipient_subscriptions.status', emailConstants.emailSubscriptionStatus.subscribed) // only consider actions taken by users in the same organization as the recipient: .andWhereRaw('recipient_users.tenant_id = activity_users.tenant_id') // exclude rows where the recipient user is the one taking the action, @@ -73,14 +81,14 @@ async function getGrantActivityByUserId(knex, userId, periodStart, periodEnd) { .from( getActivitiesQuery(knex), ) - .join({ recipient_followers: 'grant_followers' }, 'recipient_followers.grant_id', 'activity.grant_id') + .join({ recipient_followers: TABLES.grant_followers }, 'recipient_followers.grant_id', 'activity.grant_id') // incorporate users table for users responsible for the activity: - .join({ activity_users: 'users' }, 'activity_users.id', 'activity.user_id') + .join({ activity_users: TABLES.users }, 'activity_users.id', 'activity.user_id') // incorporate users table for the recipient follower: - .join({ recipient_users: 'users' }, 'recipient_users.id', 'recipient_followers.user_id') + .join({ recipient_users: TABLES.users }, 'recipient_users.id', 'recipient_followers.user_id') // Additional JOINs for data selected for use in the email's content: - .join({ g: 'grants' }, 'g.grant_id', 'activity.grant_id') - .join({ activity_users_agencies: 'agencies' }, 'activity_users_agencies.id', 'activity_users.agency_id') + .join({ g: TABLES.grants }, 'g.grant_id', 'activity.grant_id') + .join({ activity_users_agencies: TABLES.agencies }, 'activity_users_agencies.id', 'activity_users.agency_id') .where('activity.activity_at', '>', periodStart) .andWhere('activity.activity_at', '<', periodEnd) // Limit to activity where the user performing the activity belongs to the same organization: @@ -108,6 +116,7 @@ async function getGrantActivityByUserId(knex, userId, periodStart, periodEnd) { const activities = resultsByGrant[grantId].map((act) => ({ userId: act.user_id, userName: act.user_name, + userEmail: act.user_email, agencyName: act.agency_name, activityAt: act.activity_at, activityType: act.activity_type, diff --git a/packages/server/src/scripts/sendGrantActivityDigestEmail.js b/packages/server/src/scripts/sendGrantActivityDigestEmail.js index f55e77372..7e65661c5 100644 --- a/packages/server/src/scripts/sendGrantActivityDigestEmail.js +++ b/packages/server/src/scripts/sendGrantActivityDigestEmail.js @@ -1,7 +1,9 @@ #!/usr/bin/env node const tracer = require('dd-trace').init(); +const { DateTime } = require('luxon'); const { log } = require('../lib/logging'); -const email = require('../lib/email'); +const { buildAndSendGrantActivityDigestEmails } = require('../lib/email'); +const { TZ_NY } = require('../lib/email/constants'); /** * This script sends all enabled grant activity digest emails. It is triggered by a @@ -18,7 +20,9 @@ exports.main = async function main() { await tracer.trace('sendGrantActivityDigestEmail', async () => { log.info('Sending grant activity digest emails'); - await email.buildAndSendGrantActivityDigestEmails(); + const periodEnd = DateTime.local({ hours: 8, zone: TZ_NY }); + const periodStart = periodEnd.minus({ days: 1 }); + await buildAndSendGrantActivityDigestEmails(null, periodStart.toJSDate(), periodEnd.toJSDate()); }); }; From 9876c09c8ecea6d2af893fa19a4034cb271d13c7 Mon Sep 17 00:00:00 2001 From: greg-adams Date: Wed, 16 Oct 2024 10:38:26 -0700 Subject: [PATCH 4/7] resolve test --- packages/server/__tests__/email/email.test.js | 1 - packages/server/__tests__/lib/grants-collaboration.test.js | 2 +- packages/server/src/scripts/sendGrantActivityDigestEmail.js | 4 ++-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/server/__tests__/email/email.test.js b/packages/server/__tests__/email/email.test.js index 4bd5b7655..78e30d987 100644 --- a/packages/server/__tests__/email/email.test.js +++ b/packages/server/__tests__/email/email.test.js @@ -2,7 +2,6 @@ const { expect } = require('chai'); const moment = require('moment'); -const { DateTime } = require('luxon'); const sinon = require('sinon'); const _ = require('lodash'); require('dotenv').config(); diff --git a/packages/server/__tests__/lib/grants-collaboration.test.js b/packages/server/__tests__/lib/grants-collaboration.test.js index 5a76b30ab..00796f1bb 100644 --- a/packages/server/__tests__/lib/grants-collaboration.test.js +++ b/packages/server/__tests__/lib/grants-collaboration.test.js @@ -15,7 +15,7 @@ const emailConstants = require('../../src/lib/email/constants'); use(chaiAsPromised); -afterEach(async () => { +beforeEach(async () => { await seed(knex); }); diff --git a/packages/server/src/scripts/sendGrantActivityDigestEmail.js b/packages/server/src/scripts/sendGrantActivityDigestEmail.js index 7e65661c5..9197fd812 100644 --- a/packages/server/src/scripts/sendGrantActivityDigestEmail.js +++ b/packages/server/src/scripts/sendGrantActivityDigestEmail.js @@ -2,7 +2,7 @@ const tracer = require('dd-trace').init(); const { DateTime } = require('luxon'); const { log } = require('../lib/logging'); -const { buildAndSendGrantActivityDigestEmails } = require('../lib/email'); +const email = require('../lib/email'); const { TZ_NY } = require('../lib/email/constants'); /** @@ -22,7 +22,7 @@ exports.main = async function main() { log.info('Sending grant activity digest emails'); const periodEnd = DateTime.local({ hours: 8, zone: TZ_NY }); const periodStart = periodEnd.minus({ days: 1 }); - await buildAndSendGrantActivityDigestEmails(null, periodStart.toJSDate(), periodEnd.toJSDate()); + await email.buildAndSendGrantActivityDigestEmails(null, periodStart.toJSDate(), periodEnd.toJSDate()); }); }; From 752c5b2564403d99a127c38e8914d5e9b779c73a Mon Sep 17 00:00:00 2001 From: greg-adams Date: Wed, 16 Oct 2024 12:04:55 -0700 Subject: [PATCH 5/7] relocate user info --- packages/server/__tests__/email/email.test.js | 2 +- .../lib/grants-collaboration.test.js | 47 +++++++------- packages/server/src/lib/email.js | 28 +++++---- .../lib/grantsCollaboration/grantActivity.js | 61 +++++++++++-------- 4 files changed, 74 insertions(+), 64 deletions(-) diff --git a/packages/server/__tests__/email/email.test.js b/packages/server/__tests__/email/email.test.js index 78e30d987..540c3324f 100644 --- a/packages/server/__tests__/email/email.test.js +++ b/packages/server/__tests__/email/email.test.js @@ -639,7 +639,7 @@ describe('Email sender', () => { expect([firstEmail.args[0].toAddress, secondEmail.args[0].toAddress]).to.be.members([adminUser.email, staffUser.email]); }); - it('Sends an email for a user\'s saved search', async () => { + it('Sends an email for single user\'s digest', async () => { // Build digest for user following grants await email.buildAndSendGrantActivityDigestEmails(adminUser.id, periodStart, periodEnd); expect(sendFake.calledOnce).to.equal(true); diff --git a/packages/server/__tests__/lib/grants-collaboration.test.js b/packages/server/__tests__/lib/grants-collaboration.test.js index 00796f1bb..339efbe12 100644 --- a/packages/server/__tests__/lib/grants-collaboration.test.js +++ b/packages/server/__tests__/lib/grants-collaboration.test.js @@ -1,6 +1,7 @@ const { expect, use } = require('chai'); const chaiAsPromised = require('chai-as-promised'); const { DateTime } = require('luxon'); +const _ = require('lodash'); const knex = require('../../src/db/connection'); const fixtures = require('../db/seeds/fixtures'); const { seed } = require('../db/seeds/fixtures'); @@ -332,7 +333,9 @@ describe('Grants Collaboration', () => { expect(recipients).to.have.length(2); - expect(recipients).to.have.members([ + const userIds = _.map(recipients, 'userId'); + + expect(userIds).to.have.members([ adminUser.id, staffUser.id, ]); @@ -348,16 +351,14 @@ describe('Grants Collaboration', () => { }); it('retrieves all note/follow activity by period', async () => { - const grantActivity = await getGrantActivityByUserId(knex, adminUser.id, periodStart, periodEnd); - - expect(grantActivity.grants).to.have.lengthOf(2); - expect(grantActivity.userEmail).to.equal(adminUser.email); - expect(grantActivity.userName).to.equal(adminUser.name); - expect(grantActivity.grants[0].grantId).to.equal(grant1.grant_id); - expect(grantActivity.grants[0].notes).to.have.lengthOf(1); - expect(grantActivity.grants[0].follows).to.have.lengthOf(1); - expect(grantActivity.grants[0].notes[0].noteText).to.equal('Staff note'); - expect(grantActivity.grants[0].follows[0].userId).to.equal(staffUser.id); + const grantActivities = await getGrantActivityByUserId(knex, adminUser.id, periodStart, periodEnd); + + expect(grantActivities).to.have.lengthOf(2); + expect(grantActivities[0].grantId).to.equal(grant1.grant_id); + expect(grantActivities[0].notes).to.have.lengthOf(1); + expect(grantActivities[0].follows).to.have.lengthOf(1); + expect(grantActivities[0].notes[0].noteText).to.equal('Staff note'); + expect(grantActivities[0].follows[0].userId).to.equal(staffUser.id); }); it('retrieves email recipients only if OTHER users took action', async () => { @@ -368,23 +369,23 @@ describe('Grants Collaboration', () => { .insert({ grant_note_id: grant1NoteAdmin.id, text: 'Edit for Admin note' }, 'id'); const recipients1 = await getGrantActivityEmailRecipients(knex, periodStart, new Date()); - expect(recipients1).to.have.members([staffUser.id]); + expect(_.map(recipients1, 'userId')).to.have.members([staffUser.id]); // Staff edits note await knex('grant_notes_revisions') .insert({ grant_note_id: grant1NoteStaff.id, text: 'Edit for Staff note' }, 'id'); const recipients2 = await getGrantActivityEmailRecipients(knex, periodStart, new Date()); - expect(recipients2).to.have.members([staffUser.id, adminUser.id]); + expect(_.map(recipients2, 'userId')).to.have.members([staffUser.id, adminUser.id]); }); it('retrieves activity only for OTHER users action', async () => { - const adminGrantActivity = await getGrantActivityByUserId(knex, adminUser.id, periodStart, periodEnd); - const adminActivityIds = getUserIdsForActivities(adminGrantActivity.grants); + const adminGrantActivities = await getGrantActivityByUserId(knex, adminUser.id, periodStart, periodEnd); + const adminActivityIds = getUserIdsForActivities(adminGrantActivities); expect(adminActivityIds).not.to.include(adminUser.id); - const staffGrantActivity = await getGrantActivityByUserId(knex, staffUser.id, periodStart, periodEnd); - const staffActivityIds = getUserIdsForActivities(staffGrantActivity.grants); + const staffGrantActivities = await getGrantActivityByUserId(knex, staffUser.id, periodStart, periodEnd); + const staffActivityIds = getUserIdsForActivities(staffGrantActivities); expect(staffActivityIds).not.to.include(staffUser.id); }); @@ -397,8 +398,8 @@ describe('Grants Collaboration', () => { await Promise.all( [adminUser.id, staffUser.id].map(async (userId) => { - const grantActivity = await getGrantActivityByUserId(knex, userId, periodStart, periodEnd); - expect(grantActivity.grants).to.have.lengthOf(0); + const grantActivities = await getGrantActivityByUserId(knex, userId, periodStart, periodEnd); + expect(grantActivities).to.have.lengthOf(0); }), ); }); @@ -432,7 +433,8 @@ describe('Grants Collaboration', () => { it('Includes recipients from multiple tenants', async () => { // Email recipients INCLUDES multiple tenants - expect(await getGrantActivityEmailRecipients(knex, periodStart, periodEnd)).to.have.members([ + const recipients = await getGrantActivityEmailRecipients(knex, periodStart, periodEnd); + expect(_.map(recipients, 'userId')).to.have.members([ adminUser.id, staffUser.id, otherUser1.id, @@ -441,10 +443,7 @@ describe('Grants Collaboration', () => { }); it('Does NOT include cross-over activity events from multiple tenants', async () => { - const getGrantActivity = async (userId) => { - const grantActivity = await getGrantActivityByUserId(knex, userId, periodStart, periodEnd); - return grantActivity.grants; - }; + const getGrantActivity = async (userId) => getGrantActivityByUserId(knex, userId, periodStart, periodEnd); const tenantOneUsers = [adminUser.id, staffUser.id]; const tenantTwoUsers = [otherUser1.id, otherUser2.id]; diff --git a/packages/server/src/lib/email.js b/packages/server/src/lib/email.js index 99400ffda..ec79c61e0 100644 --- a/packages/server/src/lib/email.js +++ b/packages/server/src/lib/email.js @@ -604,16 +604,18 @@ async function getAndSendGrantForSavedSearch({ } async function getAndSendGrantActivityDigest({ - recipientId, + userId, + userName, + userEmail, periodStart, periodEnd, }) { - const grantActivityResults = await getGrantActivityByUserId(knex, recipientId, periodStart, periodEnd); + const grantActivityResults = await getGrantActivityByUserId(knex, userId, periodStart, periodEnd); return sendGrantActivityDigestEmail({ - name: grantActivityResults.userName, - recipientEmail: grantActivityResults.userEmail, - grants: grantActivityResults.grants, + name: userName, + recipientEmail: userEmail, + grants: grantActivityResults, periodEnd, }); } @@ -643,8 +645,8 @@ async function buildAndSendGrantDigestEmails(userId, openDate = yesterday()) { console.log(`Successfully built and sent grants digest emails for ${inputs.length} saved searches on ${openDate}`); } -async function buildAndSendGrantActivityDigestEmails(userId, periodStart, periodEnd) { - const userGroup = userId ? `user Id ${userId}` : 'all users'; +async function buildAndSendGrantActivityDigestEmails(explicitUserId, periodStart, periodEnd) { + const userGroup = explicitUserId ? `user Id ${explicitUserId}` : 'all users'; console.log(` Building and sending Grant Activity Digest email for ${userGroup} on ${DateTime.fromJSDate(periodEnd).toLocaleString(DateTime.DATE_FULL)} `); @@ -652,15 +654,17 @@ async function buildAndSendGrantActivityDigestEmails(userId, periodStart, period 1. get all email recipients 2. call getAndSendGrantActivityDigest to find activity for each user and send the digest */ - let recipientIds = await getGrantActivityEmailRecipients(knex, periodStart, periodEnd); + let recipients = await getGrantActivityEmailRecipients(knex, periodStart, periodEnd); - if (userId) { + if (explicitUserId) { // Send specific user only if activity exists - recipientIds = recipientIds.filter((recipientId) => recipientId === userId); + recipients = recipients.filter((recipient) => recipient.userId === explicitUserId); } - const inputs = recipientIds.map((recipientId) => ({ - recipientId, + const inputs = recipients.map(({ userId, userName, userEmail }) => ({ + userId, + userName, + userEmail, periodStart, periodEnd, })); diff --git a/packages/server/src/lib/grantsCollaboration/grantActivity.js b/packages/server/src/lib/grantsCollaboration/grantActivity.js index 1520ccb98..d980ad942 100644 --- a/packages/server/src/lib/grantsCollaboration/grantActivity.js +++ b/packages/server/src/lib/grantsCollaboration/grantActivity.js @@ -38,7 +38,11 @@ const getActivitiesQuery = (knex) => { async function getGrantActivityEmailRecipients(knex, periodStart, periodEnd) { const query = knex - .distinct('recipient_followers.user_id AS recipient_user_id') + .select([ + 'recipient_users.id AS recipient_user_id', + 'recipient_users.name AS recipient_user_name', + 'recipient_users.email AS recipient_user_email', + ]) .from( getActivitiesQuery(knex), ) @@ -57,11 +61,20 @@ async function getGrantActivityEmailRecipients(knex, periodStart, periodEnd) { .andWhereRaw('recipient_users.tenant_id = activity_users.tenant_id') // exclude rows where the recipient user is the one taking the action, // to ensure that users only receive a digest if OTHER users took action: - .andWhereRaw('recipient_followers.user_id != activity.user_id'); + .andWhereRaw('recipient_followers.user_id != activity.user_id') + .groupBy([ + 'recipient_user_id', + 'recipient_user_name', + 'recipient_user_email', + ]); const results = await query; - return _.map(results, 'recipient_user_id'); + return results.map((recipient) => ({ + userId: recipient.recipient_user_id, + userEmail: recipient.recipient_user_email, + userName: recipient.recipient_user_name, + })); } async function getGrantActivityByUserId(knex, userId, periodStart, periodEnd) { @@ -109,31 +122,25 @@ async function getGrantActivityByUserId(knex, userId, periodStart, periodEnd) { // Grant IDs distinct const grantIds = [...new Set(_.map(results, 'grant_id'))]; - const result = { - userEmail: results.length ? results[0].recipient_user_email : null, - userName: results.length ? results[0].recipient_user_name : null, - grants: grantIds.map((grantId) => { - const activities = resultsByGrant[grantId].map((act) => ({ - userId: act.user_id, - userName: act.user_name, - userEmail: act.user_email, - agencyName: act.agency_name, - activityAt: act.activity_at, - activityType: act.activity_type, - noteText: act.note_text, - })); - const activitiesByType = _.groupBy(activities, 'activityType'); - - return { - grantId, - grantTitle: resultsByGrant[grantId][0].grant_title, - notes: activitiesByType.note || [], - follows: activitiesByType.follow || [], - }; - }), - }; + return grantIds.map((grantId) => { + const activities = resultsByGrant[grantId].map((act) => ({ + userId: act.user_id, + userName: act.user_name, + userEmail: act.user_email, + agencyName: act.agency_name, + activityAt: act.activity_at, + activityType: act.activity_type, + noteText: act.note_text, + })); + const activitiesByType = _.groupBy(activities, 'activityType'); - return result; + return { + grantId, + grantTitle: resultsByGrant[grantId][0].grant_title, + notes: activitiesByType.note || [], + follows: activitiesByType.follow || [], + }; + }); } module.exports = { From 3ce1e7596212b7a0cf429d4b6cd0074f03978121 Mon Sep 17 00:00:00 2001 From: greg-adams Date: Wed, 16 Oct 2024 12:08:01 -0700 Subject: [PATCH 6/7] debug test --- .../__tests__/lib/grants-collaboration.test.js | 18 +++++++++--------- .../lib/grantsCollaboration/grantActivity.js | 4 ++-- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/packages/server/__tests__/lib/grants-collaboration.test.js b/packages/server/__tests__/lib/grants-collaboration.test.js index 339efbe12..40908b0dd 100644 --- a/packages/server/__tests__/lib/grants-collaboration.test.js +++ b/packages/server/__tests__/lib/grants-collaboration.test.js @@ -286,8 +286,8 @@ describe('Grants Collaboration', () => { let grant1NoteStaff; beforeEach(async () => { - subscribeUser(adminUser); - subscribeUser(staffUser); + await subscribeUser(adminUser); + await subscribeUser(staffUser); periodStart = DateTime.now().minus({ days: 1 }).toJSDate(); @@ -323,7 +323,7 @@ describe('Grants Collaboration', () => { .insert({ grant_id: grant2.grant_id, user_id: staffUser.id }, 'id'); await knex('grant_notes_revisions') - .insert({ grant_note_id: grant2NoteStaff.id, text: 'Staff note' }, 'id'); + .insert({ grant_note_id: grant2NoteStaff.id, text: 'Another Staff note' }, 'id'); periodEnd = new Date(); }); @@ -365,17 +365,17 @@ describe('Grants Collaboration', () => { periodStart = new Date(); // Admin edits note - await knex('grant_notes_revisions') - .insert({ grant_note_id: grant1NoteAdmin.id, text: 'Edit for Admin note' }, 'id'); + const [adminRevised] = await knex('grant_notes_revisions') + .insert({ grant_note_id: grant1NoteAdmin.id, text: 'Edit for Admin note' }, 'created_at'); - const recipients1 = await getGrantActivityEmailRecipients(knex, periodStart, new Date()); + const recipients1 = await getGrantActivityEmailRecipients(knex, periodStart, new Date(adminRevised.created_at.getTime() + 1)); expect(_.map(recipients1, 'userId')).to.have.members([staffUser.id]); // Staff edits note - await knex('grant_notes_revisions') - .insert({ grant_note_id: grant1NoteStaff.id, text: 'Edit for Staff note' }, 'id'); + const [staffRevised] = await knex('grant_notes_revisions') + .insert({ grant_note_id: grant1NoteStaff.id, text: 'Edit for Staff note' }, 'created_at'); - const recipients2 = await getGrantActivityEmailRecipients(knex, periodStart, new Date()); + const recipients2 = await getGrantActivityEmailRecipients(knex, periodStart, new Date(staffRevised.created_at.getTime() + 1)); expect(_.map(recipients2, 'userId')).to.have.members([staffUser.id, adminUser.id]); }); diff --git a/packages/server/src/lib/grantsCollaboration/grantActivity.js b/packages/server/src/lib/grantsCollaboration/grantActivity.js index d980ad942..63aa38fa8 100644 --- a/packages/server/src/lib/grantsCollaboration/grantActivity.js +++ b/packages/server/src/lib/grantsCollaboration/grantActivity.js @@ -54,8 +54,8 @@ async function getGrantActivityEmailRecipients(knex, periodStart, periodEnd) { .on(`recipient_followers.user_id`, '=', `recipient_subscriptions.user_id`) .on(`recipient_subscriptions.notification_type`, '=', knex.raw('?', [emailConstants.notificationType.grantActivity])); }) - .where('activity.activity_at', '>', periodStart) - .andWhere('activity.activity_at', '<', periodEnd) + .where('activity.activity_at', '>=', periodStart) + .andWhere('activity.activity_at', '<=', periodEnd) .andWhere('recipient_subscriptions.status', emailConstants.emailSubscriptionStatus.subscribed) // only consider actions taken by users in the same organization as the recipient: .andWhereRaw('recipient_users.tenant_id = activity_users.tenant_id') From 8cc26fc20d6acabc5dfe5610e9a7c86785f74534 Mon Sep 17 00:00:00 2001 From: greg-adams Date: Wed, 16 Oct 2024 14:40:49 -0700 Subject: [PATCH 7/7] limit prefix length --- terraform/modules/gost_api/grant_activity_digest.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/terraform/modules/gost_api/grant_activity_digest.tf b/terraform/modules/gost_api/grant_activity_digest.tf index 504f2d667..d4d61572b 100644 --- a/terraform/modules/gost_api/grant_activity_digest.tf +++ b/terraform/modules/gost_api/grant_activity_digest.tf @@ -2,7 +2,7 @@ module "grant_activity_digest_scheduled_task" { source = "../scheduled_ecs_task" enabled = var.enabled && var.enable_grant_activity_digest_scheduled_task - name_prefix = "${var.namespace}-grant-activity-digest-task" + name_prefix = "${var.namespace}-grant-activ-digest-task" description = "Executes an ECS task that sends grants activity digest emails at 8am ET" # Schedule