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

4311 - Missing display name fix #4363

Merged
merged 6 commits into from
Oct 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 21 additions & 15 deletions app/controllers/registrations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -22,24 +22,21 @@ def destroy
end

def create
unless params[:owner_slug].blank?
@owner = User.where(slug: params[:owner_slug]).first
end
@owner = User.find_by(slug: params[:owner_slug]) if params[:owner_slug].present?

#merge the new user information into the guest user id to change into normal user
if current_user && current_user.guest?
# Merge the new user information into the guest user id to change into normal user
if current_user&.guest?
@user = current_user
@user.update(sign_up_params)
@user.guest = false

else
@user = build_resource(sign_up_params)
end

#this is the default Devise code
# This is the default Devise code
yield resource if block_given?
if check_recaptcha(model: @user) && @user.save

if check_recaptcha(model: @user) && @user.save(context: :registration)
# Record the `joined` deed based on Ahoy Visit
join_collection = joined_from_collection(current_visit.id)
unless join_collection.nil?
Expand All @@ -57,13 +54,13 @@ def create
expire_data_after_sign_in!
respond_with resource, location: after_inactive_sign_up_path_for(resource)
end
#set the guest_user_id of the session to nil for user login/out
# set the guest_user_id of the session to nil for user login/out
if session[:guest_user_id]
session[:guest_user_id] = nil
end

if @user.owner
@user.account_type="Trial"
@user.account_type = 'Trial'
@user.save
alert_bento
end
Expand All @@ -73,7 +70,8 @@ def create
if @validatable
@minimum_password_length = resource_class.password_length.min
end
respond_with resource

after_failed_sign_up_action_for(params[:registration_type]&.to_sym)
end
end

Expand All @@ -95,7 +93,6 @@ def update
end
end


def set_saml
institution = saml_provider_param
redirect_to user_omniauth_authorize_path(institution) #go to users/auth/saml/instution_name
Expand All @@ -114,7 +111,7 @@ def alert_bento()
def after_sign_up_path_for(resource)
if @user.owner
# Always send new owners to their dashboard for analytics purposes
"#{dashboard_owner_path}#freetrial"
"#{dashboard_owner_path}#freetrial"
else
# New users should be returned to where they were or to their dashboard/watchlist
if session[:user_return_to] && !landing_pages.include?(session[:user_return_to])
Expand All @@ -131,6 +128,15 @@ def after_update_path_for(resource)
edit_registration_path(resource)
end

def after_failed_sign_up_action_for(registration_type)
case registration_type
when :free_trial
render :new_trial
else
render :new
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If in case we add more registration_type, we can easily add it here

end
end

def check_recaptcha(options)
return verify_recaptcha(options) if RECAPTCHA_ENABLED
true
Expand Down
20 changes: 15 additions & 5 deletions app/models/user.rb
Original file line number Diff line number Diff line change
Expand Up @@ -133,12 +133,13 @@ class User < ApplicationRecord

validates :website, allow_blank: true, format: { with: URI.regexp }
validate :email_does_not_match_denylist
validate :display_name_presence

before_validation :update_display_name

after_save :create_notifications
after_create :set_default_footer_block
#before_destroy :clean_up_orphans
# before_destroy :clean_up_orphans

def email_does_not_match_denylist
raw = PageBlock.where(view: "email_denylist").first
Expand All @@ -150,12 +151,21 @@ def email_does_not_match_denylist
end
end

def display_name_presence
return unless validation_context == :registration
return unless new_record?
return unless owner

errors.add(:display_name, :blank) if self[:display_name].blank?
end

def update_display_name
self.real_name = nil if self.real_name.blank?

if self.owner
self.display_name = self.real_name
self[:display_name] = self.real_name
else
self.display_name = login
self[:display_name] = self.login
end
end

Expand Down Expand Up @@ -287,9 +297,9 @@ def like_owner?(obj)

def display_name
if self.guest
"Guest"
'Guest'
else
self[:display_name] || self[:login]
self[:display_name].presence || self[:login]
end
end

Expand Down
5 changes: 2 additions & 3 deletions app/views/devise/registrations/new_trial.html.slim
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ section.signon
<br><br>
==t('.just_want_to_transcribe', sign_up_here: (link_to t('.sign_up_here'), new_user_registration_path))


=form_for(resource, as: resource_name, url: registration_path(resource_name)) do |f|
=form_for(resource, as: resource_name, url: registration_path(resource_name, registration_type: :free_trial)) do |f|
=devise_error_messages!
.signon_field
=f.label :login, t('.login')
Expand All @@ -37,4 +36,4 @@ section.signon
=f.check_box :activity_email, checked: true
=f.label :receive_activity_emails, t('devise.receive_activity_emails')
<br>
=f.button t('devise.create_account'), class: 'big'
=f.button t('devise.create_account'), class: 'big'
1 change: 1 addition & 0 deletions config/locales/devise/devise-de.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ de:
email_address: E-Mail-Adresse
errors:
keys:
display_name: Organisationsname
email: E-Mail
login: Benutzername
password: Passwort
Expand Down
1 change: 1 addition & 0 deletions config/locales/devise/devise-en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ en:
email_address: Email Address
errors:
keys:
display_name: Organization Name
email: Email
login: Username
password: Password
Expand Down
1 change: 1 addition & 0 deletions config/locales/devise/devise-es.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ es:
email_address: Dirección de correo electrónico
errors:
keys:
display_name: Nombre para mostrar
email: Correo electrónico
login: Nombre de usuario
password: Contraseña
Expand Down
1 change: 1 addition & 0 deletions config/locales/devise/devise-fr.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ fr:
email_address: Adresse e-mail
errors:
keys:
display_name: Organisation
email: E-mail
login: Nom d'utilisateur
password: Mot de passe
Expand Down
1 change: 1 addition & 0 deletions config/locales/devise/devise-pt.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ pt:
email_address: Endereço de email
errors:
keys:
display_name: Nome em Exibição
email: Email
login: Nome de usuário
password: Senha
Expand Down
4 changes: 1 addition & 3 deletions spec/features/archive_import_spec.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
require 'spec_helper'

describe "IA import actions", :order => :defined do

describe 'IA import actions', order: :defined, skip: 'IA outtage, temporarily skipping' do
before :all do

@owner = User.find_by(login: OWNER)
@collections = @owner.all_owner_collections
@collection = @collections.second
Expand Down
2 changes: 1 addition & 1 deletion spec/features/collection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -289,7 +289,7 @@

describe "collection spec (isolated)" do
before :all do
@factory_owner = create(:user, owner: true)
@factory_owner = create(:owner)
end

it 'updates collection statistics', :js => true do
Expand Down
2 changes: 1 addition & 1 deletion spec/features/import_data_spec.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require 'spec_helper'

describe "import data" do
let(:owner){ create(:user, owner: true) }
let(:owner){ create(:owner) }

before :each do
DatabaseCleaner.start
Expand Down
1 change: 1 addition & 0 deletions spec/features/owner_actions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,7 @@

visit dashboard_owner_path
page.find('.maincol').find('a', text: @collection.title).click
page.click_link('Show All')
page.find('.collection-works').find('a', text: @title).click
page.find('.tabs').click_link('Settings')
expect(page).to have_content(@title)
Expand Down
10 changes: 5 additions & 5 deletions spec/features/zz_convention_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@
before :all do
@owner = User.find_by(login: OWNER)
@collections = @owner.all_owner_collections
@collection = @collections.second
@work = @collection.works.last
@collection = @collections.find('imported-collection')
@work = @collection.works.find_by(transcription_conventions: nil)
@page = @work.pages.first
@conventions = @collection.transcription_conventions
@clean_conventions = ActionController::Base.helpers.strip_tags(@collection.transcription_conventions)
Expand Down Expand Up @@ -58,13 +58,13 @@
page.find('.tabs').click_link("Settings")
page.find('.side-tabs').click_link("Help Text")
page.fill_in 'collection_transcription_conventions', with: @new_convention
#check unchanged work for collection conventions
work2 = @collection.works.first
# check unchanged work for collection conventions
work2 = @collection.works.where.not(id: @work.id).first
page2 = work2.pages.second
visit collection_read_work_path(work2.collection.owner, work2.collection, work2)
page.find('.work-page_title', text: page2.title).click_link(page2.title)
expect(page).to have_content @new_convention
#check changed work for collection conventions
# check changed work for collection conventions
visit collection_read_work_path(@work.collection.owner, @work.collection, @work)
page.find('.work-page_title', text: @page.title).click_link(@page.title)
if page.has_content?("Facsimile")
Expand Down
Loading