Changing a user status flag, with validation

Question

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?

• 200_success: What does "kyc status" mean, and when/why would you want to change it?
• VoidZero: 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.

Answer 1

(削除) 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

• Mast: Could you expand your answer a bit? As it currently stands it's awfully minimal.
• kjprice: Good point @mast. After looking at the question again, I think it's incorrect anyway.