DCC Feature #345 - Allow Orgs to set a different Help Desk Email Address #3140
Conversation
johnpinto1
force-pushed
the
dcc_bug_345_customising_helpdesk_email_property
branch
from
f0dc7ad to
0285039
Compare
|
@briri Not sure why Postgresql Tests pass, but MySQL Tests fail. |
app/helpers/mailer_helper.rb
Outdated
| @@ -8,8 +8,12 @@ def tool_name | |||
| @tool_name ||= ApplicationService.application_name | |||
| end | |||
|
|
|||
| def helpdesk_email | |||
| @helpdesk_email ||= Rails.configuration.x.organisation.helpdesk_email | |||
| def helpdesk_email(org = nil) | |||
There was a problem hiding this comment.
prefer named attribute and no if/else block since we don't need to make it an instance variable here, that is being done in the mailer. Something like:
def helpdesk_email(org: nil)
org&.helpdesk_email || Rails.configuration.x.organisation.helpdesk_email
end
There was a problem hiding this comment.
Updated as suggested.
| inviter_name = @resource.invited_by.name | ||
| inviter = @resource.invited_by | ||
| inviter_name = inviter.name | ||
| helpdesk_email = inviter.org.present? && |
There was a problem hiding this comment.
Why not just call the helper method here?
There was a problem hiding this comment.
Updated without helper method helpdesk_email() which is not accessible in Devise views.
| <p> | ||
| <%= _('Hello %{user_name}') %{ :user_name => user_name } %> | ||
| <%= _("Hello %{user_name}"") %{ :user_name => user_name } %> |
There was a problem hiding this comment.
looks like an extra double quote here
There was a problem hiding this comment.
Updated and removed extra double quote.
| @@ -3,6 +3,11 @@ | |||
| helpdesk_email = Rails.configuration.x.organisation.helpdesk_email | |||
| contact_us = Rails.configuration.x.organisation.contact_us_url || contact_us_url | |||
| email_subject = _('Query or feedback related to %{tool_name}') %{ :tool_name => tool_name } | |||
| user = User.find_by(email: @resource.email) | |||
| helpdesk_email = user.org.present? && | |||
There was a problem hiding this comment.
Should be able to use the helper method here
There was a problem hiding this comment.
@briri How do I include MailerHelper in Devise views?
I tried but got error "undefined method `include' for #<#Class:0x00007fb1cd01b310:0x00007fb1c0e9e750>"
<%
include MailerHelper
helper MailerHelper
tool_name = ApplicationService.application_name
helpdesk_email = Rails.configuration.x.organisation.helpdesk_email
contact_us = Rails.configuration.x.organisation.contact_us_url || contact_us_url
email_subject = _('Query or feedback related to %{tool_name}') %{ :tool_name => tool_name }
user = User.find_by(email: @resource.email)
helpdesk_email = helpdesk_email(user.org)
%>
There was a problem hiding this comment.
Oh interesting. Maybe the gem isn't able to access the application's methods. Ok, leave those Devise views as you have them now and maybe just add a comment to the helper noting that any changes made there should also be made in the Devise views
db/schema.rb
Outdated
| t.index ["question_id"], name: "index_annotations_on_question_id" | ||
| t.index ["versionable_id"], name: "index_annotations_on_versionable_id" | ||
| end | ||
|
|
||
| create_table "answers", id: :integer, force: :cascade do |t| | ||
| create_table "answers", id: :serial, force: :cascade do |t| |
There was a problem hiding this comment.
Looks like some extra schema changes leaked into this PR. Just add the new helpdesk_email column to the Orgs table
There was a problem hiding this comment.
That already exists
create_table "orgs", id: :serial, force: :cascade do |t|
t.string "name"
t.string "abbreviation"
t.string "target_url"
t.datetime "created_at", null: false
t.datetime "updated_at", null: false
t.boolean "is_other", default: false, null: false
t.integer "region_id"
t.integer "language_id"
t.string "logo_uid"
t.string "logo_name"
t.string "contact_email"
t.integer "org_type", default: 0, null: false
t.text "links"
t.string "contact_name"
t.boolean "feedback_enabled", default: false
t.text "feedback_msg"
t.boolean "managed", default: false, null: false
t.string "helpdesk_email"
end
There was a problem hiding this comment.
Reverted create_table "answers", id: :serial, force: :cascade do |t| to
create_table "answers", id: :integer, force: :cascade do |t|.
johnpinto1
force-pushed
the
dcc_bug_345_customising_helpdesk_email_property
branch
2 times, most recently
from
d7c6763 to
c742fb3
Compare
…ress
from the default address.
Feature asked for by DCC https://github.com/DigitalCurationCentre/DMPonline-Service/issues/345
Changes:
- added helpdesk_email attribute to Org model with migration.
- added a Help Desk email field to
app/views/orgs/_profile_form.html.erb.
- added a method helpdesk_mail(org = nil) to app/helpers/mailer_helper.rb.
- added a variable @helpdesk_email to each method in app/mailers/user_mailer.rb.
- updated app/views/user_mailer/_email_signature.html.erb to accept
@helpdesk_mail variable.
- altered helpdesk_email variable in Devise email fragments to use Org
helpdesk_email if present.
johnpinto1
force-pushed
the
dcc_bug_345_customising_helpdesk_email_property
branch
from
c742fb3 to
7eae937
Compare
|
Looks like Migrations must be run on Test dbs. |
Updated schema.rb to have the timestamp of the new migration
|
@johnpinto1 I fixed the issue with the schema.rb and also made a few additional tweaks to get the tests passing |
from the default address.
Feature asked for by DCC https://github.com/DigitalCurationCentre/DMPonline-Service/issues/345
Changes:
app/views/orgs/_profile_form.html.erb.
@helpdesk_mail variable.
helpdesk_email if present.
Screenshots:
Org Profile Page with new field helpdesk_email set
Emails without Org WITHOUT helpdesk_email set
Emails without Org WITH helpdesk_email set = [email protected]
