Skip to content

feat: add POST /coordinators endpoint with Cognito user creation - #9

Open
pujitakalinadhabhotla wants to merge 1 commit into
mainfrom
mspca-19-create-coordinator-endpoint
Open

pujitakalinadhabhotla wants to merge 1 commit into
mainfrom
mspca-19-create-coordinator-endpoint

Conversation

@pujitakalinadhabhotla

Copy link
Copy Markdown

ℹ️ Issue

Closes MSPCA-19

📝 Description

Adds a POST /coordinators endpoint to create Foster Coordinators (MSPCA staff accounts, separate from volunteers).

  1. Validates required fields and rejects emails that don't end in @mspca.org.
  2. Registers the user in Cognito (mspca-user-pool) and adds them to the FosterCoordinator group.
  3. Only creates the Postgres row after Cognito succeeds, so a failed Cognito call never leaves behind an orphaned coordinator.
  4. Stores the Cognito sub on a new cognito_sub column (migration included, defaults to '').

Also handles duplicate emails and missing/invalid fields, and includes service + controller tests with Cognito calls mocked.

✔️ Verification

  1. Ran the migration and confirmed cognito_sub was added to foster_coordinators.
  2. Ran yarn test — all tests passing.
  3. Manually hit POST /coordinators and confirmed valid coordinators are created with a cognito_sub, invalid emails are rejected, and duplicates are rejected without creating a row.

(Backend-only change, no screenshots.)

🏕️ (Optional) Future Work / Notes

@dburkhart07
dburkhart07 self-requested a review October 6, 2026 06:33

@dburkhart07 dburkhart07 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we comment out the lines 90-95 in the cognito guard, so that we can enable auth (and therefore have access to cognito), the guard runs, but skips token verification?

@IsString()
lastName!: string;

@IsString()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

both of these need @IsPhoneNumber('US')


// Accepts every FosterCoordinator field except `active` (defaults to true) and
// `assignedVolunteers` (defaults to []), which are not client-settable on create.
export class CreateCoordinatorDto {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

For all the strings can we add a MaxLength using the coordinator entity for the exact number of characters each should be? Phone number doesn't need a max length

let controller: CoordinatorsController;
let service: jest.Mocked<CoordinatorsService>;

const dto: CreateCoordinatorDto = {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we cast this with as CreateCoordinatorDto so that, even if it's missing a field, it won't throw an error?


it('should be defined', () => {
expect(controller).toBeDefined();
it('delegates creation to the service and returns the created coordinator', async () => {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we add tests for error propagation for each specific error that may be thrown? so bad request and conflict?

private cognitoService: CognitoService,
) {}

/**

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

add a parameter for the body

const coordinator = this.repo.create({
...dto,
active: true,
assignedVolunteers: [],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

can we initialize this list to be all foster volunteers that have the same homebase as the coordinator?

})
secondaryPhone!: string | null;

@Column({ type: 'varchar', length: 255 })

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

sorry, this is an additional request that came to me as i was reviewing this pr, but can we make it so that the email fields for coordinators, volunteers, and admin are all unique? this will require an entity change in each, and a migration to write on top of it. we can honestly just add this into the migration you've already written in this pr.

export class CoordinatorsController {
constructor(private coordinatorsService: CoordinatorsService) {}

@Post()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Lets add a @public() decorator to this since everyone is going to need to have access to it when we eventually enable auth fully

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants