Birthday field validation of user in rails app - ruby-on-rails

I added birthday to the users of my web app and am having trouble with the validations I want. It is a boyscout management app and I want scouts who sign up to have their age be between 12-18, but since birthdays are a new addition there are already scouts who do not have birthdays in my database. Thus, I would like to do something like before_create :validate_birthday, but I am not sure how to write that validation function. I believe it would be something like
def validate_birthday
if self.has_role?(:scout)
if !self.has_valid_dob?
errors.add(:birthday, "invalid. Must be 12-18 years of age")
end
else
if self.birthday.to_date != "06/06/1944".to_date
self.birthday = "06/06/1944".to_date
end
end
end
and
def has_valid_dob?
self.birthday.to_date > (18.years.ago).to_date && self.birthday < (12.years.ago).to_date
end
Now running this does not throw an error, but the error message does not appear saying "Birthday invalid. Must be 12-18 years of age". I think I'm close, but I'm not sure where to go from here. Thanks in advance.

You could use a Proc:
class User < ApplicationRecord
validate :valid_age, if: proc { |u| u.birthdate.present? }
protected
def valid_age
return if valid_date_range.include?(birthdate)
errors.add(:birthdate, 'invalid. Must be 12-18 years of age')
end
def valid_date_range
maximum_date..minimum_date
end
def minimum_date
12.years.ago.to_date
end
def maximum_date
18.years.ago.to_date
end
end

Related

Rubocop claims for too many 'if' statements

I am measuring my code for a Rails app using Rubocop. This is my code:
def ratings_treatment
if #rating.advertise_id
advertise = Advertise.find(advertise_id)
rating_average(advertise)
elsif #rating.product_id
product = Product.find(product_id)
rating_average(product)
elsif #rating.combo_id
combo = Combo.find(combo_id)
rating_average(combo)
else
establishment = Establishment.find(#rating.establishment_id)
rating_average(establishment)
end
end
It does not pass a test regarding if. It is claimed that there are too many if statements.
I thought of splitting the code into some checker methods, but was wondering if there is some better way to get good metrics and still write good code.
If your #rating has associations set up then you could say:
def ratings_treatment
obj = #rating.advertise || #rating.product || #rating.combo || #rating.establishment
rating_average(obj)
end
or even:
def ratings_treatment
rating_average(#rating.advertise || #rating.product || #rating.combo || #rating.establishment)
end
or perhaps something fancy (and overkill for only four branches):
def ratings_treatment
rating_average(
%w[advertise product combo establishment].lazy.map { |m| #rating.send(m) }.find(&:present?)
)
end
or maybe even:
def rated_object
to_result = ->(m) { #rating.send(m) }
%w[advertise product combo establishment]
.lazy
.map(&to_result)
.find(&:present?)
end
def ratings_treatment
rating_average(rated_object)
end
I'd go so far as to suggest that something like the above rated_object method really should be a method on #rating. You should be able to ask a rating for the thing that was rated without having to manually spin through all the possibilities.
You could even patch that lazy.map... stuff into Enumerable if you wanted something in Enumerable that behaved like e.m1 || e.m2 || e.m3 || ... (been there, done that) all the way down to short circuiting.
I have no idea what Rubocop would make of any of these shenanigans though.
I think you could use a switch statement to assign the argument for rating_average:
def ratings_treatment
average = case
when #rating.advertise_id then Advertise.find(advertise_id)
when #rating.product_id then Product.find(product_id)
when #rating.combo_id then Combo.find(combo_id)
else Establishment.find(#rating.establishment_id)
end
rating_average average
end
Although Rubocop will complain with
Style/EmptyCaseCondition: Do not use empty case condition, instead use
an if expression
You could also simplify your if expression:
def ratings_treatment
average = if #rating.advertise_id
Advertise.find advertise_id
elsif #rating.product_id
Product.find product_id
elsif #rating.combo_id
Combo.find combo_id
else
Establishment.find #rating.establishment_id
end
rating_average average
end
This is a classic case of a style violation that has a deeper underlying cause, i.e. a modelling problem. In your case, it looks like what you're actually trying to achieve is a polymorphic association. If you change your Rating to:
# app/models/rating.rb
class Rating < ApplicationRecord
belongs_to :rateable, polymorphic: true
end
you can have the Rating relate to any number of things, by simply adding this to the other end of the association:
# app/models/product.rb
class Product < ApplicationRecord
has_many :pictures, as: :rateable
end
Your original method, if it is even needed anymore, becomes:
def ratings_treatment
rating_average(#rating.rateable)
end

Increment field within validator

I have a custom validator that checks if the user has entered the correct SMS code. When the user enters the wrong code I need to log the failed attempt and limit their retries to 3 per code.
I have created the following validator that works however the field is not being incremented.
def token_match
if token != User.find(user_id).verification_token
User.find(user_id).increment!(:verification_fails)
errors.add(:sms_code, "does not match")
end
end
The problem is as soon as I add the error the previous statement is rolled back. If I comment out the errors.add line then the increment works however there is no higher level validation performed.
Change your custom validator to be:
def token_match
if token != User.find(user_id).verification_token
errors.add(:sms_code, "does not match")
end
end
and add in your model after_validation callback to be like this:
after_validation: increase_fails_count
def increase_fails_count
unless self.errors[:sms_code].empty?
user = User.find_by(:id => user_id)
user.increment!(:verification_fails)
user.save
end
end
You can use #update_columns in your validator. It writes directly to db.
u = User.find(user_id)
u.update_columns(verification_fails: u.verification_fails + 1)
This worked for me. But if for some reason it doesn't work for you, maybe you can try running it in a new thread,which creates a new db connection:
Thread.new do
num = User.find(user_id).verification_fails
ActiveRecord::Base.connection_pool.with_connection { |con| con.exec_query("UPDATE users SET verification_fails = #{num} WHERE id = #{user_id}") }
end.join

Rails: error charting by time

I am creating a chart based on account balance. And here is my some of my codes
module AccountsHelper
def products_chart_data
orders_by_day = Account.total_grouped_by_day(3.days.ago)
(3.days.ago.to_date..Date.today).map do |date|
{
created_at: date,
balance: orders_by_day[date].first.try(:total_balance) || 0
}
end
end
end
class Account < ActiveRecord::Base
belongs_to :user
has_many :books
def self.total_grouped_by_day(start)
balances = where(created_at: start.beginning_of_day..Time.zone.now)
balances = balances.group("date(created_at)")
balances = balances.select("created_at, balance as total_balance")
balances.group_by {|o| o.created_at.to_date }
end
end
My problems are:
1) I received an error undefined method `first' when mapping 3.days.ago, but successfully run the code when I change it to 2.days.ago. I understand that it is because I do not have data on 3 days ago as this account is new. My question is, how can I rescue this error, because I could have many other new accounts that do not have data yet, and what could I do to display result for 1 month, or 2 month?
Thanks in advance!
# ⇓⇓⇓⇓⇓⇓⇓⇓⇓⇓⇓
balance: orders_by_day[date].try(:first).try(:total_balance) || 0
try is the method, introduced by rails and defined on Object class, therefore it is defined on NilClass as well.
The implementation is quite straightforward: it checks whether receiver is empty and returns either the result of the call to it, or nil otherwise.

Why doesn't my Object update?

I have this test:
describe 'check_account_status' do
it 'should send the correct reminder email one week prior to account disablement' do
# Three weeks since initial email
reverification = create(:reverification)
initial_notification = reverification.twitter_reverification_sent_at.to_datetime
ActionMailer::Base.deliveries.clear
Timecop.freeze(initial_notification + 21) do
Reverification.check_account_status
ActionMailer::Base.deliveries.size.must_equal 1
ActionMailer::Base.deliveries.first.subject.must_equal I18n.t('.account_mailer.one_week_left.subject')
reverification.reminder_sent_at.class.must_equal ActiveSupport::TimeWithZone
reverification.notification_counter.must_equal 1
must_render_template 'reverification.html.haml'
end
end
This test produces this error:
check_account_status#test_0001_should send the correct reminder email one week prior to account disablement [/Users/drubio/vms/ohloh-ui/test/models/reverification_test.rb:67]:
Expected: ActiveSupport::TimeWithZone
Actual: NilClass
Here is my code:
class Reverification < ActiveRecord::Base
belongs_to :account
FIRST_NOTIFICATION_ERROR = []
class << self
def check_account_status
Reverification.where(twitter_reverified: false).each do |reverification|
calculate_status(reverification)
one_week_left(reverification)
end
end
private
def calculate_status(reverification)
#right_now = Time.now.utc.to_datetime
#initial_email_date = reverification.twitter_reverification_sent_at.to_datetime
#notification_counter = reverification.notification_counter
end
def one_week_left(reverification)
# Check to see if three weeks have passed since the initial email
# and check to see if its before the one day notification before
# marking an account as spam.
if (#right_now.to_i >= (#initial_email_date + 21).to_i) && (#right_now.to_i < (#initial_email_date + 29).to_i)
begin
AccountMailer.one_week_left(reverification.account).deliver_now
rescue
FIRST_NOTIFICATION_FAILURE << account.id
return
end
update_reverification_fields(reverification)
end
end
def update_reverification_fields(reverification)
reverification.notification_counter += 1
reverification.reminder_sent_at = Time.now.utc
reverification.save!
reverification.reload
end
end
Forgive the indentation, but what seems to be the problem, is that my reverification object doesn't update when it leaves the check_account_status method. I've placed puts statements through out the code and I can see without a doubt that the reverification object is indeed updating. However after it leaves the update_reverification_fields and returns to the test block, the fields are not updated. Why is that? Has anyone encountered this?
I believe you have a scope issue, the methods you call from check_account_status certainly don't return the updated object back to your method and Ruby only passes parameters by value.
Try something like this instead:
def check_account_status
Reverification.where(twitter_reverified: false).each do |reverification|
reverification = calculate_status(reverification)
reverification = one_week_left(reverification)
end
end
private
def calculate_status(reverification)
# ...
reverification
end
def one_week_left(reverification)
# ...
reverification = update_reverification_fields(reverification)
reverification
end
def update_reverification_fields(reverification)
# ...
reverification
end
The problem is that reverification object in your test and objects inside of check_account_status are different instances of the same model.
def update_reverification_fields(reverification)
reverification.notification_counter += 1
reverification.reminder_sent_at = Time.now.utc
reverification.save!
reverification.reload
end
This reload here, it's doing nothing. Let's walk through your test.
# check_account_status runs a DB query, finds some objects and does things to them
Reverification.check_account_status
# this expectation succeeds because you're accessing `deliveries` for the
# first time and you don't have it cached. So you get the actual value
ActionMailer::Base.deliveries.size.must_equal 1
# this object, on the other hand, was instantiated before
# `check_account_status` was called and, naturally, it doesn't see
# the database changes that completely bypassed it.
reverification.reminder_sent_at.class.must_equal ActiveSupport::TimeWithZone
So, before making expectations on reverification, reload it, so that it pulls latest data from the DB.
reverification.reload # here
reverification.reminder_sent_at.class.must_equal ActiveSupport::TimeWithZone

Rails validating search params

I have an API which is fairly restful but am struggling to work out how to implement a search cleanly. I want to be able to search for all the records between two date-times, the date-times are allowed to be a maximum of 6 hours apart. At the moment in my controller method I have the following:
required_params = [:start_time, :end_time]
if check_required_params(required_params, params) and check_max_time_bound(params, 6.hours)
... rest of controller code here ...
end
check_required_params is an application method that looks like this:
def check_required_params(required_params, params_sent)
required_params.each do |param|
unless has_param(param, params_sent)
unprocessable_entity
return false
end
end
true
end
check_max_time is fairly similar.
I know it's against best practices to do validation in the controller but I can't see how I can add it to the model cleanly.
Actually what you are doing is (almost) best practice and will (almost) be incorporated in Rails 4 with strong parametsers. (I say almost because your check_max_time looks like it should be a validation in your model.)
You should go ahead and pull in the feature today and make upgrades easier on yourself. Strong Parameters https://github.com/rails/strong_parameters
Documentation is there, but here is how you incorporate it.
class SearchController < ApplicationController
include ActiveModel::ForbiddenAttributesProtection
def create
# Doesn't have to be an ActiveRecord model
#results = Search.create(search_params)
respond_with #results
end
private
def search_params
# This will ensure that you have :start_time and :end_time, but will allow :foo and :bar
params.require(:start_time, :end_time).permit(:foo, :bar #, whatever else)
end
end
class Search < ActiveRecord::Base
validates :time_less_than_six_hours
private
def time_less_than_six_hours
errors.add(:end_time, "should be less than 6 hours from start") if (end_time - start_time) > 6.hours
end
end
Never found a clean answer for this. However if you're making an API Grape has inbuilt Parameter Validation and Coercion to take care of it.
Well, what I would do in this scenario is the set the default value between these two datetimes so that i won't have to do validation and raise the exception.
class SearchController < ApplicationController
before_filter :assign_default_params
def index
end
private
def assign_default_params
params[:start_time] ||= Time.now
params[:end_time] ||= params[:start_time] + 6.hours
params[:end_time] = params[:start_time] + 6.hours if ((params[:end_time] - params[:start_time]) / 3600).round) > 6
end
end
With this code above, it always has the params required to the search. The method assign_default_params try to assign default values if they are not sent from the clients. The last thing it does is that it assign params[:end_time] to a maximum value.
It's much neater because we don't have to do validation and the client won't need to handle different response code such as 422. And you should have an API documentation, stating about this fact as well.

Resources