-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat(condo): DOMA-10042 handler for suggesting providers #5163
Conversation
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.
You need to use Service Provider instead of provider
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.test.js
Show resolved
Hide resolved
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.
Overall looks OK, got some code-style issues, which can be easily resolved...
But I've also got some serious issues, like:
- Ordering logic
- Redis keys naming convention
- TIN duplicates display
- Server utils usage
So I cannot pass it for now, but code itself is fine
eba67bd
to
e5a050a
Compare
…hing name case-insensitive
…more grouping for tests
e5a050a
to
569db8d
Compare
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.
Redis Keys must be changed
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
apps/condo/domains/resident/schema/SuggestServiceProviderService.js
Outdated
Show resolved
Hide resolved
Quality Gate passedIssues Measures |
Added handler for suggesting providers by tin or name