-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add initial filtering capability and tests to listings service #18
Conversation
…ed repository to enable mocking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
And thanks for adding the test scaffolding!
One other question here: should the tests live in |
@anders-schneider the other unit tests are in the same directory as the code under test, for example: https://github.com/CityOfDetroit/bloom/blob/main/backend/core/src/user/user.controller.spec.ts |
Ah sorry I missed that, sounds good on including unit tests in the same directory like you have it! |
|
||
@Injectable() | ||
export class ListingsService { | ||
constructor(@InjectRepository(Listing) private readonly repository: Repository<Listing>) {} | ||
|
||
private getQueryBuilder() { | ||
return Listing.createQueryBuilder("listings") | ||
return this.repository |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the switch here and elsewhere to use this.repository instead of Listing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Listing
is using the entity object directly, but this.repository
is the injected entity. That lets me swap it with a mock in the tests.
Issue
CityOfDetroit/affordable-housing-app#63
Addresses # (issue)
Description
Adds an initial filtering capability to the listings service, and some basic tests
Details
Type of change
How Can This Be Tested/Reviewed?
Please describe the tests that you ran to verify your changes. Provide instructions so we can review. Please also list any relevant details for your test configuration
yarn test -t "listings"
to run the new testsselect neighborhood, count(*) from property group by neighborhood;
in the db to see them all)Checklist: