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
Jw email part 5 request deletion #687
Conversation
@@ -598,6 +598,16 @@ def has_non_first_draft_ssrs? | |||
sub_service_requests.where.not(status: 'first_draft').any? | |||
end | |||
|
|||
def ssrs_associated_with_service_provider (service_provider) |
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 think if you used #map here, you could condense this method a bit. Map will create an array and return it. So you wouldn't need to instantiate an empty array.
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.
Could be a cleaner solution, but honestly it is just a different way of doing it. Fine if we keep it how it is.
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'll make a note to look at this later! I definitely have some refactoring to do once all the different email scenarios are done. Thanks for the input!
@@ -437,9 +437,9 @@ def remove_service | |||
to_delete = @service_request.sub_service_requests.map(&:organization_id) - @service_request.service_list.keys | |||
to_delete.each do |org_id| | |||
ssr = @service_request.sub_service_requests.find_by_organization_id(org_id) | |||
if !['first_draft', 'draft'].include?(@service_request.status) and !@service_request.submitted_at.nil? and @service_request.submitted_at > ssr.created_at | |||
if @service_request.line_items.count.zero? && !ssr.submitted_at.nil? |
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 a pretty hefty change. Can you explain why we're now only looking at line items count?
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.
Yeah! Trigger: When a user goes back to a protocol through "Edit" or "Modify Request" button, and delete all the services in an SSR from his/her shopping cart, this email should be send out to the service provider(s) when the last service on a SSR has been deleted from the SPARCRequest shopping cart. Before it would send out on each removed service, but Wenjun wanted that changed. Here's the link to the story: https://www.pivotaltracker.com/story/show/130746829. If you look at Wenjun's last comment, she doesn't want the logic to rely on the status anymore, only submitted_at. Does that make sense?
@notes = service_request.notes | ||
@status = service_request.status | ||
|
||
if all_ssrs_deleted |
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.
@status = all_ssrs_deleted ? 'all_ssrs_deleted' : service_request.status
attachments_to_add.each do |file_name, document| | ||
next if document.nil? | ||
attachments["#{file_name}"] = document | ||
if !all_ssrs_deleted |
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.
unless
See link for description: https://www.pivotaltracker.com/story/show/130746829