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

app/components/user_form_component.html.erb

@@ -15,8 +15,8 @@
   </div>
 
   <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.label :city_name, "Campus" %>
+    <%= 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,8 +4,7 @@ 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

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

db/seeds.rb

@@ -16,7 +16,20 @@
   Batch.destroy_all
 end
 
-KittScraperJob.perform_now(scrape_kitt: false)
+def fetch(url)
+  uri = URI(url)
+  https = Net::HTTP.new(uri.hostname, uri.port)
+  https.use_ssl = true
+
+  request = Net::HTTP::Get.new(uri)
+  request["Cookie"] = "_kitt2017_=#{ENV.fetch('KITT_SESSION_COOKIE')}"
+
+  https.request(request)
+end
+
+data = JSON.parse(fetch("https://kitt.lewagon.com/api/v1/users").read_body)
+cities = data["all_places"]
+cities.each { |city| City.find_or_create_by(name: city["label"], size: city["count"]) }
 
 Rails.logger.info "✔ Cities initialized"
 
@@ -26,49 +39,57 @@
   User.create!([
                  {
                    username: "test_1",
-                   batch: Batch.find_by(number: 343),
+                   batch: Batch.find_or_create_by(number: 343),
+                   city: City.find_by(name: "Paris"),
                    aoc_id: 151_323,
                    uid: 1
                  },
                  {
                    username: "test_2",
-                   batch: Batch.find_by(number: 454),
+                   batch: Batch.find_or_create_by(number: 454),
+                   city: City.find_by(name: "Paris"),
                    aoc_id: 1_095_582,
                    uid: 2
                  },
                  {
                    username: "test_3",
-                   batch: Batch.find_by(number: 123),
+                   batch: Batch.find_or_create_by(number: 123),
+                   city: City.find_by(name: "Bordeaux"),
                    aoc_id: 1_266_664,
                    uid: 3
                  },
                  {
                    username: "test_4",
-                   batch: Batch.find_by(number: 123),
+                   batch: Batch.find_or_create_by(number: 123),
+                   city: City.find_by(name: "London"),
                    aoc_id: 1_237_086,
                    uid: 4
                  },
                  {
                    username: "test_5",
-                   batch: Batch.find_by(number: 123),
+                   batch: Batch.find_or_create_by(number: 123),
+                   city: City.find_by(name: "London"),
                    aoc_id: 1_258_899,
                    uid: 5
                  },
                  {
                    username: "test_6",
-                   batch: Batch.find_by(number: 454),
+                   batch: Batch.find_or_create_by(number: 454),
+                   city: City.find_by(name: "Paris"),
                    aoc_id: 1_259_034,
                    uid: 6
                  },
                  {
                    username: "test_7",
-                   batch: Batch.find_by(number: 454),
+                   batch: Batch.find_or_create_by(number: 454),
+                   city: City.find_by(name: "Brussels"),
                    aoc_id: 1_259_062,
                    uid: 7
                  },
                  {
                    username: "test_8",
-                   batch: Batch.find_by(number: 343),
+                   batch: Batch.find_or_create_by(number: 343),
+                   city: City.find_by(name: "Paris"),
                    aoc_id: 1_259_379,
                    uid: 8
                  }

spec/domains/ranks/city_scores_spec.rb

@@ -4,9 +4,7 @@
 
 RSpec.describe Ranks::CityScores do
   let(:bordeaux) { create :city, name: "Bordeaux", size: 60 }
-  let(:bordeaux_batch) { create(:batch, number: 1, city: bordeaux) }
   let(:brussels) { create :city, name: "Brussels", size: 10 }
-  let(:brussels_batch) { create(:batch, number: 2, city: brussels) }
 
   let(:input) do
     [
@@ -34,10 +32,10 @@
 
     let!(:completions) do
       [
-        *create_list(:user, 11, batch: bordeaux_batch).map do |u|
+        *create_list(:user, 11, city: bordeaux).map do |u|
           create(:completion, user: u, day: 1, challenge: 1, completion_unix_time: Aoc.begin_time + 23.hours)
         end,
-        *create_list(:user, 10, batch: brussels_batch).map do |u|
+        *create_list(:user, 10, city: brussels).map do |u|
           create(:completion, user: u, day: 1, challenge: 1, completion_unix_time: Aoc.begin_time + 23.hours)
         end
       ]
@@ -60,17 +58,17 @@
 
       let!(:completions) do
         [
-          *create_list(:user, 12, batch: bordeaux_batch).map do |u|
+          *create_list(:user, 12, city: bordeaux).map do |u|
             create(:completion, user: u, day: 1, challenge: 1, completion_unix_time: Aoc.begin_time + 23.hours)
           end,
-          *create_list(:user, 10, batch: brussels_batch).map do |u|
+          *create_list(:user, 10, city: brussels).map do |u|
             create(:completion, user: u, day: 1, challenge: 1, completion_unix_time: Aoc.begin_time + 23.hours)
           end,
 
-          *create_list(:user, 11, batch: bordeaux_batch).map do |u|
+          *create_list(:user, 11, city: bordeaux).map do |u|
             create(:completion, user: u, day: 1, challenge: 2, completion_unix_time: Aoc.begin_time + 40.hours)
           end,
-          *create_list(:user, 9, batch: brussels_batch).map do |u|
+          *create_list(:user, 9, city: brussels).map do |u|
             create(:completion, user: u, day: 1, challenge: 2, completion_unix_time: Aoc.begin_time + 40.hours)
           end
         ]

spec/domains/scores/city_points_spec.rb

@@ -5,8 +5,7 @@
 RSpec.describe Scores::CityPoints do
   let!(:state) { create :state, completions_fetched: solo_points.count, fetch_api_end: Time.zone.now }
   let(:brussels) { create(:city, name: "Brussels", size: 400) }
-  let(:brussels_batch) { create(:batch, number: 1, city: brussels) }
-  let(:brussels_users) { create_list(:user, 18, batch: brussels_batch) }
+  let(:brussels_users) { create_list(:user, 18, city: brussels) }
   let(:brussels_points) do
     [
       { day: 1, challenge: 1, score: 50, user_id: brussels_users[0].id },
@@ -19,8 +18,7 @@
   end
 
   let(:bordeaux) { create(:city, name: "Bordeaux", size: 400) }
-  let(:bordeaux_batch) { create(:batch, number: 1, city: bordeaux) }
-  let(:bordeaux_users) { create_list(:user, 18, batch: bordeaux_batch) }
+  let(:bordeaux_users) { create_list(:user, 18, city: bordeaux) }
   let(:bordeaux_points) do
     [
       { day: 1, challenge: 1, score: 46, user_id: bordeaux_users[0].id }
@@ -164,7 +162,7 @@
       end
 
       context "when the users makeup of the city has changed in the meantime" do
-        let(:new_bordeaux_users) { create_list :user, 2, batch: bordeaux_batch }
+        let(:new_bordeaux_users) { create_list :user, 2, city: bordeaux }
 
         before do
           travel 10.seconds # Specs go too fast, updated_at stays the same otherwise

spec/domains/scores/city_scores_spec.rb

@@ -6,9 +6,7 @@
   let(:state) { create(:state, completions_fetched: 6, fetch_api_end: Time.zone.now) }
 
   let(:bordeaux) { create :city, name: "Bordeaux", size: 10 }
-  let(:bordeaux_batch) { create(:batch, number: 1, city: bordeaux) }
   let(:brussels) { create :city, name: "Brussels", size: 10 }
-  let(:brussels_batch) { create(:batch, number: 2, city: brussels) }
   let(:city_points) do
     [
       { day: 1, challenge: 1, city_id: brussels.id, contributor_count: 2, total_score: 96, score: 8.00 },
@@ -98,7 +96,7 @@
         before do
           travel 10.seconds # Specs go too fast, updated_at stays the same otherwise
           create(:state, fetch_api_begin: Time.zone.now, completions_fetched: 1)
-          create_list :user, 2, batch: bordeaux_batch # creating after travel
+          create_list :user, 2, city: bordeaux # creating after travel
           allow(Scores::CityPoints).to receive(:get).and_return(new_city_points)
         end
 

spec/factories/users.rb

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

spec/presenters/scores/city_scores_presenter_spec.rb

@@ -4,15 +4,12 @@
 
 RSpec.describe Scores::CityScoresPresenter do
   let!(:city_1) { create :city, id: 1, name: "Bordeaux", size: 200 }
-  let!(:batch_1) { create(:batch, number: 1, city: city_1) }
   let!(:city_2) { create :city, id: 2, name: "Rio de Janeiro", size: 100 }
-  let!(:batch_2) { create(:batch, number: 2, city: city_2) }
   let!(:city_3) { create :city, id: 3, name: "Paris", size: 500 }
-  let!(:batch_3) { create(:batch, number: 3, city: city_3) }
 
-  let!(:user_1) { create :user, id: 1, batch: batch_1 }
-  let!(:user_2) { create :user, id: 2, batch: batch_2 }
-  let!(:user_3) { create :user, id: 3, batch: batch_1 }
+  let!(:user_1) { create :user, id: 1, city: city_1 }
+  let!(:user_2) { create :user, id: 2, city: city_2 }
+  let!(:user_3) { create :user, id: 3, city: city_1 }
 
   let(:input) do
     [