From 25602972abed0213c7c83cbb34eea89fd5260f03 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 12 Dec 2018 14:11:57 +0100 Subject: [PATCH 1/3] Prevent delivery of mails to non-existent empty mail addresses --- app/mailers/user_mailer.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/mailers/user_mailer.rb b/app/mailers/user_mailer.rb index 497c2a31..b7b3d52f 100644 --- a/app/mailers/user_mailer.rb +++ b/app/mailers/user_mailer.rb @@ -1,4 +1,10 @@ class UserMailer < ActionMailer::Base + + def mail(*args) + # used to prevent the delivery to pseudonymous users without a valid email address + super unless args.first[:to].blank? + end + def activation_needed_email(user) @activation_url = activate_internal_user_url(user, token: user.activation_token) mail(subject: t('mailers.user_mailer.activation_needed.subject'), to: user.email) From b4b9ab48d05f37445ee7238d07013dc1f0e0070f Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 12 Dec 2018 16:47:49 +0100 Subject: [PATCH 2/3] External User: Set name to displayname and introduce real_name --- app/models/external_user.rb | 28 +++++++++++++++++++------ app/views/external_users/show.html.slim | 2 +- 2 files changed, 23 insertions(+), 7 deletions(-) diff --git a/app/models/external_user.rb b/app/models/external_user.rb index b877f08d..61efaea5 100644 --- a/app/models/external_user.rb +++ b/app/models/external_user.rb @@ -4,12 +4,28 @@ class ExternalUser < ApplicationRecord validates :consumer_id, presence: true validates :external_id, presence: true - def displayname - result = name - if(result == nil || result == "") - result = "User " + id.to_s - end - result + def name + # Internal name, shown to teachers and administrators + pseudo_name end + def displayname + # External name, shown to the user itself and other users, e.g. on RfCs + pseudo_name + end + + def real_name + # Name attribute of the object as persistet in the database + self[:name] + end + + def pseudo_name + if real_name.blank? + "User " + id.to_s + else + real_name + end + end + private :pseudo_name + end diff --git a/app/views/external_users/show.html.slim b/app/views/external_users/show.html.slim index 33e2dd6b..c68a91f5 100644 --- a/app/views/external_users/show.html.slim +++ b/app/views/external_users/show.html.slim @@ -1,6 +1,6 @@ h1 = @user.name -= row(label: 'external_user.name', value: @user.name) += row(label: 'external_user.name', value: @user.real_name) //= row(label: 'external_user.email', value: @user.email) = row(label: 'external_user.consumer', value: link_to(@user.consumer, @user.consumer)) From 88081bb5417e3d90f7f5cb9dbe2f2984192f22bd Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Thu, 13 Dec 2018 13:11:32 +0100 Subject: [PATCH 3/3] Replace usage of name in favor of displayname --- app/models/external_user.rb | 21 ++----------------- app/views/application/welcome.html.slim | 2 +- app/views/exercise_collections/show.html.slim | 4 ++-- app/views/exercises/feedback.html.slim | 2 +- app/views/external_users/index.html.slim | 2 +- app/views/external_users/show.html.slim | 4 ++-- .../user_exercise_feedbacks/index.html.slim | 2 +- .../user_mailer/send_thank_you_note.slim | 2 +- spec/features/authentication_spec.rb | 4 ++-- 9 files changed, 13 insertions(+), 30 deletions(-) diff --git a/app/models/external_user.rb b/app/models/external_user.rb index 61efaea5..fbd3cd3d 100644 --- a/app/models/external_user.rb +++ b/app/models/external_user.rb @@ -4,28 +4,11 @@ class ExternalUser < ApplicationRecord validates :consumer_id, presence: true validates :external_id, presence: true - def name - # Internal name, shown to teachers and administrators - pseudo_name - end - def displayname - # External name, shown to the user itself and other users, e.g. on RfCs - pseudo_name - end - - def real_name - # Name attribute of the object as persistet in the database - self[:name] - end - - def pseudo_name - if real_name.blank? + if name.blank? "User " + id.to_s else - real_name + name end end - private :pseudo_name - end diff --git a/app/views/application/welcome.html.slim b/app/views/application/welcome.html.slim index 8f9448c7..84692ec1 100644 --- a/app/views/application/welcome.html.slim +++ b/app/views/application/welcome.html.slim @@ -3,6 +3,6 @@ h1 = t('.title', application_name: application_name) - if current_user.try(:external_user?) p = t('.text_signed_in_as_external_user', application_name: application_name) - elsif current_user.try(:internal_user?) - p = t('.text_signed_in_as_internal_user', user_name: current_user.name) + p = t('.text_signed_in_as_internal_user', user_name: current_user.displayname) - else p == t('.text_signed_out', application_name: application_name, sign_in_path: sign_in_path) diff --git a/app/views/exercise_collections/show.html.slim b/app/views/exercise_collections/show.html.slim index 4fa6a8ac..db0945f3 100644 --- a/app/views/exercise_collections/show.html.slim +++ b/app/views/exercise_collections/show.html.slim @@ -3,7 +3,7 @@ h1 = render('shared/edit_button', object: @exercise_collection) = row(label: 'exercise_collections.name', value: @exercise_collection.name) -= row(label: 'exercise_collections.user', value: link_to(@exercise_collection.user.name, @exercise_collection.user)) unless @exercise_collection.user.nil? += row(label: 'exercise_collections.user', value: link_to(@exercise_collection.user.displayname, @exercise_collection.user)) unless @exercise_collection.user.nil? = row(label: 'exercise_collections.use_anomaly_detection', value: @exercise_collection.use_anomaly_detection) = row(label: 'exercise_collections.updated_at', value: @exercise_collection.updated_at) @@ -24,5 +24,5 @@ h4.mt-4 = t('activerecord.attributes.exercise_collections.exercises') td = exercise_collection_item.position td = link_to(exercise.title, exercise) td = link_to_if(exercise.execution_environment && policy(exercise.execution_environment).show?, exercise.execution_environment, exercise.execution_environment) - td = link_to_if(exercise.user && policy(exercise.user).show?, exercise.user.name, exercise.user) + td = link_to_if(exercise.user && policy(exercise.user).show?, exercise.user.displayname, exercise.user) td = link_to(t('shared.statistics'), statistics_exercise_path(exercise), 'data-turbolinks' => "false") diff --git a/app/views/exercises/feedback.html.slim b/app/views/exercises/feedback.html.slim index 5090d0e2..26180cfd 100644 --- a/app/views/exercises/feedback.html.slim +++ b/app/views/exercises/feedback.html.slim @@ -13,7 +13,7 @@ h1 = link_to(@exercise, exercise_path(@exercise)) li.card.mt-2 .card-header role="tab" id="heading" div.clearfix.feedback-header - span.username = link_to(feedback.user.name, statistics_external_user_exercise_path(id: @exercise.id, external_user_id: feedback.user.id)) + span.username = link_to(feedback.user.displayname, statistics_external_user_exercise_path(id: @exercise.id, external_user_id: feedback.user.id)) - if feedback.anomaly_notification i class="fa fa-envelope-o" data-placement="top" data-toggle="tooltip" data-container="body" title=feedback.anomaly_notification.reason span.date = feedback.created_at diff --git a/app/views/external_users/index.html.slim b/app/views/external_users/index.html.slim index 34359b2a..bff2509f 100644 --- a/app/views/external_users/index.html.slim +++ b/app/views/external_users/index.html.slim @@ -10,7 +10,7 @@ h1 = ExternalUser.model_name.human(count: 2) tbody - @users.each do |user| tr - td = user.name + td = user.displayname td = link_to(user.consumer, user.consumer) td = link_to(t('shared.show'), user) diff --git a/app/views/external_users/show.html.slim b/app/views/external_users/show.html.slim index c68a91f5..e5aee19e 100644 --- a/app/views/external_users/show.html.slim +++ b/app/views/external_users/show.html.slim @@ -1,6 +1,6 @@ -h1 = @user.name +h1 = @user.displayname -= row(label: 'external_user.name', value: @user.real_name) += row(label: 'external_user.name', value: @user.name) //= row(label: 'external_user.email', value: @user.email) = row(label: 'external_user.consumer', value: link_to(@user.consumer, @user.consumer)) diff --git a/app/views/user_exercise_feedbacks/index.html.slim b/app/views/user_exercise_feedbacks/index.html.slim index 52249229..a9e6b33b 100644 --- a/app/views/user_exercise_feedbacks/index.html.slim +++ b/app/views/user_exercise_feedbacks/index.html.slim @@ -19,7 +19,7 @@ h1 = UserExerciseFeedback.model_name.human(count: 2) - @uefs.each do |uef| tr td = uef.user.id - td = uef.user.name + td = uef.user.displayname td = link_to(uef.exercise.title, uef.exercise) td = link_to(t('shared.show'), uef) td = link_to(t('shared.destroy'), uef, data: {confirm: t('shared.confirm_destroy')}, method: :delete) diff --git a/app/views/user_mailer/send_thank_you_note.slim b/app/views/user_mailer/send_thank_you_note.slim index fcdcdf8d..465e3083 100644 --- a/app/views/user_mailer/send_thank_you_note.slim +++ b/app/views/user_mailer/send_thank_you_note.slim @@ -1 +1 @@ -== t('mailers.user_mailer.send_thank_you_note.body', receiver_displayname: @receiver_displayname, link_to_comment: link_to(@rfc_link, @rfc_link), author: @author, thank_you_note: @thank_you_note ) +== t('mailers.user_mailer.send_thank_you_note.body', receiver_displayname: @receiver_displayname, link_to_comment: link_to(@rfc_link, @rfc_link), author: @author.displayname, thank_you_note: @thank_you_note ) diff --git a/spec/features/authentication_spec.rb b/spec/features/authentication_spec.rb index 2a777dab..1cd08727 100644 --- a/spec/features/authentication_spec.rb +++ b/spec/features/authentication_spec.rb @@ -38,8 +38,8 @@ describe 'Authentication' do visit(root_path) end - it "displays the user's name" do - expect(page).to have_content(user.name) + it "displays the user's displayname" do + expect(page).to have_content(user.displayname) end it 'displays a sign out link' do