[Bug]: When Grant is assigned to a team, all teams assigned receive notification

[ClaireValdivia]

Why is this issue important?

Assignment notifications are not sending correctly and will be confusing to users

Current State

Steps to recreate: Two users from two different teams, both have notifications enabled - one belongs to team USDR and one belongs to team Claire Test.

• Assign grant to team USDR - the user in this team recieves a notification of the assignment
• Assign the same grant to team Claire Test - both users from both teams will receive assignment email
• For every new assignment made, every person who has be previously assigned the grant will receive another notification

Expected State

A user will only receive a notification when their team is assigned, not when another team is assigned

Implementation Plan

The issue happens because when the front-end makes a request to the backend to assign agencies, it does not explicitly provide which agencies were just added.

There could be two ways to solve this problem:

1. Update the front-end to only send the agencyIds that were added to the /assign/agencies endpoint.

   • Note: this is not @as1729's preferred option however, if you think this is easier to implement go for it. Otherwise feel free to implement option two laid out below.

2. Update how the backend interprets the results when inserting records in the grants_assigned_agencies table. https://github.com/usdigitalresponse/usdr-gost/blob/main/packages/server/src/routes/grants.js#L362

First update the return value of the assignGrantsToAgencies function.

// @return List[int] : this function returns a list of integers which map to agency_id field.
//                                the function will only return values that have been inserted.
function assignGrantsToAgencies({ grantId, agencyIds, userId }) {
    const insertPayload = agencyIds.map((aId) => ({
        ...
    }));
    return knex(TABLES.assigned_grants_agency)
        // Add the `agency_id` here as part of the return value see details here: https://knexjs.org/guide/query-builder.html#insert
        .insert(insertPayload, ['agency_id'])
        .onConflict(['agency_id', 'grant_id'])
        .ignore();
}

Next change what value is passed into sendGrantAssignedEmails now that the return value of the above function has changed:

router.put('/:grantId/assign/agencies', requireUser, async (req, res) => {
    ...
    // verify the knex insert documentation to make sure you're getting the correct return values: https://knexjs.org/guide/query-builder.html#insert
    // but from my understanding with the changes made it should return a list of agency_ids of only the newly created rows
    const addedAgencyIds = await db.assignGrantsToAgencies({ grantId, agencyIds, userId: user.id });
    try {
        await email.sendGrantAssignedEmails({ grantId, addedAgencyIds, userId: user.id });
    } catch {
        ...
    }

    res.json({});
});

Relevant Code Snippets

No response

[jeffsmohan]

@as1729 Personally, I found the API design here unexpected, which threw me off the case a bit this morning. Specifically, I would normally expect to find an endpoint like /:grantId/assign/agencies to behave something like this:

• GET: returns the list of assigned agencies for the grant (current behavior)
• POST: expects a single agency_id to add to the list of assigned agencies (NEW endpoint)
• PUT: expects an agency_ids array and resets the list of assigned agencies to the supplied list (existing endpoint, but CHANGED behavior — the current endpoint effectively does a set union between the existing assignments and the provided agency_ids, which feels like unexpected behavior)
• DELETE: expects a single agency_id to remove from the list of assigned agencies (current behavior)
• [note that there's no PUT action, which would be a change]

And fwiw, I don't know of any good current use cases for the PUT behavior above, so I would fully remove it. Instead, when a user assigns a grant, the frontend should hit the POST endpoint described above with just the single added agency ID. Then we can easily have the server trigger emails just for that given agency ID as part of the route handler for that endpoint.

So to summarize, I would propose a third implementation path:

1. Add a POST /:grantId/assign/agencies endpoint that expects a single agency ID, assigns it, and triggers emails just for that agency
2. Update the frontend to hit this endpoint instead of the existing PUT endpoint
3. Remove the PUT endpoint

What do you think?

[as1729]

This is definitely a cleaner implementation rather than building on an already confusing existing api. Let's go with your approach. Thank you!

[ClaireValdivia]

This is working as expected in staging! Assigned a grant to two teams, and users only received the notification when assigned to their team.