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

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,7 +8,7 @@ class User < ApplicationRecord
   CONTRIBUTORS = { pilou: "6788", aquaj: "449", louis: "19049", aurelie: "9168" }.freeze
 
   belongs_to :batch, optional: true
-  belongs_to :city, optional: true
+  belongs_to :city, optional: true, touch: true
   belongs_to :squad, optional: true, touch: true
   belongs_to :referrer, class_name: "User", optional: true
 
@@ -42,13 +42,8 @@ def self.from_kitt(auth)
 
     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) do |b|
-        city = oldest_batch&.city
-        b.city = City.find_or_initialize_by(name: city&.slug)
-      end
-
-      u.city = u.batch.city
+      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&.slug)
     end
 
     user.github_username = auth.info.github_nickname

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/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_030521) 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|
@@ -398,7 +395,6 @@
   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"

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/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
     [

spec/presenters/scores/user_scores_presenter_spec.rb

@@ -7,16 +7,17 @@
   let!(:bordeaux) { create :city, name: "Bordeaux" }
   let!(:squad_1) { create :squad, name: "The Killers" }
   let!(:squad_2) { create :squad, name: "Grouplove" }
-  let!(:batch_1) { create :batch, number: 1, city: paris }
-  let!(:batch_2) { create :batch, number: 2, city: bordeaux }
-  let!(:batch_3) { create :batch, number: 3, city: bordeaux }
+  let!(:batch_1) { create :batch, number: 1 }
+  let!(:batch_2) { create :batch, number: 2 }
+  let!(:batch_3) { create :batch, number: 3 }
 
   let!(:user_1) do
     create :user,
            id: 1,
            username: "Saunier",
            squad: squad_1,
            batch: batch_1,
+           city: paris,
            entered_hardcore: false
   end
   let!(:user_2) do
@@ -25,6 +26,7 @@
            username: "pil0u",
            squad: squad_2,
            batch: batch_2,
+           city: bordeaux,
            entered_hardcore: true
   end
 
@@ -150,10 +152,21 @@
     end
   end
 
+  context "when user has no city" do
+    before do
+      user_1.update!(city: nil)
+    end
+
+    it "includes no info about it in the output" do
+      expect(described_class.new(input).get).to include(
+        hash_including(uid: 1, city_vanity_name: nil)
+      )
+    end
+  end
+
   context "when user has no batch" do
     before do
-      user_1.batch_id = nil
-      user_1.save(validate: false)
+      user_1.update!(batch: nil)
     end
 
     it "includes no info about it in the output" do