From 4d8e5207b914347a3f39a1a5ead268eaaba72db4 Mon Sep 17 00:00:00 2001 From: Eric Schultz Date: Fri, 17 Apr 2020 17:51:59 -0500 Subject: [PATCH] Add initial verification for Nonprofit registration through model --- app/models/nonprofit.rb | 40 ++++++---- .../api/nonprofits_controller_spec.rb | 79 ++++--------------- spec/models/nonprofit_spec.rb | 41 ++++++++++ spec/support/time_helpers.rb | 4 + 4 files changed, 84 insertions(+), 80 deletions(-) create mode 100644 spec/support/time_helpers.rb diff --git a/app/models/nonprofit.rb b/app/models/nonprofit.rb index d60cbc70..5d89a682 100755 --- a/app/models/nonprofit.rb +++ b/app/models/nonprofit.rb @@ -2,7 +2,7 @@ # License: AGPL-3.0-or-later WITH Web-Template-Output-Additional-Permission-3.0-or-later class Nonprofit < ApplicationRecord - attr_accessor :register_np_only, :user + attr_accessor :register_np_only, :user_id, :user Categories = ['Public Benefit', 'Human Services', 'Education', 'Civic Duty', 'Human Rights', 'Animals', 'Environment', 'Health', 'Arts, Culture, Humanities', 'International', 'Children', 'Religion', 'LGBTQ', "Women's Rights", 'Disaster Relief', 'Veterans'].freeze # :name, # str @@ -80,9 +80,6 @@ class Nonprofit < ApplicationRecord has_one :billing_plan, through: :billing_subscription has_one :miscellaneous_np_info - ##only meaningful on registration - has_one :user - validates :name, presence: true validates :city, presence: true validates :state_code, presence: true @@ -90,7 +87,8 @@ class Nonprofit < ApplicationRecord validates_uniqueness_of :slug, scope: %i[city_slug state_code_slug] validates_presence_of :slug - validates_presence_of :user, on: :create, unless: -> {register_np_only} + validates_presence_of :user_id, on: :create, unless: -> {register_np_only} + validate :user_is_valid, on: :create, unless: -> {register_np_only} validate :user_registerable_as_admin, on: :create, unless: -> {register_np_only} scope :vetted, -> { where(vetted: true) } @@ -110,13 +108,10 @@ class Nonprofit < ApplicationRecord before_validation(on: :create) do set_slugs + set_user self end - after_validation(on: :create) do - correct_nonunique_slug - end - after_create :build_admin_role, unless: -> {register_np_only} # Register (create) a nonprofit with an initial admin @@ -159,19 +154,32 @@ class Nonprofit < ApplicationRecord unless state_code_slug self.state_code_slug = Format::Url.convert_to_slug state_code end + if Nonprofit.where(slug: slug, city_slug: city_slug, state_code_slug: state_code_slug).any? + correct_nonunique_slug + end self end def correct_nonunique_slug - if errors[:slug] - begin - slug = SlugNonprofitNamingAlgorithm.new(self.state_code_slug, self.city_slug).create_copy_name(self.slug) - self.slug = slug - rescue UnableToCreateNameCopyError - end + begin + slug = SlugNonprofitNamingAlgorithm.new(self.state_code_slug, self.city_slug).create_copy_name(self.slug) + self.slug = slug + rescue UnableToCreateNameCopyError + errors.add(:slug, "another nonprofit admin") end end + def set_user + if (user_id && User.where(id: user_id).any?) + @user = User.find(user_id) + end + self + end + + def user_is_valid + (user && user.is_a?(User)) || errors.add(:user_id, "is not a valid user") + end + def full_address Format::Address.full_address(address, city, state_code) end @@ -222,7 +230,7 @@ class Nonprofit < ApplicationRecord def user_registerable_as_admin if user && user.roles.nonprofit_admins.any? - errors.add(:user, "cannot already be an admin for a nonprofit.") + errors.add(:user_id, "cannot already be an admin for a nonprofit.") end end private diff --git a/spec/controllers/api/nonprofits_controller_spec.rb b/spec/controllers/api/nonprofits_controller_spec.rb index 739dc206..874e0a27 100644 --- a/spec/controllers/api/nonprofits_controller_spec.rb +++ b/spec/controllers/api/nonprofits_controller_spec.rb @@ -1,15 +1,14 @@ require 'rails_helper' describe Api::NonprofitsController, type: :request do - it 'do things' do - - post '/api/nonprofits', params: {nonprofit: {name: 'hathatoh'}, user: {email: 'thoahtoa'}} - - byebug - expect(response.code).to eq 400 - end - + let(:user) { create(:user)} + let(:nonprofit_admin_role) do + role = user.roles.build(host: nonprofit, name: 'nonprofit_admin') + role.save! + role + end + let(:nonprofit) {create(:nm_justice)} describe 'get' do end @@ -34,23 +33,6 @@ describe Api::NonprofitsController, type: :request do h.with_indifferent_access end - let(:totally_empty_errors) do - { - errors: - [ - h(params: ['nonprofit[name]'], messages: gr_e('presence', 'blank')), - h(params: ['nonprofit[zip_code]'], messages: gr_e('presence', 'blank')), - h(params: ['nonprofit[state_code]'], messages: gr_e('presence', 'blank')), - h(params: ['nonprofit[city]'], messages: gr_e('presence', 'blank')), - - h(params: ['user[name]'], messages: gr_e('presence', 'blank')), - h(params: ['user[email]'], messages: gr_e('presence', 'blank')), - h(params: ['user[password]'], messages: gr_e('presence', 'blank')), - h(params: ['user[password_confirmation]'], messages: gr_e('presence', 'blank')) - ] - - }.with_indifferent_access - end describe 'authorization' do around(:each) do |e| Rails.configuration.action_controller.allow_forgery_protection = true @@ -89,21 +71,6 @@ describe Api::NonprofitsController, type: :request do expect_validation_errors(JSON.parse(response.body), expected) end - it 'should reject unmatching passwords ' do - input = { - - user: { - email: 'wmeil@email.com', - name: 'name', - password: 'password', - password_confirmation: 'doesn\'t match' - } - } - post :create, params: input, xhr: true - expect(response.code).to eq '400' - expect(JSON.parse(response.body)['errors']).to include(h(params: ['user[password]', 'user[password_confirmation]'], messages: gr_e('is_equal_to'))) - end - it 'attempts to make a slug copy and returns the proper errors' do force_create(:nonprofit, slug: 'n', state_code_slug: 'wi', city_slug: 'appleton') input = { @@ -125,30 +92,14 @@ describe Api::NonprofitsController, type: :request do ]) end - it 'errors on attempt to add user with email that already exists' do - force_create(:user, email: 'em@em.com') - - input = { - nonprofit: { name: 'n', state_code: 'WI', city: 'appleton', zip_code: 54_915 }, - user: { name: 'Name', email: 'em@em.com', password: '12345678', password_confirmation: '12345678' } - } - - expect do - post :create, params: input, xhr: true - end.to raise_error {|error| - - expect(error).to be_a Errors::MessageInvalid - } - byebug - expect(response.code).to eq '400' - - expect_validation_errors(JSON.parse(response.body), - errors: [ - h( - params: ['user[email]'], - messages: ['has already been taken'] - ) - ]) + it 'errors on attempt to add the user to a second nonprofit' do + nonprofit_admin_role + input = { name: 'n', state_code: 'WI', city: 'appleton', zip_code: 54_915, user_id: user.id } + post '/api/nonprofits', params: input, xhr:true + expect(response).to have_http_status :unprocessable_entity + expect(response.parsed_body['errors'].keys).to match_array 'user' + byebug + expect(response.parsed_body['errors']['user'].first).to has_include? 'nonprofit admin' end it 'succeeds' do diff --git a/spec/models/nonprofit_spec.rb b/spec/models/nonprofit_spec.rb index a61da1ce..98cfd286 100644 --- a/spec/models/nonprofit_spec.rb +++ b/spec/models/nonprofit_spec.rb @@ -43,4 +43,45 @@ RSpec.describe Nonprofit, type: :model do expect(nonprofit.currency_symbol).to eq euro end end + + describe 'create' do + describe 'validates on parameters' do + let(:nonprofit) { Nonprofit.new()} + let(:nonprofit_with_invalid_user) { Nonprofit.new(user_id: 3333)} + let(:nonprofit_with_user_who_already_admin) {nonprofit_admin_role; Nonprofit.new(user_id: user.id)} + let(:user) { create(:user)} + let(:nonprofit_admin_role) do + role = user.roles.build(host: nonprofit, name: 'nonprofit_admin') + role.save! + role + end + let(:nm_justice) {create(:nm_justice)} + + before(:each) { nonprofit.valid?; nonprofit_with_invalid_user.valid?; nonprofit_with_user_who_already_admin.valid?} + it 'has an error for no name' do + expect(nonprofit.errors['name'].first).to match /.*blank.*/ + end + + it 'has an error for no user' do + expect(nonprofit.errors['user_id'].first).to match /.*blank.*/ + end + + it 'has an error for no city' do + expect(nonprofit.errors['city'].first).to match /.*blank.*/ + end + + it 'has an error for no state' do + expect(nonprofit.errors['state_code'].first).to match /.*blank.*/ + end + + it 'rejects an invalid user' do + expect(nonprofit_with_invalid_user.errors['user_id'].first).to match /.*not a valid user.*/ + end + + it 'rejects a user who is already an admin' do + byebug + expect(nonprofit_with_user_who_already_admin.errors['user_id'].first).to match /.*admin.*/ + end + end + end end diff --git a/spec/support/time_helpers.rb b/spec/support/time_helpers.rb new file mode 100644 index 00000000..2cd3ea89 --- /dev/null +++ b/spec/support/time_helpers.rb @@ -0,0 +1,4 @@ +require 'active_support/testing/time_helpers' +RSpec.configure do |config| + config.include ActiveSupport::Testing::TimeHelpers +end \ No newline at end of file