diff --git a/.rubocop.yml b/.rubocop.yml index 4f95a4d6..253b9e70 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -13,6 +13,10 @@ Lint/MissingSuper: Lint/SymbolConversion: EnforcedStyle: consistent +Metrics/ClassLength: + Exclude: + - "**/app/models/user.rb" + Naming/VariableNumber: EnforcedStyle: snake_case Exclude: diff --git a/app/controllers/users_controller.rb b/app/controllers/users_controller.rb index d0097a09..e8075d2c 100644 --- a/app/controllers/users_controller.rb +++ b/app/controllers/users_controller.rb @@ -46,7 +46,12 @@ def edit end def update - if current_user.update(updated_params) + current_user.batch_id = -1 if params.dig(:user, :batch_number) # Set impossible value to trigger validation + + referrer_code = params.dig(:user, :referrer_code) + current_user.referrer_id = User.find_by_referral_code(referrer_code)&.id || -1 if referrer_code + + if current_user.update(user_params) unlock_achievements redirect_back fallback_location: "/", notice: "Your user information was updated" else @@ -85,7 +90,7 @@ def authenticate_admin end def restrict_after_lock - return unless Time.now.utc > Aoc.lock_time && (form_params[:entered_hardcore] == "1") != current_user.entered_hardcore + return unless Time.now.utc > Aoc.lock_time && (user_params[:entered_hardcore] == "1") != current_user.entered_hardcore redirect_back( fallback_location: "/", @@ -93,22 +98,11 @@ def restrict_after_lock ) end - def updated_params - { - accepted_coc: form_params[:accepted_coc], - aoc_id: form_params[:aoc_id], - entered_hardcore: form_params[:entered_hardcore], - username: form_params[:username], - city_id: form_params[:city_id], - referrer: User.find_by_referral_code(form_params[:referrer_code]) - }.compact - end - def unlock_achievements Achievements::UnlockJob.perform_later(:city_join, current_user.id) end - def form_params - params.require(:user).permit(:accepted_coc, :aoc_id, :entered_hardcore, :username, :city_id, :referrer_code) + def user_params + params.require(:user).permit(:accepted_coc, :aoc_id, :city_id, :entered_hardcore, :username) end end diff --git a/app/models/user.rb b/app/models/user.rb index 3a3ca4e0..a455fe6c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -26,10 +26,12 @@ class User < ApplicationRecord validates :aoc_id, numericality: { in: 1...(2**31), message: "should be between 1 and 2^31" }, allow_nil: true validates :aoc_id, uniqueness: { allow_nil: true } validates :username, presence: true - validates :private_leaderboard, presence: true - validate :not_referring_self + validate :batch_cannot_be_changed, on: :update, if: :batch_id_changed? + validate :city_cannot_be_changed_if_present, on: :update, if: :city_id_changed? + validate :referrer_must_exist, on: :update, if: :referrer_id_changed? + validate :referrer_cannot_be_self, on: :update scope :admins, -> { where(uid: ADMINS.values) } scope :confirmed, -> { where(accepted_coc: true, synced: true).where.not(aoc_id: nil) } @@ -39,11 +41,11 @@ class User < ApplicationRecord before_validation :assign_private_leaderboard, on: :create def self.from_kitt(auth) - batches = auth.info&.schoolings - oldest_batch = batches.min_by { |batch| batch.camp.starts_at } + oldest_batch = auth.info&.schoolings&.min_by { |batch| batch.camp.starts_at } user = find_or_initialize_by(provider: auth.provider, uid: auth.uid) do |u| u.username = auth.info.github_nickname + u.batch = Batch.find_or_initialize_by(number: oldest_batch&.camp&.slug.to_i) u.city = City.find_or_initialize_by(name: oldest_batch&.city&.name) end @@ -51,15 +53,13 @@ def self.from_kitt(auth) user.github_username = auth.info.github_nickname user.save - user end def self.find_by_referral_code(code) - uid = code&.gsub(/R0*/, "")&.to_i - return if uid.nil? + return unless code&.match?(/R\d{5}/) - User.find_by(uid:) + User.find_by(uid: code.gsub(/R0*/, "").to_i) end def admin? @@ -113,8 +113,20 @@ def referrer_code private - def not_referring_self - errors.add(:referrer, "must not be you") if referrer == self + def batch_cannot_be_changed + errors.add(:batch, "can't be changed") + end + + def city_cannot_be_changed_if_present + errors.add(:city, "can't be changed") if city_id_was.present? + end + + def referrer_must_exist + errors.add(:referrer, "must exist") unless User.exists?(referrer_id) + end + + def referrer_cannot_be_self + errors.add(:referrer, "can't be you (nice try!)") if referrer == self end def assign_private_leaderboard diff --git a/spec/presenters/scores/user_scores_presenter_spec.rb b/spec/presenters/scores/user_scores_presenter_spec.rb index 04001f82..ae367cba 100644 --- a/spec/presenters/scores/user_scores_presenter_spec.rb +++ b/spec/presenters/scores/user_scores_presenter_spec.rb @@ -154,7 +154,8 @@ context "when user has no city" do before do - user_1.update!(city: nil) + user_1.city = nil + user_1.save(validate: false) end it "includes no info about it in the output" do @@ -166,7 +167,8 @@ context "when user has no batch" do before do - user_1.update!(batch: nil) + user_1.batch = nil + user_1.save(validate: false) end it "includes no info about it in the output" do