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.

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

Merged
merged 10 commits into from
Sep 29, 2016
14 changes: 6 additions & 8 deletions app/controllers/service_requests_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Copy link
Contributor

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?

Copy link
Contributor Author

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?

@protocol = @service_request.protocol
send_ssr_service_provider_notifications(@service_request, ssr, ssr_deleted=true)
send_ssr_service_provider_notifications(@service_request, ssr, true)
end
ssr.destroy
end
Expand Down Expand Up @@ -545,13 +545,12 @@ def send_admin_notifications(service_request, sub_service_requests)
end
end

def send_ssr_service_provider_notifications(service_request, sub_service_request, ssr_deleted=false) #single sub-service request
def send_ssr_service_provider_notifications(service_request, sub_service_request, all_ssrs_deleted=false) #single sub-service request
previously_submitted_at = service_request.previous_submitted_at.nil? ? Time.now.utc : service_request.previous_submitted_at.utc
audit_report = sub_service_request.audit_report(current_user, previously_submitted_at, Time.now.utc)

sub_service_request.organization.service_providers.where("(`service_providers`.`hold_emails` != 1 OR `service_providers`.`hold_emails` IS NULL)").each do |service_provider|
send_individual_service_provider_notification(service_request, sub_service_request, service_provider, audit_report, ssr_deleted)
end
send_individual_service_provider_notification(service_request, sub_service_request, service_provider, audit_report, all_ssrs_deleted) end
end

def ssr_has_changed?(service_request, sub_service_request) #specific ssr has changed?
Expand All @@ -571,9 +570,8 @@ def service_request_has_changed_ssr?(service_request) #any ssr on sr has changed
return false
end

def send_individual_service_provider_notification(service_request, sub_service_request, service_provider, audit_report=nil, ssr_deleted=false)
def send_individual_service_provider_notification(service_request, sub_service_request, service_provider, audit_report=nil, all_ssrs_deleted=false)
attachments = {}

@service_list_true = @service_request.service_list(true, service_provider)
@service_list_false = @service_request.service_list(false, service_provider)

Expand All @@ -600,7 +598,7 @@ def send_individual_service_provider_notification(service_request, sub_service_r
previously_submitted_at = service_request.previous_submitted_at.nil? ? Time.now.utc : service_request.previous_submitted_at.utc
audit_report = sub_service_request.audit_report(current_user, previously_submitted_at, Time.now.utc)
end
Notifier.notify_service_provider(service_provider, service_request, attachments, current_user, audit_report, ssr_deleted).deliver_now
Notifier.notify_service_provider(service_provider, service_request, attachments, current_user, audit_report, all_ssrs_deleted).deliver_now
end

def send_epic_notification_for_user_approval(protocol)
Expand Down
28 changes: 15 additions & 13 deletions app/mailers/notifier.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,15 @@ def notify_admin(service_request, submission_email_address, xls, user_current, s
mail(:to => email, :from => NO_REPLY_FROM, :subject => subject)
end

def notify_service_provider(service_provider, service_request, attachments_to_add, user_current, audit_report=nil, ssr_deleted=false)
def notify_service_provider(service_provider, service_request, attachments_to_add, user_current, audit_report=nil, all_ssrs_deleted=false)
@notes = service_request.notes
@status = service_request.status

if all_ssrs_deleted
Copy link
Contributor

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

@status = 'all_ssrs_deleted'
else
@status = service_request.status
end

@role = 'none'
@full_name = service_provider.identity.full_name

Expand All @@ -100,22 +106,18 @@ def notify_service_provider(service_provider, service_request, attachments_to_ad
@service_requester_id = @service_request.sub_service_requests.first.service_requester_id

@audit_report = audit_report
@ssr_deleted = ssr_deleted

@portal_link = DASHBOARD_LINK + "/protocols/#{@protocol.id}"
@portal_text = "Administrators/Service Providers, Click Here"

# if the current user is service provider, only show SSR's that are associated with them
@ssrs_to_be_displayed = []
@service_request.sub_service_requests.each do |ssr|
if service_provider.identity.is_service_provider?(ssr)
@ssrs_to_be_displayed << ssr
end
end
# only display the ssrs that are associated with service_provider
@ssrs_to_be_displayed = @service_request.ssrs_associated_with_service_provider(service_provider)

attachments_to_add.each do |file_name, document|
next if document.nil?
attachments["#{file_name}"] = document
if !all_ssrs_deleted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unless

attachments_to_add.each do |file_name, document|
next if document.nil?
attachments["#{file_name}"] = document
end
end

# only send these to the correct person in the production env
Expand Down
10 changes: 10 additions & 0 deletions app/models/service_request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Copy link
Contributor

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.

Copy link
Contributor

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.

Copy link
Contributor Author

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!

ssrs_to_be_displayed = []
self.sub_service_requests.each do |ssr|
if service_provider.identity.is_service_provider?(ssr)
ssrs_to_be_displayed << ssr
end
end
ssrs_to_be_displayed
end

private

def set_original_submitted_date
Expand Down
6 changes: 6 additions & 0 deletions app/views/notifier/_deleted_all_services_from_cart.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
%p
= I18n.t('notifier.body1', :full_name => @full_name)
%p.inline
= I18n.t('notifier.deleted_all_services_from_cart', protocol_type: @protocol.type)
%br
%br
26 changes: 0 additions & 26 deletions app/views/notifier/_deleted_services.html.haml

This file was deleted.

17 changes: 17 additions & 0 deletions app/views/notifier/_deleted_srid_information.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
%table.table{cellpadding: 3}
%thead
%tr.skinny-black-border
%td{:colspan => 2}
%strong= t(:notifier)[:srid_info]
%tr
%th.skinny-black-border= t(:notifier)[:sr_id_acronym]
%th.skinny-black-border= t(:notifier)[:organization]
%tbody
- @ssrs_to_be_displayed.each do |ssr|
%tr.skinny-black-border
%td.skinny-black-border.center
%strike= ssr.display_id
%td.skinny-black-border.center
%strike= ssr.org_tree_display
%br
%br
34 changes: 25 additions & 9 deletions app/views/notifier/_notification_email.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,43 @@
%title= t(:notifier)[:service_request]

%body
- if @status == 'get_a_cost_estimate'
/***** EMAIL INTRO *****
- case @status
- when 'all_ssrs_deleted'
= render "deleted_all_services_from_cart"
- when 'get_a_cost_estimate'
= render "welcome"
- if @status == 'submitted'
- when 'submitted'
= render "welcome_for_submitted_status"
/***** END EMAIL INTRO *****

/***** EMAIL TABLES *****
= render "protocol_information", protocol: @protocol

- if @service_request.has_per_patient_per_visit_services? and @service_request.arms.count > 0
= render "arm_information"

= render "user_information"
- if @ssr_deleted
= render "deleted_services"
- elsif @audit_report.present? && @audit_report[:line_items].present?
= render "audit_action"

- if @ssrs_to_be_displayed
= render "srid_information"
- if @status == 'all_ssrs_deleted'
= render "deleted_srid_information"
- else
= render "srid_information"

- if @status != 'all_ssrs_deleted' && @audit_report.present? && @audit_report[:line_items].present?
= render "audit_action"
/***** END EMAIL TABLES *****

/***** OTHER EMAIL TIDBITS *****
- if @status == "submitted" && @role == 'none' && !@notes.empty?
%p= t(:notifier)[:notes_present]

%p= t(:notifier)[:body6]
- if @status != 'all_ssrs_deleted'
%p= t(:notifier)[:body6]

%p= t(:issue_contact)

- if @role != 'none'
= render "acknowledgments"
= render "acknowledgments"
/***** END OTHER EMAIL TIDBITS *****
4 changes: 2 additions & 2 deletions app/views/notifier/_user_information.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
%th.skinny-black-border= t(:notifier)[:name]
%th.skinny-black-border= t(:notifier)[:contact_info]
%th.skinny-black-border= t(:notifier)[:role]
- if @protocol.selected_for_epic == true
- if @protocol.selected_for_epic
%th.skinny-black-border= t(:notifier)[:epic_access]
%tbody
- @protocol.project_roles.each do |pr|
Expand All @@ -38,7 +38,7 @@
%td.skinny-black-border.center= pr.role.upcase + " " + t(:notifier)[:requester]
- else
%td.skinny-black-border.center= pr.role.upcase
- if @protocol.selected_for_epic == true
- if @protocol.selected_for_epic
%td.skinny-black-border.center= pr.epic_access == true ? "Yes" : "No"
%br
%br
4 changes: 2 additions & 2 deletions app/views/user_mailer/_user_info_table.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,13 @@
%th{:style => 'border: 1px solid black;'}= t(:notifier)[:contact_info]
%th{:style => 'border: 1px solid black;'}= t(:notifier)[:role]
%th{:style => 'border: 1px solid black;'}= t(:mailer)[:proxy]
- if @protocol.selected_for_epic == true
- if @protocol.selected_for_epic
%th{:style => 'border: 1px solid black;'}= t(:notifier)[:epic_access]
%tbody
%tr{:style => 'border: 1px solid black;'}
%td{:style => 'border: 1px solid black; text-align: center;'}= @modified_role.identity.full_name
%td{:style => 'border: 1px solid black; text-align: center;'}= @modified_role.identity.email
%td{:style => 'border: 1px solid black; text-align: center;'}= @modified_role.role.upcase
%td{:style => 'border: 1px solid black; text-align: center;'}= @modified_role.display_rights
- if @protocol.selected_for_epic == true
- if @protocol.selected_for_epic
%td{:style => 'border: 1px solid black; text-align: center;'}= @modified_role.epic_access == true ? "Yes" : "No"
2 changes: 1 addition & 1 deletion config/locales/en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -1632,7 +1632,6 @@ en:
notifier:
action: "Action"
admin_data: "Admin Information"
all_on_request: "All Services on Request Have Been Deleted"
arm_data: "Protocol Arm Information"
arm_name: "Arm Name"
arm_subject_count: "Subject Count"
Expand All @@ -1641,6 +1640,7 @@ en:
body6: "A list of requested services is attached."
body7: "To approve the charges associated with the selected services please click"
contact_info: "Contact Information"
deleted_all_services_from_cart: "All services have been deleted in SPARCRequest for the %{protocol_type} below to which you have been granted access."
epic_access: "Epic Access"
link: "HERE"
name: "User Name"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,8 @@
let!(:service2) { service = create( :service, organization_id: core.id) }
let!(:service3) { service = create( :service, organization_id: core2.id) }

let!(:ssr1) { create(:sub_service_request, service_request_id: service_request.id, organization_id: core.id) }
let!(:ssr2) { create(:sub_service_request, service_request_id: service_request.id, organization_id: core2.id) }
let!(:ssr1) { create(:sub_service_request, service_request_id: service_request.id, organization_id: core.id, submitted_at: Time.now) }
let!(:ssr2) { create(:sub_service_request, service_request_id: service_request.id, organization_id: core2.id, submitted_at: Time.now) }

let!(:line_item1) { create(:line_item, service_id: service1.id, service_request_id: service_request.id, sub_service_request_id: ssr1.id) }
let!(:line_item2) { create(:line_item, service_id: service2.id, service_request_id: service_request.id, sub_service_request_id: ssr1.id) }
Expand Down Expand Up @@ -221,17 +221,20 @@
}.to raise_error(ActiveRecord::RecordNotFound)
end

context 'ServiceRequest not in draft or first_draft and has been submitted' do
context 'SSR has been previously submitted' do

before(:each) { service_request.update_attribute(:status, 'not_draft') }

context 'removed SubServiceRequest created before submit time' do
before(:each) { sub_service_request.update_attribute(:submitted_at, Time.now) }

context 'removed all SSRs' do
before :each do
line_item.destroy
line_item1.destroy
line_item2.destroy
end

it 'should send notifications to the service provider' do
service_request.update_attribute(:submitted_at, Time.zone.now)
ssr2.update_attribute(:created_at, Time.zone.now.ago(60))

expect(controller).to receive(:send_ssr_service_provider_notifications)
expect(controller).to receive(:send_ssr_service_provider_notifications).thrice
post :remove_service, {
:id => service_request.id,
:service_id => service3.id,
Expand All @@ -241,20 +244,30 @@
end
end

context 'removed SubServiceRequest created after submit time' do
context 'removed one SSR' do
it 'should not send notifications to the service_provider' do
expect(controller).not_to receive(:send_ssr_service_provider_notifications)
post :remove_service, {
:id => service_request.id,
:service_id => service3.id,
:line_item_id => line_item3.id,
:format => :js,
}.with_indifferent_access
end
end
end

it 'should not send notifications to the service provider' do
service_request.update_attribute(:submitted_at, Time.zone.now.ago(60))
ssr2.update_attribute(:created_at, Time.zone.now)
context 'SSR has not been previously submitted' do
before(:each) { sub_service_request.update_attribute(:submitted_at, nil) }

expect(controller).to_not receive(:send_ssr_service_provider_notifications)
post :remove_service, {
it 'should not send notifications to the service_provider' do
expect(controller).not_to receive(:send_ssr_service_provider_notifications)
post :remove_service, {
:id => service_request.id,
:service_id => service1.id,
:line_item_id => line_item1.id,
:service_id => service3.id,
:line_item_id => line_item3.id,
:format => :js,
}.with_indifferent_access
end
end
end

Expand Down