Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
4175 add request type flag #4271
4175 add request type flag #4271
Changes from 21 commits
35c4e27
d1911fe
3007e64
7ae36b4
a2c79df
7e17565
9b5977e
854cd39
8435518
45a9495
4791021
3866a8f
8a775ec
a037b1c
26afeb4
5c2997b
0cd4451
94a1be4
42f1f84
c6e45c8
0cfddca
67e9d71
d8e9806
1e19d30
43f7365
c1792ac
6441532
a27b4e6
d53785c
56b7c04
36cb9b2
41a0429
c0af65c
f46736e
656e712
3a0e841
389e6d9
c1acb98
15ecc79
9c0572b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Wait, why was this put back? We agreed that it wouldn't be needed because the service itself knows what type it is just by the current parameters being passed in.
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.
This service should already know the request type just by whether
for_families
is true or false.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.
I feel that defining request_type in this way is more transparent. Would you prefer to add logic in the controller to determine request_type rather than using the instance var?
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.
I'd prefer putting the business logic as far back as possible. The controller is calling a service, which already has to know what kind of request it's creating. We can rely on the service to set the request type.