Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Jump to bottom

Implement find capital projects by community district endpoint #327

Merged
merged 1 commit into from
Jul 5, 2024
Merged

Implement find capital projects by community district endpoint #327

merged 1 commit into from
Jul 5, 2024

Conversation

pratishta
Copy link
Contributor

@pratishta pratishta commented Jun 18, 2024 •
edited
Loading

Closes #247

Several things to note:

  • Wasn't sure where to place the check for community district ID. First commit (a2a9cd8) has it in borough and second commit (bda6221) has it in its own domain community-district
  • This endpoint is the first one where we're passing in both path and query params. I pass the ZodTransformPipe directly in the pram decorators because adding both param schemas in UsePipes() was causing errors.

@pratishta pratishta force-pushed the 247-implement-findCapitalProjectsByCommunityDistrict-endpoint branch 4 times, most recently from 4d1dcd6 to 08b2142 Compare June 18, 2024 22:44
@pratishta
Copy link
Contributor Author

pratishta commented Jun 18, 2024 •
edited
Loading

I rebased off main with the Kubb upgrade changes. There are some things I did not have time for:
- More robust testing checking for query params e.g. default vs specified
- Checking for community district ID... I wasn't sure if we wanted to have that function be dependent on checking the borough ID or if they should both be independent. I chose independent in this PR and this affects the repository mock functionality as well. Addressed in description
- Adding the ZodTransform pipe to boroughs.controller.ts... Adding the the new pipe causes some type errors in the param I couldn't quite figure out Addressed in description

@pratishta pratishta mentioned this pull request Jun 18, 2024
Closed
4 tasks
@pratishta pratishta force-pushed the 247-implement-findCapitalProjectsByCommunityDistrict-endpoint branch 3 times, most recently from 233b621 to bda6221 Compare June 26, 2024 16:43
@pratishta pratishta marked this pull request as ready for review June 26, 2024 17:04
@pratishta pratishta force-pushed the 247-implement-findCapitalProjectsByCommunityDistrict-endpoint branch from bda6221 to 1a3446b Compare June 26, 2024 18:45
TangoYankee

This comment was marked as resolved.

Sign in to view

TangoYankee

This comment was marked as resolved.

Sign in to view

@pratishta pratishta force-pushed the 247-implement-findCapitalProjectsByCommunityDistrict-endpoint branch from 1a3446b to a796442 Compare July 5, 2024 15:02
@TangoYankee TangoYankee mentioned this pull request Jul 5, 2024
Open
TangoYankee
TangoYankee requested changes Jul 5, 2024
View reviewed changes
src/schema/community-district.ts Show resolved Hide resolved
src/borough/borough.controller.ts Show resolved Hide resolved
@pratishta
Implement find capital projects by community district
813d0a5 
Flesh out repository

Generated zod schema can't be nullable

Remove WIP emoji

Comment out pipe to get capital projects

Add unit test

Add e2e test

Fix type errors

Add index

Clean up

Add pipe validation

Use transform pipe in param and query decorator

Add tests for query params

Move check community district id to community district domain

Fix unit tests

Get community district mock for borough repository mock

Use dependency injection to inject community district mock into borough mock

Use dependency injection to inject community district mock into borough mock

Expand unit test

Generate migration
@pratishta pratishta force-pushed the 247-implement-findCapitalProjectsByCommunityDistrict-endpoint branch from a796442 to 813d0a5 Compare July 5, 2024 18:09
@pratishta pratishta merged commit 4ecdb0b into main Jul 5, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TangoYankee TangoYankee TangoYankee approved these changes

@TylerMatteo TylerMatteo Awaiting requested review from TylerMatteo

@horatiorosa horatiorosa Awaiting requested review from horatiorosa

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement findCapitalProjectsByBoroughIdCommunityDistrictId endpoint
2 participants
@pratishta @TangoYankee