Plug Batch info on Kitt callback and move back City on User to simpli…

…fy validation logic. (#325)

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

* Rubocop

* fix(users): revert some changes

* fix(users): undo everything from #289

* fix(users): use kitt city name instead of slug

* fix(users) last comments

* fix(users): remove empty city froms seed

* Wording + spacing

* Remove Testville

---------

Co-authored-by: Louis Ramos <louisramosdev@gmail.com>
Co-authored-by: pil0u <8350914+pil0u@users.noreply.github.com>

app/components/user_form_component.html.erb

@@ -16,7 +16,7 @@
 
   <div class="flex flex-col gap-y-1 sm:flex-row sm:items-center sm:gap-x-3">
     <%= f.label :campus_name, "Campus" %>
-    <%= f.collection_select(:city_id, City.order(:vanity_name), :id, :vanity_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

@@ -94,18 +94,12 @@ 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,
+      city_id: form_params[:city_id],
       referrer: User.find_by_referral_code(form_params[:referrer_code])
     }.compact
   end

app/domains/scores/city_points.rb

@@ -19,7 +19,7 @@ def compute
       points = Scores::SoloPoints.get
 
       # index for o(1) fetch
-      city_for_user = User.joins(:batch).where(id: points.pluck(:user_id)).pluck(:id, "batches.city_id").to_h
+      city_for_user = User.where(id: points.pluck(:user_id)).pluck(:id, :city_id).to_h
       city_users = points.group_by { |user| city_for_user[user[:user_id]] }
 
       city_users.without(nil).flat_map do |city_id, user_points|

app/models/batch.rb

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

app/models/city.rb

@@ -4,13 +4,12 @@ class City < ApplicationRecord
   has_many :city_points, class_name: "Cache::CityPoint", dependent: :delete_all
   has_many :city_scores, class_name: "Cache::CityScore", dependent: :delete_all
 
-  has_many :batches, dependent: :nullify
-  has_many :users, through: :batches
+  has_many :users, dependent: :nullify
   has_many :completions, through: :users
 
   before_create :set_default_vanity_name
 
-  validates :name, uniqueness: { case_sensitive: false }
+  validates :name, presence: true, uniqueness: { case_sensitive: false }
 
   def self.find_by_slug(slug)
     find_by!("REPLACE(LOWER(name), ' ', '-') = ?", slug)

app/models/user.rb

@@ -8,11 +8,10 @@ class User < ApplicationRecord
   CONTRIBUTORS = { pilou: "6788", aquaj: "449", louis: "19049", aurelie: "9168" }.freeze
 
   belongs_to :batch, optional: true
+  belongs_to :city, optional: true, touch: true
   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
@@ -38,13 +37,19 @@ 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.to_i)
+      u.city = City.find_or_initialize_by(name: oldest_batch&.city&.name)
     end
 
-    user.update(github_username: auth.info.github_nickname)
+    user.github_username = auth.info.github_nickname
+
+    user.save
+
     user
   end
 

app/presenters/scores/user_scores_presenter.rb

@@ -12,7 +12,7 @@ def initialize(scores)
     def get
       @scores ||= User # rubocop:disable Naming/MemoizedInstanceVariableName
                   .where(id: scores_per_user.keys)
-                  .includes(:squad, :completions, batch: :city)
+                  .includes(:squad, :completions, :city)
                   .map { |user| { **identity_of(user), **stats_of(user) } }
                   .sort_by { |user| user[:order] || Float::INFINITY }
     end

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/migrate/20231116174333_remove_city_and_size_from_batches.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+
+class RemoveCityAndSizeFromBatches < ActiveRecord::Migration[7.1]
+  def change
+    safety_assured { remove_column :batches, :city_id, :bigint }
+    safety_assured { remove_column :batches, :size, :int }
+  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_16_024537) do
+ActiveRecord::Schema[7.1].define(version: 2023_11_16_174333) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "citext"
   enable_extension "pgcrypto"
@@ -28,12 +28,9 @@
   end
 
   create_table "batches", force: :cascade do |t|
-    t.bigint "city_id"
     t.datetime "created_at", null: false
     t.integer "number"
-    t.integer "size"
     t.datetime "updated_at", null: false
-    t.index ["city_id"], name: "index_batches_on_city_id"
   end
 
   create_table "blazer_audits", force: :cascade do |t|
@@ -374,6 +371,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"
@@ -392,11 +390,11 @@
     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
 
   add_foreign_key "achievements", "users"
-  add_foreign_key "batches", "cities"
   add_foreign_key "city_points", "cities"
   add_foreign_key "city_scores", "cities"
   add_foreign_key "completions", "users"
@@ -409,5 +407,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