From cb723ea43254efa031928e4682ffbf5d84d60ae9 Mon Sep 17 00:00:00 2001 From: Benjamin ter Kuile Date: Tue, 24 Feb 2015 17:56:06 +0100 Subject: [PATCH] More advance signup flow --- .../supplier/app/models/supplier.js.coffee | 1 + .../supplier/app/templates/application.emblem | 13 ++++--- app/controllers/application_controller.rb | 9 ++++- app/controllers/new_suppliers_controller.rb | 4 +- app/controllers/registrations_controller.rb | 4 ++ .../suppliers/sessions_controller.rb | 7 ++++ app/models/employee.rb | 2 +- app/models/supplier.rb | 1 + app/services/new_supplier.rb | 24 ++++++++---- app/views/layouts/supplier/app.html.slim | 1 - config/locales/supplier.en.yml | 2 +- config/routes.rb | 6 ++- spec/acceptance/suppliers/settings.feature | 8 +--- spec/acceptance/suppliers/signup.feature | 24 +++++++++--- .../acceptance_steps/global_employee_steps.rb | 9 +++++ .../suppliers/navigation_steps.rb | 6 +++ .../suppliers/settings_steps.rb | 9 +---- .../suppliers/signup_steps.rb | 39 ++++++++++++++----- spec/factories/employee_factory.rb | 2 +- 19 files changed, 123 insertions(+), 48 deletions(-) create mode 100644 app/controllers/suppliers/sessions_controller.rb create mode 100644 spec/acceptance_steps/global_employee_steps.rb diff --git a/app/assets/javascripts/supplier/app/models/supplier.js.coffee b/app/assets/javascripts/supplier/app/models/supplier.js.coffee index f5825765..6d66890b 100644 --- a/app/assets/javascripts/supplier/app/models/supplier.js.coffee +++ b/app/assets/javascripts/supplier/app/models/supplier.js.coffee @@ -1,6 +1,7 @@ attr = DS.attr App.Supplier = DS.Model.extend name: attr 'string' + email: attr 'string' time_zone: attr 'string' address: attr 'string' house_number: attr 'number' diff --git a/app/assets/javascripts/supplier/app/templates/application.emblem b/app/assets/javascripts/supplier/app/templates/application.emblem index 421438f1..de28b901 100644 --- a/app/assets/javascripts/supplier/app/templates/application.emblem +++ b/app/assets/javascripts/supplier/app/templates/application.emblem @@ -1,5 +1,8 @@ -= partial "global/top_menu" -= partial "global/side_menu" -.main-section= outlet -= view flash_message -= outlet modal +if loading + span.fa.fa-3x.fa-spinner.fa-spin +else + = partial "global/top_menu" + = partial "global/side_menu" + .main-section= outlet + = view flash_message + = outlet modal diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index f9f55b4d..3a44bf47 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -36,7 +36,7 @@ private def set_locale I18n.locale = (params[:locale].presence || I18n.default_locale).to_sym end - + def _render_with_renderer_json(resource, options) serializer = build_json_serializer(resource, options) @@ -55,6 +55,13 @@ private "theme1" end + def after_sign_in_path_for(resource) + case resource + when Employee then supplier_root_path + else root_path + end + end + def check_active_list_state if current_user.try(:active_list_id) unless active_list.active? diff --git a/app/controllers/new_suppliers_controller.rb b/app/controllers/new_suppliers_controller.rb index 04e4eba9..a2f23528 100644 --- a/app/controllers/new_suppliers_controller.rb +++ b/app/controllers/new_suppliers_controller.rb @@ -14,7 +14,9 @@ class NewSuppliersController < ApplicationController @new_supplier.current_employee = current_employee if @new_supplier.save - redirect_to supplier_root_path + sign_in @new_supplier.employee unless current_employee + session[:supplier_id] = @new_supplier.supplier.try(:id) + redirect_to supplier_root_path(anchor: '/pages/introduction') else render 'new' end diff --git a/app/controllers/registrations_controller.rb b/app/controllers/registrations_controller.rb index cac13d6b..bb71c0ec 100644 --- a/app/controllers/registrations_controller.rb +++ b/app/controllers/registrations_controller.rb @@ -1,4 +1,8 @@ class RegistrationsController < Devise::RegistrationsController + protected + + #def after_sign_up_path(resource) + #end private diff --git a/app/controllers/suppliers/sessions_controller.rb b/app/controllers/suppliers/sessions_controller.rb new file mode 100644 index 00000000..bbe5be15 --- /dev/null +++ b/app/controllers/suppliers/sessions_controller.rb @@ -0,0 +1,7 @@ +class Suppliers::SessionsController < Devise::SessionsController + + def destroy + session[:supplier_id] = nil + super + end +end diff --git a/app/models/employee.rb b/app/models/employee.rb index 70603e7b..15d6b785 100644 --- a/app/models/employee.rb +++ b/app/models/employee.rb @@ -16,7 +16,7 @@ class Employee end view :by_confirmation_token, key: :confirmation_token # devise confirmable - devise :database_authenticatable, :recoverable, :rememberable, :trackable, :registerable, :confirmable + devise :database_authenticatable, :recoverable, :rememberable, :trackable #, :registerable #, :confirmable property :unconfirmed_email property :name diff --git a/app/models/supplier.rb b/app/models/supplier.rb index 3b54c56a..bb3cf887 100644 --- a/app/models/supplier.rb +++ b/app/models/supplier.rb @@ -6,6 +6,7 @@ class Supplier #devise :database_authenticatable, :recoverable, :rememberable, :trackable, :registerable, :confirmable property :name + property :email property :open, type: :boolean, default: false property :time_zone, default: 'UTC' property :night_offset, type: Fixnum, default: 0 # Minutes diff --git a/app/services/new_supplier.rb b/app/services/new_supplier.rb index 0be0991c..d5053769 100644 --- a/app/services/new_supplier.rb +++ b/app/services/new_supplier.rb @@ -1,19 +1,20 @@ class NewSupplier include ActiveAttr::Model attr_accessor :current_employee + attr_reader :employee, :supplier attribute :supplier_name attribute :email attribute :password attribute :password_confirmation - without_current_supplier = { unless: 'current_employee.present?' } + without_current_employee = { unless: 'current_employee.present?' } validates :supplier_name, presence: true - validates :email, email: without_current_supplier - validates :password, presence: without_current_supplier, confirmation: without_current_supplier + validates :email, email: without_current_employee + validates :password, presence: without_current_employee, confirmation: without_current_employee validate :supplier_name_uniqueness - validate :employee_uniqueness + validate :employee_uniqueness_or_existence def save if valid? @@ -27,15 +28,22 @@ class NewSupplier private def persist! - employee = Employee.new unconfirmed_email: email, password: password, password_confirmation: password_confirmation + @employee ||= Employee.new email: email, password: password, password_confirmation: password_confirmation raise "Cannot create employee with #{attributes.inspect}" unless employee.save - supplier = Supplier.create name: supplier_name + @supplier = Supplier.create name: supplier_name supplier.add_manager employee end - def employee_uniqueness + def employee_uniqueness_or_existence return if current_employee.present? - errors.add :email, :taken if Employee.count_by_email(email) > 0 + if Employee.count_by_email(email) > 0 + employee = Employee.find_by_email(email) + if employee.valid_password? password + @employee = employee + else + errors.add :email, :taken + end + end end def supplier_name_uniqueness diff --git a/app/views/layouts/supplier/app.html.slim b/app/views/layouts/supplier/app.html.slim index 2c9e60d5..924356a7 100644 --- a/app/views/layouts/supplier/app.html.slim +++ b/app/views/layouts/supplier/app.html.slim @@ -16,6 +16,5 @@ html lang="en" = javascript_include_tag 'supplier/app/application' = yield :head /= javascript_include_tag "https://maps.googleapis.com/maps/api/js?v=3.exp&sensor=false&libraries=places&language=#{I18n.locale}" - body #ember-app-container diff --git a/config/locales/supplier.en.yml b/config/locales/supplier.en.yml index 53277de4..96218beb 100644 --- a/config/locales/supplier.en.yml +++ b/config/locales/supplier.en.yml @@ -34,7 +34,7 @@ en: close_for_orders: Close the shop close_for_orders_confirmation: If you close the shop, you cannot receive orders. Are you sure? open_for_orders: 'Open up the place!' - you_are_currently_closed_alert: 'You are currently closed and not able to take orders' + you_are_currently_closed_alert: 'You are currently closed so cannot receive orders!' settings: Settings sign_out: Sign out table_number: Table diff --git a/config/routes.rb b/config/routes.rb index f237e523..9d5ac635 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -2,7 +2,11 @@ ALLOWED_LOCALES = /nl|be|de|fr|en/ Qwaiter::Application.routes.draw do devise_for :users, controllers: { omniauth_callbacks: "users/omniauth_callbacks" } #devise_for :suppliers, controllers: { confirmations: 'confirmations', registrations: 'registrations' } - devise_for :employees, controllers: { confirmations: 'confirmations', registrations: 'registrations' } + devise_for :employees, controllers: { + #confirmations: 'confirmations', + #registrations: 'registrations', + sessions: 'suppliers/sessions' + } devise_for :administrators namespace :admin do resources :users do diff --git a/spec/acceptance/suppliers/settings.feature b/spec/acceptance/suppliers/settings.feature index 05888211..8413d61f 100644 --- a/spec/acceptance/suppliers/settings.feature +++ b/spec/acceptance/suppliers/settings.feature @@ -18,13 +18,7 @@ Feature: Manage settings When I visit the supplier settings path And I provide a new supplier email address And the supplier submits the supplier settings form - #Then the supplier should see a settings saved message - Then the supplier email should not have been changed - And the supplier unconfirmed email should have been set to the new supplier email - And an email should have been sent to the original supplier email with email confirmation instructions - When the supplier clicks on the new email confirmation link - Then the supplier should be redirected to the supplier settings path - And the supplier email is the new email and the unconfirmed email is empty + Then the supplier email is the new email @javascript @broken Scenario: Setting the timezone diff --git a/spec/acceptance/suppliers/signup.feature b/spec/acceptance/suppliers/signup.feature index aa1e12ae..0220b135 100644 --- a/spec/acceptance/suppliers/signup.feature +++ b/spec/acceptance/suppliers/signup.feature @@ -6,8 +6,22 @@ Feature: A supplier can sign up And fill in the supplier signup form with new credentials And click on the supplier signup submit button And I wait 1 second - Then a new unconfirmed supplier with the new signup credentials should be created - And a supplier signup confirmation mail should be sent containing a link to the token - When I visit the supplier signup confirmation path with the correct token - Then the new signup supplier should be confirmed - And the supplier should be redirected to the supplier settings path + Then a new supplier with the new signup data should be created + And the supplier should be redirected to the supplier '/pages/introduction' path + + @javascript + Scenario: Existing non signed-in employee creates new supplier + Given there is a confirmed employee + When I visit the supplier signup path + And fill in the supplier signup form with existing employee credentials + And click on the supplier signup submit button + And I wait 1 second + Then a new supplier with the existing employee should be created + And the supplier should be redirected to the supplier '/pages/introduction' path + + +#Scenarios +#- non existing non signed in employee +#- existing non signed in employee +#- existing signed in emloyee +#- Supplier name exist for existing employee diff --git a/spec/acceptance_steps/global_employee_steps.rb b/spec/acceptance_steps/global_employee_steps.rb new file mode 100644 index 00000000..0f7a42b0 --- /dev/null +++ b/spec/acceptance_steps/global_employee_steps.rb @@ -0,0 +1,9 @@ + +step "there is a confirmed employee" do + @employee_password = 'admin123' + @employee = nil + expect { + @employee = create :employee, :confirmed, password: @employee_password + }.to change{ Employee.count }.by 1 +end + diff --git a/spec/acceptance_steps/suppliers/navigation_steps.rb b/spec/acceptance_steps/suppliers/navigation_steps.rb index 2fcbf678..24e36279 100644 --- a/spec/acceptance_steps/suppliers/navigation_steps.rb +++ b/spec/acceptance_steps/suppliers/navigation_steps.rb @@ -36,3 +36,9 @@ end step "the supplier employee visits the :path_spec path" do |path_spec| visit "/supplier#/#{path_spec}" end + +step "the supplier should be redirected to the supplier :path path" do |path| + # route_should_be 'supplier#edit' + ember_route_should_be path +end + diff --git a/spec/acceptance_steps/suppliers/settings_steps.rb b/spec/acceptance_steps/suppliers/settings_steps.rb index 274ce867..4a263f10 100644 --- a/spec/acceptance_steps/suppliers/settings_steps.rb +++ b/spec/acceptance_steps/suppliers/settings_steps.rb @@ -35,15 +35,9 @@ step "the supplier clicks on the new email confirmation link" do visit @confirmation_link end -step "the supplier should be redirected to the supplier settings path" do - # route_should_be 'supplier#edit' - ember_route_should_be '/settings' -end - -step "the supplier email is the new email and the unconfirmed email is empty" do +step "the supplier email is the new email" do @supplier.reload @supplier.email.should == 'new-supplier-mail@mozo.bar' - @supplier.unconfirmed_email.should be_blank end step "the supplier selects :time_zone as Time Zone" do |visual_time_zone| @@ -51,6 +45,7 @@ step "the supplier selects :time_zone as Time Zone" do |visual_time_zone| end step "the supplier timezone should be :time_zone" do |time_zone| + sleep 1 @supplier.reload @supplier.time_zone.should == time_zone end diff --git a/spec/acceptance_steps/suppliers/signup_steps.rb b/spec/acceptance_steps/suppliers/signup_steps.rb index d076338d..0ef35eb1 100644 --- a/spec/acceptance_steps/suppliers/signup_steps.rb +++ b/spec/acceptance_steps/suppliers/signup_steps.rb @@ -1,29 +1,50 @@ step "I visit the supplier signup path" do - visit new_supplier_registration_path + visit new_suppliers_path end step "fill in the supplier signup form with new credentials" do - js_set_field '#supplier_name', 'Signup Supplier' - js_set_field '#supplier_email', 'signup_supplier@example.com' - js_set_field '#supplier_password', 'SignupSupplier' - js_set_field '#supplier_password_confirmation', 'SignupSupplier' + js_set_field '#new_supplier_supplier_name', 'Signup Supplier' + js_set_field '#new_supplier_email', 'signup_supplier@example.com' + js_set_field '#new_supplier_password', 'SignupSupplier' + js_set_field '#new_supplier_password_confirmation', 'SignupSupplier' +end +step "fill in the supplier signup form with existing employee credentials" do + js_set_field '#new_supplier_supplier_name', 'Signup Supplier' + js_set_field '#new_supplier_email', @employee.email + js_set_field '#new_supplier_password', @employee_password + js_set_field '#new_supplier_password_confirmation', @employee_password end step "click on the supplier signup submit button" do find('[name="commit"]').click end -step "a new unconfirmed supplier with the new signup credentials should be created" do - @signup_supplier = Supplier.find_by_email('signup_supplier@example.com') +step "a new supplier with the new signup data should be created" do + @signup_supplier = Supplier.find_by_name('Signup Supplier') @signup_supplier.should be_present - @signup_supplier.should_not be_confirmed + + @signup_employee = Employee.find_by_email('signup_supplier@example.com') + @signup_employee.should be_present + + @employee_settings = @signup_supplier.settings_for(@signup_employee) + @employee_settings.should be_present + @employee_settings.manager?.should be true +end + +step "a new supplier with the existing employee should be created" do + @signup_supplier = Supplier.find_by_name('Signup Supplier') + @signup_supplier.should be_present + Employee.count.should eq 1 # No new employee, but existing + @employee_settings = @signup_supplier.settings_for(@employee) + @employee_settings.should be_present + @employee_settings.manager?.should be true end step "a supplier signup confirmation mail should be sent containing a link to the token" do mail = ActionMailer::Base.deliveries.last mail.should be_present - mail.body.should match /confirmation_token=#{@signup_supplier.confirmation_token}/ + mail.body.should match /confirmation_token=\w+/ end step "I visit the supplier signup confirmation path with the correct token" do diff --git a/spec/factories/employee_factory.rb b/spec/factories/employee_factory.rb index 2a11c79e..c1b5ac90 100644 --- a/spec/factories/employee_factory.rb +++ b/spec/factories/employee_factory.rb @@ -5,7 +5,7 @@ FactoryGirl.define do password 'secret' trait :confirmed do - confirmed_at { Time.now } + #confirmed_at { Time.now } end end end