Commit 907b59ab authored by Reuben Pereira's avatar Reuben Pereira

Refactor based on reviewer comments

- Move initialization of user.credit_card_validation to view.
- Refactor controller to use permitted_params for credit card
  validation.
parent 7894b19e
...@@ -46,7 +46,6 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -46,7 +46,6 @@ class Admin::UsersController < Admin::ApplicationController
end end
def edit def edit
user.credit_card_validation || user.build_credit_card_validation
user user
end end
...@@ -210,12 +209,8 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -210,12 +209,8 @@ class Admin::UsersController < Admin::ApplicationController
user_params_with_pass.merge!(password_params) user_params_with_pass.merge!(password_params)
end end
cc_validation = params.dig(:user, :credit_card_validation_attributes, :credit_card_validated_at) cc_validation_params = process_credit_card_validation_params(user_params_with_pass.delete(:credit_card_validation_attributes))
if cc_validation == "1" && !user.credit_card_validated_at user_params_with_pass.merge!(cc_validation_params)
user_params_with_pass[:credit_card_validation_attributes] = { credit_card_validated_at: Time.zone.now }
elsif cc_validation == "0" && user.credit_card_validated_at
user.credit_card_validation.destroy
end
respond_to do |format| respond_to do |format|
result = Users::UpdateService.new(current_user, user_params_with_pass.merge(user: user)).execute do |user| result = Users::UpdateService.new(current_user, user_params_with_pass.merge(user: user)).execute do |user|
...@@ -261,6 +256,27 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -261,6 +256,27 @@ class Admin::UsersController < Admin::ApplicationController
protected protected
def process_credit_card_validation_params(cc_validation_params)
return unless cc_validation_params && cc_validation_params[:credit_card_validated_at]
cc_validation = cc_validation_params[:credit_card_validated_at]
if cc_validation == "1" && !user.credit_card_validated_at
{
credit_card_validation_attributes: {
credit_card_validated_at: Time.zone.now
}
}
elsif cc_validation == "0" && user.credit_card_validated_at
{
credit_card_validation_attributes: {
_destroy: true
}
}
end
end
def paginate_without_count? def paginate_without_count?
counts = Gitlab::Database::Count.approximate_counts([User]) counts = Gitlab::Database::Count.approximate_counts([User])
...@@ -338,7 +354,8 @@ class Admin::UsersController < Admin::ApplicationController ...@@ -338,7 +354,8 @@ class Admin::UsersController < Admin::ApplicationController
:twitter, :twitter,
:username, :username,
:website_url, :website_url,
:note :note,
credit_card_validation_attributes: [:credit_card_validated_at]
] ]
end end
......
...@@ -321,7 +321,7 @@ class User < ApplicationRecord ...@@ -321,7 +321,7 @@ class User < ApplicationRecord
accepts_nested_attributes_for :user_preference, update_only: true accepts_nested_attributes_for :user_preference, update_only: true
accepts_nested_attributes_for :user_detail, update_only: true accepts_nested_attributes_for :user_detail, update_only: true
accepts_nested_attributes_for :credit_card_validation, update_only: true accepts_nested_attributes_for :credit_card_validation, update_only: true, allow_destroy: true
state_machine :state, initial: :active do state_machine :state, initial: :active do
event :block do event :block do
......
...@@ -50,6 +50,7 @@ ...@@ -50,6 +50,7 @@
= s_('AdminUsers|Automatically marked as default internal user') = s_('AdminUsers|Automatically marked as default internal user')
.form-group.row .form-group.row
- @user.credit_card_validation || @user.build_credit_card_validation
= f.fields_for :credit_card_validation do |ff| = f.fields_for :credit_card_validation do |ff|
.col-sm-2.col-form-label.gl-pt-0 .col-sm-2.col-form-label.gl-pt-0
= ff.label s_("AdminUsers|Validate user account") = ff.label s_("AdminUsers|Validate user account")
......
...@@ -652,7 +652,7 @@ RSpec.describe Admin::UsersController do ...@@ -652,7 +652,7 @@ RSpec.describe Admin::UsersController do
end end
end end
context 'when updating validate user account' do context 'when updating credit card validation for user account' do
let(:params) do let(:params) do
{ {
id: user.to_param, id: user.to_param,
...@@ -719,6 +719,26 @@ RSpec.describe Admin::UsersController do ...@@ -719,6 +719,26 @@ RSpec.describe Admin::UsersController do
it_behaves_like 'no credit card validation param' it_behaves_like 'no credit card validation param'
end end
context 'invalid parameters' do
let(:user_params) do
{ credit_card_validation_attributes: { credit_card_validated_at: Time.current.iso8601 } }
end
it_behaves_like 'no credit card validation param'
end
context 'with non permitted params' do
let(:user_params) do
{ credit_card_validation_attributes: { _destroy: true } }
end
before do
user.create_credit_card_validation!(credit_card_validated_at: Time.zone.now)
end
it_behaves_like 'no credit card validation param'
end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment