Improve code
This commit is contained in:
parent
734e2c6828
commit
b9c708a61e
8 changed files with 74 additions and 7 deletions
|
@ -10,7 +10,7 @@ class PassportConfirmationsController < ApplicationController
|
|||
ActiveRecord::Base.transaction do
|
||||
ConfirmPassport.call(passport: @passport,
|
||||
user: current_user).tap do |context|
|
||||
authorize context.passport_confirmation
|
||||
authorize_if_present context.passport_confirmation
|
||||
end
|
||||
end
|
||||
|
||||
|
@ -22,4 +22,12 @@ private
|
|||
def set_passport
|
||||
@passport = Passport.find params[:passport_id]
|
||||
end
|
||||
|
||||
def authorize_if_present(record)
|
||||
if record
|
||||
authorize record
|
||||
else
|
||||
skip_authorization
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
@ -9,10 +9,12 @@ class ConfirmPassport
|
|||
end
|
||||
|
||||
def create_passport_confirmation
|
||||
context.passport_confirmation =
|
||||
passport_confirmation =
|
||||
context.passport.passport_confirmations.build user: context.user
|
||||
|
||||
context.fail! unless context.passport_confirmation.save
|
||||
context.fail! passport_confirmation: nil unless passport_confirmation.save
|
||||
|
||||
context.passport_confirmation = passport_confirmation
|
||||
end
|
||||
|
||||
def confirm_passport
|
||||
|
|
|
@ -5,4 +5,14 @@ class PassportConfirmation < ApplicationRecord
|
|||
belongs_to :user
|
||||
|
||||
validates :user_id, uniqueness: { scope: :passport_id }
|
||||
|
||||
validate :passport_has_image
|
||||
|
||||
private
|
||||
|
||||
def passport_has_image
|
||||
return if passport.nil?
|
||||
|
||||
errors.add :passport, 'must have an image' if passport.image.nil?
|
||||
end
|
||||
end
|
||||
|
|
|
@ -7,7 +7,7 @@
|
|||
|
||||
<hr/>
|
||||
|
||||
<% if user_signed_in? %>
|
||||
<% if @passport.image && user_signed_in? %>
|
||||
<% if @passport.passport_confirmations.where(user: current_user).exists? %>
|
||||
<div role="alert" class="alert alert-success">
|
||||
<%= translate '.instruction_confirmed' %>
|
||||
|
|
|
@ -2,7 +2,7 @@
|
|||
|
||||
FactoryBot.define do
|
||||
factory :passport_confirmation do
|
||||
passport
|
||||
association :passport, factory: :passport_with_image
|
||||
user
|
||||
end
|
||||
end
|
||||
|
|
|
@ -5,7 +5,7 @@ require 'rails_helper'
|
|||
RSpec.describe ConfirmPassport do
|
||||
subject { described_class.call passport: passport, user: user }
|
||||
|
||||
let!(:passport) { create :passport }
|
||||
let!(:passport) { create :passport_with_image }
|
||||
let!(:user) { create :user }
|
||||
|
||||
specify do
|
||||
|
@ -34,6 +34,22 @@ RSpec.describe ConfirmPassport do
|
|||
expect { subject }.not_to change { passport.reload.confirmed? }.from(false)
|
||||
end
|
||||
|
||||
context 'when passport has no image' do
|
||||
let!(:passport) { create :passport }
|
||||
|
||||
specify do
|
||||
expect(subject).to be_failure
|
||||
end
|
||||
|
||||
specify do
|
||||
expect(subject).to have_attributes(
|
||||
passport: passport,
|
||||
user: user,
|
||||
passport_confirmation: nil,
|
||||
)
|
||||
end
|
||||
end
|
||||
|
||||
context 'when confirmations count is almost enough' do
|
||||
before do
|
||||
(Passport::REQUIRED_CONFIRMATIONS - 1).times do
|
||||
|
|
|
@ -11,4 +11,7 @@ RSpec.describe PassportConfirmation do
|
|||
it do
|
||||
is_expected.to validate_uniqueness_of(:user_id).scoped_to(:passport_id)
|
||||
end
|
||||
|
||||
it { is_expected.not_to allow_value(create(:passport)).for :passport }
|
||||
it { is_expected.to allow_value(create(:confirmed_passport)).for :passport }
|
||||
end
|
||||
|
|
|
@ -3,7 +3,7 @@
|
|||
require 'rails_helper'
|
||||
|
||||
RSpec.describe 'POST /passports/:passport_id/passport_confirmations' do
|
||||
let!(:passport) { create :passport }
|
||||
let!(:passport) { create :passport_with_image }
|
||||
|
||||
def make_request
|
||||
post "/passports/#{passport.id}/passport_confirmations"
|
||||
|
@ -147,4 +147,32 @@ RSpec.describe 'POST /passports/:passport_id/passport_confirmations' do
|
|||
end
|
||||
end
|
||||
end
|
||||
|
||||
context 'when passport has no image' do
|
||||
let!(:passport) { create :passport }
|
||||
|
||||
let(:current_user) { create :user }
|
||||
|
||||
before do
|
||||
sign_in current_user
|
||||
end
|
||||
|
||||
specify do
|
||||
expect { make_request }.not_to \
|
||||
change(PassportConfirmation, :count).from(0)
|
||||
end
|
||||
|
||||
specify do
|
||||
expect { make_request }.not_to \
|
||||
change { passport.reload.confirmed? }.from(false)
|
||||
end
|
||||
|
||||
context 'after request' do
|
||||
before { make_request }
|
||||
|
||||
specify do
|
||||
expect(response).to redirect_to passport
|
||||
end
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
Reference in a new issue