From 5aa20dc4079dff7785c2742f0be6cbf05b9228d0 Mon Sep 17 00:00:00 2001 From: bhavi Date: Tue, 24 Oct 2017 12:59:39 -0500 Subject: [PATCH] Added Feedback Request to Org Details Initial form changes for the New Org Details added new db components for the org profile tab split org page into tabs and updated profile form switched section headers from labels to h3 finished org details tab added feedback tab made textarea a tinymce fixed test broken by org controller update Added btree back to schema.rb added foreign keys back to schema.rb fixed default email for feedback --- app/controllers/application_controller.rb | 12 +++ app/controllers/orgs_controller.rb | 37 +++++--- app/mailers/user_mailer.rb | 55 +++++++---- app/models/org.rb | 12 ++- app/models/user.rb | 6 +- app/views/layouts/_branding.html.erb | 14 +-- app/views/orgs/_feedback_form.html.erb | 33 +++++++ app/views/orgs/_org_link.html.erb | 15 +++ app/views/orgs/_profile_form.html.erb | 92 +++++++++++++++++++ app/views/orgs/admin_edit.html.erb | 70 ++++---------- .../feedback_notification.html.erb | 6 ++ config/initializers/constants.rb | 7 ++ .../20171024214257_add_links_to_orgs.rb | 5 + ...20171024220146_add_contact_name_to_orgs.rb | 5 + ...71025200301_add_feedback_fields_to_orgs.rb | 7 ++ db/schema.rb | 39 ++++---- lib/assets/javascripts/constants.js | 5 +- lib/assets/javascripts/utils/tabHelper.js | 1 + .../javascripts/views/orgs/admin_edit.js | 91 ++++++++++++++---- lib/assets/package-lock.json | 19 ++-- lib/assets/package.json | 1 + lib/assets/stylesheets/overrides.scss | 90 ++++++++++++------ lib/tasks/migrate.rake | 19 ++++ test/functional/orgs_controller_test.rb | 5 +- 24 files changed, 476 insertions(+), 170 deletions(-) create mode 100644 app/views/orgs/_feedback_form.html.erb create mode 100644 app/views/orgs/_org_link.html.erb create mode 100644 app/views/orgs/_profile_form.html.erb create mode 100644 app/views/user_mailer/feedback_notification.html.erb create mode 100644 db/migrate/20171024214257_add_links_to_orgs.rb create mode 100644 db/migrate/20171024220146_add_contact_name_to_orgs.rb create mode 100644 db/migrate/20171025200301_add_feedback_fields_to_orgs.rb diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index 7df29f7a9a..8475af7921 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -85,6 +85,18 @@ def success_message(obj_name, action) "#{_('Successfully %{action} your %{object}.') % {object: obj_name, action: action}}" end + # Check whether the string is a valid array of JSON objects + def is_json_array_of_objects?(string) + if string.present? + begin + json = JSON.parse(string) + return (json.is_a?(Array) && json.all?{ |o| o.is_a?(Hash) }) + rescue JSON::ParserError + return false + end + end + end + private # Override rails default render action to look for a branded version of a # template instead of using the default one. If no override exists, the diff --git a/app/controllers/orgs_controller.rb b/app/controllers/orgs_controller.rb index 590d30f9c6..b5183b4ba7 100644 --- a/app/controllers/orgs_controller.rb +++ b/app/controllers/orgs_controller.rb @@ -16,25 +16,38 @@ def admin_update attrs = org_params @org = Org.find(params[:id]) authorize @org - @org.banner_text = params["org_banner_text"] - @org.logo = org_params[:logo] if org_params[:logo] - + @org.logo = attrs[:logo] if attrs[:logo] + + tab = (attrs[:feedback_enabled].present? ? 'feedback' : 'profile') + + if attrs[:links].present? + if is_json_array_of_objects?(attrs[:links]) + json = JSON.parse(attrs[:links]) + # Make sure that the JSON hash is structured as: {"link":"string","text":"string"} + if json.all?{ |o| o['link'].present? && o['text'].present? } + @org.links = json + else + redirect_to "#{admin_edit_org_path(@org)}\##{tab}", alert: _('Unable to save your changes. Invalid URLs.') + end + else + redirect_to "#{admin_edit_org_path(@org)}\##{tab}", alert: _('Unable to save your changes. Invalid URLs.') + end + attrs.delete('links') + end + begin - if @org.update_attributes(org_params) - flash[:notice] = success_message(_('organisation'), _('saved')) - render action: "admin_edit" + if @org.update_attributes(attrs) + redirect_to "#{admin_edit_org_path(@org)}\##{tab}", notice: success_message(_('organisation'), _('saved')) else # For some reason our custom validator returns as a string and not a hash like normal activerecord # errors. We followed the example provided in the Rails guides when building the validator so # its unclear why its doing this. Placing a check here for the data type. We should reasses though # when doing a broader eval of the look/feel of the site and we come up with a standardized way of # displaying errors - flash[:alert] = failed_update_error(@org, _('organisation')) - render action: "admin_edit" + redirect_to "#{admin_edit_org_path(@org)}\##{tab}", alert: failed_update_error(@org, _('organisation')) end rescue Dragonfly::Job::Fetch::NotFound => dflye - flash[:alert] = _('There seems to be a problem with your logo. Please upload it again.') - render action: "admin_edit" + redirect_to "#{admin_edit_org_path(@org)}\##{tab}", alert: _('There seems to be a problem with your logo. Please upload it again.') end end @@ -82,7 +95,7 @@ def shibboleth_ds_passthru private def org_params - params.require(:org).permit(:name, :abbreviation, :target_url, :is_other, :banner_text, :language_id, - :region_id, :logo, :contact_email, :remove_logo) + params.require(:org).permit(:name, :abbreviation, :logo, :contact_email, :contact_name, :remove_logo, :links, + :feedback_enabled, :feedback_email_subject, :feedback_email_msg) end end diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index f679ba2dbc..a1f6243023 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -1,6 +1,6 @@ class UserMailer < ActionMailer::Base - default from: Rails.configuration.branding[:organisation][:email] - + default from: Rails.configuration.branding[:organisation][:email] + def welcome_notification(user) @user = user FastGettext.with_locale FastGettext.default_locale do @@ -9,33 +9,33 @@ def welcome_notification(user) end end - def sharing_notification(role, user) + def sharing_notification(role, user) @role = role @user = user FastGettext.with_locale FastGettext.default_locale do - mail(to: @role.user.email, + mail(to: @role.user.email, subject: "#{_('A Data Management Plan in ')} #{Rails.configuration.branding[:application][:name]} #{_(' has been shared with you')}") end - end - - def permissions_change_notification(role, current_user) - @role = role + end + + def permissions_change_notification(role, current_user) + @role = role @current_user = current_user - FastGettext.with_locale FastGettext.default_locale do + FastGettext.with_locale FastGettext.default_locale do mail(to: @role.user.email, subject: "#{_('Changed permissions on a DMP in')} #{Rails.configuration.branding[:application][:name]}") end - end - - def project_access_removed_notification(user, plan, current_user) - @user = user - @plan = plan + end + + def project_access_removed_notification(user, plan, current_user) + @user = user + @plan = plan @current_user = current_user FastGettext.with_locale FastGettext.default_locale do - mail(to: @user.email, + mail(to: @user.email, subject: "#{_('Permissions removed on a DMP in')} #{Rails.configuration.branding[:application][:name]}") end - end + end def api_token_granted_notification(user) @user = user @@ -44,4 +44,27 @@ def api_token_granted_notification(user) subject: "#{_('API rights in')} #{Rails.configuration.branding[:application][:name]}") end end + + def feedback_notification(user, plan) + @user = user + + if user.org.present? + @org = org + @plan = plan + + # Use the generic feedback message unless the Org has specified one + subject = org.feedback_email_subject ||= EMAIL_FEEDBACK_REQUESTED_CONFIRMATION_SUBJECT + + # Send an email to all of the org admins as well as the Org's administrator email + emails = user.org.users.select{ |usr| usr.can_org_admin? && usr != user } + emails << user.org.contact_email if user.org.contact_email.present? + + emails.each do |email| + @email = email + FastGettext.with_locale FastGettext.default_locale do + mail(to: email, subject: subject) + end + end + end + end end diff --git a/app/models/org.rb b/app/models/org.rb index 069c95e04e..dde3833145 100644 --- a/app/models/org.rb +++ b/app/models/org.rb @@ -7,6 +7,8 @@ class Org < ActiveRecord::Base # Sort order: Name ASC default_scope { order(name: :asc) } + # Stores links as an JSON array: [{"link":"http://www.myorg.edu","text":"My Org"},...] + serialize :links, JSON ## # Associations @@ -25,12 +27,13 @@ class Org < ActiveRecord::Base ## # Possibly needed for active_admin # -relies on protected_attributes gem as syntax depricated in rails 4.2 - attr_accessible :abbreviation, :banner_text, :logo, :remove_logo, - :logo_file_name, :name, :target_url, + attr_accessible :abbreviation, :logo, :remove_logo, + :logo_file_name, :name, :links, :organisation_type_id, :wayfless_entity, :parent_id, :sort_name, - :token_permission_type_ids, :language_id, :contact_email, + :token_permission_type_ids, :language_id, :contact_email, :contact_name, :language, :org_type, :region, :token_permission_types, - :guidance_group_ids, :is_other, :region_id, :logo_uid, :logo_name + :guidance_group_ids, :is_other, :region_id, :logo_uid, :logo_name, + :feedback_enabled, :feedback_email_subject, :feedback_email_msg ## # Validators @@ -40,7 +43,6 @@ class Org < ActiveRecord::Base dragonfly_accessor :logo do after_assign :resize_image end - validates_property :height, of: :logo, in: (0..165), message: _("height must be less than 165px") validates_property :format, of: :logo, in: ['jpeg', 'png', 'gif','jpg','bmp'], message: _("must be one of the following formats: jpeg, jpg, png, gif, bmp") validates_size_of :logo, maximum: 500.kilobytes, message: _("can't be larger than 500KB") diff --git a/app/models/user.rb b/app/models/user.rb index 8762b292e7..75b25fe373 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -48,7 +48,11 @@ def filter(query) # Scopes default_scope { includes(:org, :perms) } - + # Retrieves all of the org_admins for the specified org + scope :org_admins, -> (org_id) { + joins(:perms).where("users.org_id = ? AND perms.name IN (?)", org_id, + ['grant_permissions', 'modify_templates', 'modify_guidance', 'change_org_details']) + } # EVALUATE CLASS AND INSTANCE METHODS BELOW # diff --git a/app/views/layouts/_branding.html.erb b/app/views/layouts/_branding.html.erb index 62ec0d12a2..eb05a15665 100644 --- a/app/views/layouts/_branding.html.erb +++ b/app/views/layouts/_branding.html.erb @@ -21,15 +21,11 @@