DMPbelgium / roadmap

5 stars 1 forks source link

feedback requests should probably be sent from Rails.configuration.x.organisation.do_not_reply_email #91

Open StCyr opened 1 year ago

StCyr commented 1 year ago

Please complete the following fields as applicable:

What version of the DMPRoadmap code are you running? (e.g. v2.2.0)

dmponline-prod

Expected behaviour:

replies to feedback request emails should not be sent to servicedesk@belnet.be

Actual behaviour:

replies to feedback request emails are sent to servicedesk@belnet.be

Steps to reproduce:

  1. request a feedback => an email is sent to the organisation's data management staff
  2. see that the from field is servicedesk@belnet.be
  3. confirm that if the organisation's data management staff replies to this email, the reply will be sent to servicedesk@belnet.be
StCyr commented 1 year ago

Sending of this email probably appears in UserMailer::feedback_notification:

  def feedback_notification(recipient, plan, requestor)
    return unless recipient.active?

    @user           = requestor
    @plan           = plan
    @recipient      = recipient
    @recipient_name = @recipient.name(false)
    @requestor_name = @user.name(false)
    @plan_name      = @plan.title
    @helpdesk_email = helpdesk_email(org: @plan.org)

    I18n.with_locale I18n.default_locale do
      mail(to: @recipient.email,
           subject: format(_('%{user_name} has requested feedback on a %{tool_name} plan'),
                           tool_name: tool_name, user_name: @user.name(false)))
    end
  end

  # rubocop:disable Metrics/AbcSize
  def feedback_complete(recipient, plan, requestor)
    return unless recipient.active?

    @requestor_name = requestor.name(false)
    @user           = recipient
    @recipient_name = @user.name(false)
    @plan           = plan
    @phase          = @plan.phases.first
    @plan_name      = @plan.title
    @helpdesk_email = helpdesk_email(org: @plan.org)

    I18n.with_locale I18n.default_locale do
      sender = Rails.configuration.x.organisation.do_not_reply_email ||
               Rails.configuration.x.organisation.email

      mail(to: recipient.email,
           from: sender,
           subject: format(_('%{tool_name}: Expert feedback has been provided for %{plan_title}'),
                           tool_name: tool_name, plan_title: @plan.title))
    end
  end

we should maybe replace:

      mail(to: @recipient.email,
           subject: format(_('%{user_name} has requested feedback on a %{tool_name} plan'),
                           tool_name: tool_name, user_name: @user.name(false)))

by

      mail(to: @recipient.email,
           from: Rails.configuration.x.organisation.do_not_reply_email,
           subject: format(_('%{user_name} has requested feedback on a %{tool_name} plan'),
                           tool_name: tool_name, user_name: @user.name(false)))
nicolasfranck commented 1 year ago

@StCyr which should work if that value was set to a different value.

See https://github.com/DMPbelgium/roadmap/blob/master/app/mailers/user_mailer.rb#L130 where it either chooses the do_not_reply_email or the regular email when that previous one is not present.

I see here https://github.com/DMPbelgium/roadmap/blob/master/config/initializers/_dmproadmap_ugent.rb#L37 that the values are the same for both situations.

Anyway, do you want your users to reply back? All of the mails are sent by a bot ...

nicolasfranck commented 1 year ago

Point taken: https://github.com/DMPbelgium/roadmap/blob/master/app/mailers/user_mailer.rb#L99 -> method feedback_notification does not use that email address..