2
\$\begingroup\$

I am writing a function which on update of any attribute of model sets the status of variable is_kyc_verified to false.

Here is the code of the User model and the method which changes the status:

class User < ActiveRecord::Base
  # Include default devise modules. Others available are:
  # :confirmable, :lockable, :timeoutable and :omniauthable
  devise :database_authenticatable, :registerable,
         :recoverable, :rememberable, :trackable, :validatable
  before_update :change_kyc_status, unless: :is_kyc_verified_changed?

  #Associations
  has_one  :address, dependent: :destroy
  has_one  :kyc, dependent: :destroy
  has_one  :pan_detail, dependent: :destroy
  has_one  :document, dependent: :destroy
  has_many :nominee_details, dependent: :destroy
  has_many :bank_details, dependent: :destroy

  #Accept Attributes for associated model

  accepts_nested_attributes_for :address, :kyc, :pan_detail,
                                :document, :nominee_details, :bank_details,
                                allow_destroy: true, reject_if: :all_blank


  #validates
  validates :name, :mobile_no, :gender, :dob,  presence: true
  validates :mobile_no, numericality: true, length: { is: 10 }

  private
  ##
  # Check if is_kyc_verified is set to true
  # if 'yes' then alert user and set is_kyc_verified to false
  def change_kyc_status
    self.is_kyc_verified = false if self.valid? and self.is_kyc_verified.present?
    true
  end

end

As you can see the method initially used to return the self.is_kyc_verified which was false, which in turn resulted in "rollback of transaction" so I explicitly added a true at the end so it won't "rollback the transaction"

However I feel, that this is not the right way to implement this function. Can you please review my code and suggest me the right way to do so?

\$\endgroup\$
  • \$\begingroup\$ What does "kyc status" mean, and when/why would you want to change it? \$\endgroup\$ – 200_success Mar 21 '16 at 16:40
  • \$\begingroup\$ is_kyc_verified is flag which indicates that the customer has verified his know your customer details. but if he edits his personal information again i.e after he is being verified then he has to re-verify his details hence I need to set is_kyc_verified to false if he updates any info. \$\endgroup\$ – VoidZero Mar 21 '16 at 16:48
1
\$\begingroup\$

Maybe just change the before_update to execute when #changed? is true?

http://api.rubyonrails.org/classes/ActiveModel/Dirty.html#method-i-changed-3F

EDIT: After looking at the question again, I'd change my answer. I think it's reasonable to return true. There's not really anyway around it, considering the way callbacks handle the return value. I suppose you could invert the values of the is_kyc_verified to be something like is_kyc_unverified?. But, that already seems confusing.

I do however think you should move the unless: :is_kyc_verified_changed? into the change_kyc_status method. I find the current flow a little hard to follow, as you must check in two places if is_kyc_verified will be set.

I might change it to something like:

before_update :confirm_is_kyc_verified
...
def confirm_is_kyc_verified
  if !is_kyc_verified.changed? && is_kyc_verified && valid?
    is_kyc_verified = false
  end

  true
end
\$\endgroup\$
  • \$\begingroup\$ Could you expand your answer a bit? As it currently stands it's awfully minimal. \$\endgroup\$ – Mast Aug 19 '16 at 10:03
  • \$\begingroup\$ Good point @mast. After looking at the question again, I think it's incorrect anyway. \$\endgroup\$ – kjprice Aug 19 '16 at 17:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.