Plug Batch info on Kitt callback and move back City on User to simplify validation logic.

app/components/user_form_component.html.erb

@@ -11,12 +11,12 @@
 
   <div class="flex flex-col gap-y-1 sm:flex-row sm:items-center sm:gap-x-3">
     <%= f.label :batch_number, "Batch #" %>
-    <%= f.number_field :batch_number, value: @user.batch&.number, min: 1, class: "input", disabled: true %>
+    <%= f.number_field :batch_number, value: @user.batch&.number, min: 1, class: "input", disabled: @user.batch_id? %>
   </div>
 
   <div class="flex flex-col gap-y-1 sm:flex-row sm:items-center sm:gap-x-3">
     <%= f.label :city_name, "Campus" %>
-    <%= f.collection_select(:city_id, City.order(:name), :id, :name, { prompt: "", selected: @user.batch&.city_id }, class: "input", disabled: @user.batch&.city_id?) %>
+    <%= f.collection_select(:city_id, City.order(:vanity_name), :id, :vanity_name, { prompt: "", selected: @user.city_id }, class: "input", disabled: @user.city_id?) %>
   </div>
 
   <div class="flex items-center gap-x-3 w-48 sm:w-fit">

app/controllers/users_controller.rb

@@ -86,18 +86,13 @@ def restrict_after_lock
   end
 
   def updated_params
-    batch = nil
-
-    if current_user.batch_id.nil? && form_params[:city_id] # rubocop:disable Style/IfUnlessModifier
-      batch = Batch.find_or_create_by(number: nil, city_id: form_params[:city_id])
-    end
-
     {
       accepted_coc: form_params[:accepted_coc],
       aoc_id: form_params[:aoc_id],
       entered_hardcore: form_params[:entered_hardcore],
       username: form_params[:username],
-      batch_id: batch&.id,
+      batch_id: Batch.find_by(number: form_params[:batch_number])&.id,
+      city_id: form_params[:city_id],
       referrer: User.find_by_referral_code(form_params[:referrer_code])
     }.compact
   end
@@ -107,6 +102,6 @@ def unlock_achievements
   end
 
   def form_params
-    params.require(:user).permit(:accepted_coc, :aoc_id, :entered_hardcore, :username, :city_id, :referrer_code)
+    params.require(:user).permit(:accepted_coc, :aoc_id, :entered_hardcore, :username, :batch_number, :city_id, :referrer_code)
   end
 end

app/models/batch.rb

@@ -2,7 +2,7 @@
 
 class Batch < ApplicationRecord
   has_many :users, dependent: :nullify
-  belongs_to :city
+  belongs_to :city, optional: true
 
   validates :number, numericality: { in: 1...(2**31), message: "should be between 1 and 2^31" }, allow_nil: true
 end

app/models/user.rb

@@ -7,11 +7,11 @@ class User < ApplicationRecord
   CONTRIBUTORS = { pilou: "6788", aquaj: "449", louis: "19049", aurelie: "9168" }.freeze
 
   belongs_to :batch, optional: true
+  belongs_to :city, optional: true
+  has_one :batch_city, through: :batch, source: :city
   belongs_to :squad, optional: true, touch: true
   belongs_to :referrer, class_name: "User", optional: true
 
-  delegate :city, :city_id, to: :batch, allow_nil: true
-
   has_many :completions, dependent: :destroy
   has_many :user_day_scores, class_name: "Cache::UserDayScore", dependent: :delete_all
   has_many :solo_points, class_name: "Cache::SoloPoint", dependent: :delete_all
@@ -28,6 +28,8 @@ class User < ApplicationRecord
   validates :username, presence: true
 
   validate :not_referring_self
+  validate :not_changing_cities
+  validate :city_must_match_batch
 
   scope :admins, -> { where(uid: ADMINS.values) }
   scope :confirmed, -> { where(accepted_coc: true, synced: true).where.not(aoc_id: nil) }
@@ -37,13 +39,24 @@ class User < ApplicationRecord
   after_create :assign_private_leaderboard
 
   def self.from_kitt(auth)
-    user = where(provider: auth.provider, uid: auth.uid).first_or_create do |u|
+    batches = auth.info&.schoolings
+    oldest_batch = batches.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.github_username = auth.info.github_nickname
-      u.batch_id = Batch.find_or_create_by(number: auth.info.last_batch_slug.to_i).id
+
+      u.batch = Batch.find_or_initialize_by(number: oldest_batch&.camp&.slug) do |b|
+        city = oldest_batch&.city
+        b.city = City.find_or_initialize_by(name: city&.slug, vanity_name: city&.name)
+      end
+
+      u.city = u.batch.city
     end
 
-    user.update(github_username: auth.info.github_nickname)
+    user.github_username = auth.info.github_nickname
+
+    user.save
+
     user
   end
 
@@ -101,6 +114,18 @@ def not_referring_self
     errors.add(:referrer, "must not be you") if referrer == self
   end
 
+  def not_changing_cities
+    return if city_id_was.blank?
+
+    errors.add(:city_id, "can't be modified") if city_id_changed?
+  end
+
+  def city_must_match_batch
+    return unless batch&.city_id?
+
+    errors.add(:city_id, "must match batch city") if city_id != batch.city_id
+  end
+
   def assign_private_leaderboard
     return if private_leaderboard.present?
 

db/migrate/20231116030303_add_back_city_on_users.rb

@@ -0,0 +1,10 @@
+# frozen_string_literal: true
+
+class AddBackCityOnUsers < ActiveRecord::Migration[7.1]
+  disable_ddl_transaction!
+
+  def change
+    add_reference :users, :city, index: { algorithm: :concurrently }
+    add_foreign_key :users, :cities, validate: false
+  end
+end

db/migrate/20231116030521_add_foreign_key_for_users_city_relationship.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddForeignKeyForUsersCityRelationship < ActiveRecord::Migration[7.1]
+  def change
+    validate_foreign_key :users, :cities
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[7.1].define(version: 2023_11_01_144409) do
+ActiveRecord::Schema[7.1].define(version: 2023_11_16_030521) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
   enable_extension "pgcrypto"
@@ -374,6 +374,7 @@
     t.boolean "accepted_coc", default: false, null: false
     t.integer "aoc_id"
     t.bigint "batch_id"
+    t.bigint "city_id"
     t.datetime "created_at", null: false
     t.boolean "entered_hardcore", default: false, null: false
     t.string "github_username"
@@ -389,6 +390,7 @@
     t.string "username"
     t.index ["aoc_id"], name: "index_users_on_aoc_id", unique: true
     t.index ["batch_id"], name: "index_users_on_batch_id"
+    t.index ["city_id"], name: "index_users_on_city_id"
     t.index ["referrer_id"], name: "index_users_on_referrer_id"
   end
 
@@ -406,5 +408,6 @@
   add_foreign_key "squad_points", "squads"
   add_foreign_key "squad_scores", "squads"
   add_foreign_key "users", "batches"
+  add_foreign_key "users", "cities"
   add_foreign_key "users", "users", column: "referrer_id"
 end

spec/factories/users.rb

@@ -5,5 +5,6 @@
     username { "pil0u" }
     synced { true }
     uid { id }
+    city_id { batch&.city_id }
   end
 end