From e8ab94e1c73b028846ba3fbe6a456ec99869077b Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Thu, 12 Mar 2015 23:53:57 +0100 Subject: [PATCH 1/2] Implements sign up during registration. Solves most of #215 --- app/assets/stylesheets/osem.css.scss | 4 + app/controllers/application_controller.rb | 2 +- .../conference_registrations_controller.rb | 31 +++++--- .../users/omniauth_callbacks_controller.rb | 3 +- app/helpers/application_helper.rb | 3 +- app/models/ability.rb | 4 + app/models/registration.rb | 2 + .../conference_registrations/_form.html.haml | 74 +++++++++++++++---- .../_questions.html.haml | 8 +- config/initializers/devise.rb | 2 +- config/initializers/formtastic.rb | 2 +- db/schema.rb | 2 +- 12 files changed, 101 insertions(+), 36 deletions(-) diff --git a/app/assets/stylesheets/osem.css.scss b/app/assets/stylesheets/osem.css.scss index 85f4337a..83a921a6 100644 --- a/app/assets/stylesheets/osem.css.scss +++ b/app/assets/stylesheets/osem.css.scss @@ -68,3 +68,7 @@ p.comment-body { .well.comment-section { padding-bottom: 40px; } + +#account-already { + font-size: 0.6em; +} \ No newline at end of file diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f99bff34..fca129ac 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -43,7 +43,7 @@ class ApplicationController < ActionController::Base end rescue_from CanCan::AccessDenied do |exception| - Rails.logger.debug('Access denied!') + Rails.logger.debug "Access denied on #{exception.action} #{exception.subject.inspect}" redirect_to root_path, alert: exception.message end diff --git a/app/controllers/conference_registrations_controller.rb b/app/controllers/conference_registrations_controller.rb index 28ba9766..b6a616ec 100644 --- a/app/controllers/conference_registrations_controller.rb +++ b/app/controllers/conference_registrations_controller.rb @@ -1,11 +1,16 @@ class ConferenceRegistrationsController < ApplicationController - before_filter :authenticate_user! + before_filter :authenticate_user!, except: [:new, :create] load_resource :conference, find_by: :short_title authorize_resource :conference_registrations, class: Registration before_action :set_registration, only: [:edit, :update, :destroy, :show] def new - @registration = current_user.registrations.build(conference_id: @conference.id) + # ichain does not allow us to create users during registration + if CONFIG['authentication']['ichain']['enabled'] && !current_user + redirect_to root_path, alert: 'You need to sign in or sign up before continuing.' + end + @registration = Registration.new + @registration.build_user end def show @@ -17,19 +22,24 @@ class ConferenceRegistrationsController < ApplicationController def edit; end def create - @registration = current_user.registrations.build(registration_params) - @registration.conference_id = @conference.id + @registration = Registration.new(registration_params) + @registration.conference = @conference + @registration.user = current_user if current_user if @registration.save # Trigger ahoy event ahoy.track 'Registered', title: 'New registration' + # Sign in the new user + if !current_user + sign_in(@registration.user) + end + + flash[:notice] = 'You are now registered and will be receiving E-Mail notifications.' if @conference.tickets.any? && !current_user.supports?(@conference) - redirect_to conference_tickets_path(@conference.short_title), - notice: 'You are now registered and will be receiving E-Mail notifications.' + redirect_to conference_tickets_path(@conference.short_title) else - redirect_to conference_conference_registrations_path(@conference.short_title), - notice: 'You are now registered and will be receiving E-Mail notifications.' + redirect_to conference_conference_registrations_path(@conference.short_title) end else flash[:error] = "An error prohibited the registration for #{@conference.title}: "\ @@ -63,8 +73,9 @@ class ConferenceRegistrationsController < ApplicationController protected def set_registration - @registration = current_user.registrations.find_by(conference_id: @conference.id) + @registration = Registration.find_by(conference: @conference, user: current_user) if !@registration + flash[:alert] = "Can't find a registration for #{@conference.title} for you. Please register." redirect_to new_conference_conference_registrations_path(@conference.short_title) end end @@ -78,7 +89,7 @@ class ConferenceRegistrationsController < ApplicationController qanswers_attributes: [], event_ids: [], user_attributes: [ - :id, :name, :tshirt, :mobile, :volunteer_experience, :languages] + :username, :email, :name, :password, :password_confirmation] ) end end diff --git a/app/controllers/users/omniauth_callbacks_controller.rb b/app/controllers/users/omniauth_callbacks_controller.rb index e007b84f..ec42b3e2 100644 --- a/app/controllers/users/omniauth_callbacks_controller.rb +++ b/app/controllers/users/omniauth_callbacks_controller.rb @@ -28,7 +28,8 @@ module Users openid.save! sign_in user - redirect_to root_path, notice: user.email + " signed in successfully with #{provider}" + flash[:notice] = "#{user.email} signed in successfully with #{provider}" + redirect_to request.env['omniauth.origin'] || root_path rescue => e flash[:error] = e.message redirect_back_or_to new_user_registration_path diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index 722384f0..0de2afb9 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -24,7 +24,6 @@ module ApplicationHelper end def bootstrap_class_for(flash_type) - logger.debug "flash_type is #{flash_type}" case flash_type when 'success' 'alert-success' @@ -35,7 +34,7 @@ module ApplicationHelper when 'notice' 'alert-info' else - flash_type.to_s + 'alert-warning' end end diff --git a/app/models/ability.rb b/app/models/ability.rb index 78ce1f77..757f421c 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -121,6 +121,10 @@ class Ability can :show, Commercial, commercialable_type: 'Event', commercialable_id: Event.where(state: 'confirmed').pluck(:id) # can view others can :show, User + # can register + can [:show, :create], Registration do |registration| + registration.new_record? + end end def signed_in(user) diff --git a/app/models/registration.rb b/app/models/registration.rb index b595b310..9f068317 100644 --- a/app/models/registration.rb +++ b/app/models/registration.rb @@ -1,5 +1,7 @@ class Registration < ActiveRecord::Base belongs_to :user + validates :user, presence: true + accepts_nested_attributes_for :user belongs_to :conference belongs_to :dietary_choice diff --git a/app/views/conference_registrations/_form.html.haml b/app/views/conference_registrations/_form.html.haml index d5b4ada6..89025cda 100644 --- a/app/views/conference_registrations/_form.html.haml +++ b/app/views/conference_registrations/_form.html.haml @@ -6,18 +6,62 @@ Registration for = @conference.title .row - .col-md-8 - = semantic_form_for(@registration, url: conference_conference_registrations_path(@conference.short_title)) do |f| - - if @conference.questions.any? - = render partial: 'questions', locals: { f: f } - - if @conference.events.workshops.any? - =f.inputs 'Register to Workshops' do - = f.input :events, as: :check_boxes, label: false, collection: @conference.events.workshops - = f.inputs 'Your Travel Info' do - = f.input :arrival, as: :string, label: 'Your arrival time', input_html: { value: (f.object.arrival.to_formatted_s(:db_without_seconds) unless f.object.arrival.nil?), id: 'registration-arrival-datepicker', readonly: 'readonly' } - = f.input :departure, as: :string, label: 'Your departure time', input_html: { value: (f.object.departure.to_formatted_s(:db_without_seconds) unless f.object.departure.nil?), id: 'registration-departure-datepicker', readonly: 'readonly' } - %p.pull-right - - if @conference.user_registered?(current_user) - = f.action :submit, button_html: { value: 'Update Registration', class: 'btn btn-primary' } - - else - = f.action :submit, button_html: { value: 'Register', class: 'btn btn-primary', id: 'register' } + .col-md-6 + - if !current_user + %legend + %span + =link_to('#signup', role: 'tab', "aria-controls" => "home", "data-toggle" => "tab") do + = CONFIG['name'] + Account + %span.pull-right#account-already + =link_to('#signin', role: 'tab', "aria-controls" => "home", "data-toggle" => "tab") do + Already have an account? + .tab-content + .tab-pane.active{role: 'tabpanel', id: 'signup'} + = semantic_form_for(@registration, url: conference_conference_registrations_path(@conference.short_title)) do |f| + - if !current_user + = f.fields_for :user do |u| + = u.input :username, input_html: {required: 'required', autocomplete: 'off'} + = u.input :email, input_html: {required: 'required', autocomplete: 'off'} + = u.input :password, input_html: {required: 'required', autocomplete: 'off'} + = u.input :password_confirmation, :required => true, input_html: {required: 'required', autocomplete: 'off'} + - if @conference.questions.any? + = render partial: 'questions', locals: { f: f } + - if @conference.events.workshops.any? + =f.inputs 'Register to Workshops' do + = f.input :events, as: :check_boxes, label: false, collection: @conference.events.workshops + = f.inputs 'Your Travel Info' do + = f.input :arrival, as: :string, label: 'Your arrival time', input_html: { value: (f.object.arrival.to_formatted_s(:db_without_seconds) unless f.object.arrival.nil?), id: 'registration-arrival-datepicker', readonly: 'readonly' } + = f.input :departure, as: :string, label: 'Your departure time', input_html: { value: (f.object.departure.to_formatted_s(:db_without_seconds) unless f.object.departure.nil?), id: 'registration-departure-datepicker', readonly: 'readonly' } + %p.pull-right + - if @conference.user_registered?(current_user) + = f.action :submit, button_html: { value: 'Update Registration', class: 'btn btn-primary' } + - else + = f.action :submit, button_html: { value: 'Register', class: 'btn btn-primary', id: 'register' } + .tab-pane{role: 'tabpanel', id: 'signin'} + - if !CONFIG['authentication']['ichain']['enabled'] + = form_tag(new_user_session_path, class: 'form-horizontal') do + %legend + %span + Sign In + .form-group + %label{for: "user[login]", class: 'col-sm-2 control-label'} + Username + .col-sm-10 + = text_field_tag 'user[login]', nil, placeholder: 'Username', class: 'form-control', required: 'required' + .form-group + %label{for: "user[password]", class: 'col-sm-2 control-label'} + Password + .col-sm-10 + = password_field_tag 'user[password]', nil, placeholder: 'Password', class: 'form-control', required: 'required' + .form-group + .col-sm-12 + %button.btn.btn-success.pull-right + Sign in + - unless omniauth_configured.empty? + .form-group + %hr + %p.text-center + or sign in using + .text-center + = render 'devise/shared/openid' \ No newline at end of file diff --git a/app/views/conference_registrations/_questions.html.haml b/app/views/conference_registrations/_questions.html.haml index da80e2d3..2443b7dc 100644 --- a/app/views/conference_registrations/_questions.html.haml +++ b/app/views/conference_registrations/_questions.html.haml @@ -1,8 +1,8 @@ -- @conference.questions.each do |q| - = f.inputs 'Additional Info' do += f.inputs 'Additional Info' do + - @conference.questions.each do |q| - if q.question_type.id == 1 || q.question_type.id == 2 # yes/no or single choice - = f.input :qanswers, :collection => q.qanswers, :as => :select, :input_html => { :multiple => false, class: 'col-sm-10' }, label: q.title, :include_blank => "Please make your choice", + = f.input :qanswers, :collection => q.qanswers, :as => :select, :input_html => { :multiple => false }, label: q.title, :include_blank => "Please make your choice", :member_label => Proc.new {|a| a.answer.title} - if q.question_type.id == 3 # multiple choice - = f.input :qanswers, :collection => q.qanswers, :as => :check_boxes, :input_html => { class: 'col-sm-2' }, :label => false, + = f.input :qanswers, :collection => q.qanswers, :as => :check_boxes, label: q.title, :member_label => Proc.new {|a| a.answer.title} \ No newline at end of file diff --git a/config/initializers/devise.rb b/config/initializers/devise.rb index 0c7d813e..3312206f 100644 --- a/config/initializers/devise.rb +++ b/config/initializers/devise.rb @@ -105,7 +105,7 @@ Devise.setup do |config| # able to access the website for two days without confirming his account, # access will be blocked just in the third day. Default is 0.days, meaning # the user cannot access the website without confirming his account. - # config.allow_unconfirmed_access_for = 2.days + config.allow_unconfirmed_access_for = 2.days # If true, requires any email changes to be confirmed (exactly the same way as # initial account confirmation) to be applied. Requires additional unconfirmed_email diff --git a/config/initializers/formtastic.rb b/config/initializers/formtastic.rb index 29962539..4374676b 100644 --- a/config/initializers/formtastic.rb +++ b/config/initializers/formtastic.rb @@ -22,7 +22,7 @@ Formtastic::FormBuilder.include_blank_for_select_by_default = false # '*'. In other words, if you configure formtastic.required # in your locale, it will replace the abbr title properly. But if you don't want to use # abbr tag, you can simply give a string as below -Formtastic::FormBuilder.required_string = '(required)' +Formtastic::FormBuilder.required_string = proc { Formtastic::Util.html_safe(%{ *}) } # Set the string that will be appended to the labels/fieldsets which are optional # Defaults to an empty string ("") and also accepts procs (see required_string above) diff --git a/db/schema.rb b/db/schema.rb index e5e78cce..38c9bda2 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20141130182139) do +ActiveRecord::Schema.define(version: 20150304135935) do create_table "ahoy_events", force: true do |t| t.uuid "visit_id" From 0ce4aa0a496d2e974a149100575e1349ce5d0d83 Mon Sep 17 00:00:00 2001 From: Henne Vogelsang Date: Fri, 13 Mar 2015 00:43:09 +0100 Subject: [PATCH 2/2] Fix the ability spec * Get rid of the volunteer coordinator role, we don't use it currently * Test the ability with users that exclusively have a single role --- .../admin/registrations_controller.rb | 2 +- app/models/ability.rb | 2 +- spec/features/ability_spec.rb | 91 +++---------------- 3 files changed, 13 insertions(+), 82 deletions(-) diff --git a/app/controllers/admin/registrations_controller.rb b/app/controllers/admin/registrations_controller.rb index d92f6f3d..27dacfa4 100644 --- a/app/controllers/admin/registrations_controller.rb +++ b/app/controllers/admin/registrations_controller.rb @@ -1,7 +1,7 @@ module Admin class RegistrationsController < Admin::BaseController load_and_authorize_resource :conference, find_by: :short_title - load_and_authorize_resource through: :conference + load_and_authorize_resource :registration, through: :conference before_filter :set_user, except: [:index] def index diff --git a/app/models/ability.rb b/app/models/ability.rb index 757f421c..8bdab15e 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -122,7 +122,7 @@ class Ability # can view others can :show, User # can register - can [:show, :create], Registration do |registration| + can [:read, :create], Registration do |registration| registration.new_record? end end diff --git a/spec/features/ability_spec.rb b/spec/features/ability_spec.rb index 2ae0e5c7..26b41a77 100644 --- a/spec/features/ability_spec.rb +++ b/spec/features/ability_spec.rb @@ -13,11 +13,14 @@ feature 'Has correct abilities' do let(:role_info_desk) { create(:role, name: 'info_desk', resource: conference3) } let(:role_volunteer_coordinator) { create(:role, name: 'volunteer_coordinator', resource: conference4) } - let(:user) { create(:user, role_ids: [role_organizer.id, role_cfp.id, role_info_desk.id, role_volunteer_coordinator.id]) } + let(:user_organizer) { create(:user, role_ids: [role_organizer.id]) } + let(:user_cfp) { create(:user, role_ids: [role_cfp.id]) } + let(:user_info_desk) { create(:user, role_ids: [role_info_desk.id]) } + let(:user_volunteer_coordinator) { create(:user, role_ids: [role_volunteer_coordinator.id]) } scenario 'when user is organizer' do - user.is_admin = false - sign_in user + user_organizer.is_admin = false + sign_in user_organizer visit admin_conference_path(conference1.short_title) expect(page).to have_selector('li.nav-header.nav-header-bigger a', text: 'Dashboard') @@ -87,8 +90,8 @@ feature 'Has correct abilities' do end scenario 'when user is cfp' do - user.is_admin = false - sign_in user + user_cfp.is_admin = false + sign_in user_cfp visit admin_conference_path(conference2.short_title) expect(page).to have_selector('li.nav-header.nav-header-bigger a', text: 'Dashboard') @@ -121,7 +124,7 @@ feature 'Has correct abilities' do expect(current_path).to eq(admin_conference_path(conference2.short_title)) visit admin_conference_registrations_path(conference2.short_title) - expect(current_path).to eq(root_path) + expect(current_path).to eq(admin_conference_registrations_path(conference2.short_title)) visit admin_conference_events_path(conference2.short_title) expect(current_path).to eq(admin_conference_events_path(conference2.short_title)) @@ -158,8 +161,8 @@ feature 'Has correct abilities' do end scenario 'when user is info desk' do - user.is_admin = false - sign_in user + user_info_desk.is_admin = false + sign_in user_info_desk visit admin_conference_path(conference3.short_title) expect(page).to have_selector('li.nav-header.nav-header-bigger a', text: 'Dashboard') @@ -226,77 +229,5 @@ feature 'Has correct abilities' do visit admin_conference_commercials_path(conference3.short_title) expect(current_path).to eq(admin_conference_commercials_path(conference3.short_title)) - - end - - scenario 'when user is volunteer coordinator' do - user.is_admin = false - sign_in user - visit admin_conference_path(conference4.short_title) - - expect(page).to have_selector('li.nav-header.nav-header-bigger a', text: 'Dashboard') - expect(page).to have_link('Basics', href: "/admin/conference/#{conference4.short_title}/edit") - expect(page).to_not have_link('Contact', href: "/admin/conference/#{conference4.short_title}/contact/edit") - expect(page).to have_link('Commercials', href: "/admin/conference/#{conference4.short_title}/commercials") - expect(page).to_not have_link('Events', href: "/admin/conference/#{conference4.short_title}/events") - expect(page).to_not have_link('Registrations', href: "/admin/conference/#{conference4.short_title}/registrations") - expect(page).to_not have_link('Schedule', href: "/admin/conference/#{conference4.short_title}/schedule") - expect(page).to_not have_link('Campaigns', href: "/admin/conference/#{conference4.short_title}/campaigns") - expect(page).to_not have_link('Goals', href: "/admin/conference/#{conference4.short_title}/targets") - expect(page).to_not have_link('Venue', href: "/admin/conference/#{conference4.short_title}/venue") - expect(page).to_not have_link('Rooms', href: "/admin/conference/#{conference4.short_title}/rooms") - expect(page).to_not have_link('Lodgings', href: "/admin/conference/#{conference4.short_title}/lodgings") - expect(page).to_not have_link('Sponsorship', href: "/admin/conference/#{conference4.short_title}/sponsorship_levels") - expect(page).to_not have_link('Sponsors', href: "/admin/conference/#{conference4.short_title}/sponsors") - expect(page).to_not have_link('Supporter Levels', href: "/admin/conference/#{conference4.short_title}/supporter_levels") - expect(page).to_not have_link('E-Mails', href: "/admin/conference/#{conference4.short_title}/emails") - expect(page).to_not have_link('Call for Papers', href: "/admin/conference/#{conference4.short_title}/call_for_paper") - expect(page).to_not have_link('Tracks', href: "/admin/conference/#{conference4.short_title}/tracks") - expect(page).to_not have_link('Event Types', href: "/admin/conference/#{conference4.short_title}/event_types") - expect(page).to_not have_link('Difficulty Levels', href: "/admin/conference/#{conference4.short_title}/difficulty_levels") - expect(page).to_not have_link('Questions', href: "/admin/conference/#{conference4.short_title}/questions") - expect(page).to_not have_link('Roles', href: "/admin/conference/#{conference4.short_title}/roles") - - visit edit_admin_conference_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit admin_conference_path(conference4.short_title) - expect(current_path).to eq(admin_conference_path(conference4.short_title)) - - visit admin_conference_registrations_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit admin_conference_events_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit admin_conference_schedule_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit admin_conference_campaigns_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit admin_conference_targets_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit edit_admin_conference_venue_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit admin_conference_sponsorship_levels_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit admin_conference_tickets_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit admin_conference_emails_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit new_admin_conference_call_for_paper_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit admin_conference_questions_path(conference4.short_title) - expect(current_path).to eq(root_path) - - visit admin_conference_commercials_path(conference4.short_title) - expect(current_path).to eq(admin_conference_commercials_path(conference4.short_title)) end end