Skip to content

Commit

Permalink
Remove Game#[started|ended]_at in favor of transactions
Browse files Browse the repository at this point in the history
Now that we have GameStartTransaction and GameEndTransaction we can
transition over to relying on these records instead of the old
Game#started_at and Game#ended_at attributes.

To support this:
- Remove the old columns from the database
- Update named scopes to join/merge on the Game[Start|End]Transaction
  scopes instead
- Move GameTransaction.create_between into the subclasses, so they can
  do the right thing for building object associations (in memory).
  Otherwise, we end up with a Game/User that knows about the new
  GameTransaction record, but not vice-versa... until reload.
  • Loading branch information
Paul DobbinSchmaltz committed Nov 14, 2024
1 parent b1d9ffd commit 33ccde8
Show file tree
Hide file tree
Showing 10 changed files with 161 additions and 66 deletions.
29 changes: 14 additions & 15 deletions app/models/game.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,6 @@
# @attr type [String] The preset or Game type used to generate this Game's
# {Board} / {Grid} of {Cell}s.
# Note: This attribute/column is *not* used for STI.
# @attr started_at [DateTime] When this Game started. i.e. transitioned from
# {#status} "Standing By" to "Sweep in Progress".
# @attr ended_at [DateTime] When this Game ended. i.e. transitioned from
# {#status} "Sweep in Progress" to either "Alliance Wins" or "Mines Win".
# @attr score [Integer] The play-time in seconds of a victorious Game. Maxes out
# at {Game::CalcStats::MAX_SCORE} (999).
# @attr bbbv [Integer] The 3BV value for the associated, solved {Board}.
Expand Down Expand Up @@ -74,9 +70,14 @@ class Game < ApplicationRecord # rubocop:disable Metrics/ClassLength
scope :for_game_over_statuses, -> {
for_status([status_alliance_wins, status_mines_win])
}
scope :for_ended_at, ->(time_range) { where(ended_at: time_range) }
scope :for_ended_at, ->(time_range) {
joins(:game_end_transaction).merge(
GameEndTransaction.for_created_at(time_range))
}

scope :by_most_recently_ended, -> { order(ended_at: :desc) }
scope :by_most_recently_ended, -> {
joins(:game_end_transaction).merge(GameEndTransaction.by_most_recent)
}
scope :by_score_asc, -> { where.not(score: nil).order(score: :asc) }
scope :by_bbbvps_desc, -> { where.not(bbbvps: nil).order(bbbvps: :desc) }
scope :by_efficiency_desc, -> {
Expand All @@ -93,9 +94,8 @@ def self.find_or_create_current(...)
end

def self.current(within: DEFAULT_JUST_ENDED_DURATION)
for_game_on_statuses.or(
for_game_over_statuses.for_ended_at(within.ago..)).
last
for_game_on_statuses.last ||
for_game_over_statuses.for_ended_at(within.ago..).last
end

def self.create_for(user:, **)
Expand Down Expand Up @@ -127,7 +127,6 @@ def start(seed_cell:, user:)
return self unless status_standing_by?

transaction do
touch(:started_at)
GameStartTransaction.create_between(user:, game: self)
board.on_game_start(seed_cell:)
set_status_sweep_in_progress!
Expand Down Expand Up @@ -168,6 +167,8 @@ def engagement_time_range
started_at..(ended_at if over?)
end

def started_at = game_start_transaction&.created_at
def ended_at = game_end_transaction&.created_at
def duration = ended_at - started_at

def board_settings = board&.settings
Expand All @@ -178,7 +179,6 @@ def end_game(user:)
return self if over?

transaction do
touch(:ended_at)
GameEndTransaction.create_between(user:, game: self)
yield
end
Expand Down Expand Up @@ -249,7 +249,7 @@ def render
def reset
do_reset {
set_status_sweep_in_progress! if over?
update(started_at: Time.current)
game_start_transaction.update(created_at: Time.current)
board.reset
}
end
Expand All @@ -258,8 +258,8 @@ def reset
# the {Board}.
def reset!
do_reset {
game_start_transaction&.delete
set_status_standing_by!
update(started_at: nil)
board.reset!
}
end
Expand Down Expand Up @@ -291,13 +291,12 @@ def do_reset

transaction do
update!(
ended_at: nil,
score: nil,
bbbv: nil,
bbbvps: nil,
efficiency: nil)

game_transactions.clear
game_end_transaction&.delete
CellTransaction.for_id(cell_transaction_ids).delete_all

yield
Expand Down
5 changes: 4 additions & 1 deletion app/models/transactions/game_transaction.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,13 @@ class GameTransaction < ApplicationRecord
scope :for_user, ->(user) { where(user:) }
scope :for_game, ->(game) { where(game:) }

scope :for_created_at, ->(time_range) { where(created_at: time_range) }

validates :game, uniqueness: { scope: :type }

# :reek:UnusedParams
def self.create_between(user:, game:)
new(user:, game:).tap(&:save!)
raise(NotImplementedError)
end

def self.exists_between?(user:, game:)
Expand Down
11 changes: 4 additions & 7 deletions db/migrate/20240809213612_create_games.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,11 @@ def up
:status,
unique: true,
where:
<<-SQL.squish
status IN
('#{Game.status_standing_by}', '#{Game.status_sweep_in_progress}')
SQL
)
Game.arel_table[:status].in([
Game.status_standing_by,
Game.status_sweep_in_progress,
]))

t.datetime(:started_at)
t.datetime(:ended_at, index: true)
t.float(:score, index: true)
t.integer(:bbbv, index: true)
t.float(:bbbvps, index: true)
Expand Down
9 changes: 0 additions & 9 deletions db/structure.sql
Original file line number Diff line number Diff line change
Expand Up @@ -176,8 +176,6 @@ CREATE TABLE public.games (
id bigint NOT NULL,
type character varying NOT NULL,
status character varying DEFAULT 'Standing By'::character varying NOT NULL,
started_at timestamp(6) with time zone,
ended_at timestamp(6) with time zone,
score double precision,
bbbv integer,
bbbvps double precision,
Expand Down Expand Up @@ -536,13 +534,6 @@ CREATE INDEX index_games_on_created_at ON public.games USING btree (created_at);
CREATE INDEX index_games_on_efficiency ON public.games USING btree (efficiency);


--
-- Name: index_games_on_ended_at; Type: INDEX; Schema: public; Owner: -
--

CREATE INDEX index_games_on_ended_at ON public.games USING btree (ended_at);


--
-- Name: index_games_on_score; Type: INDEX; Schema: public; Owner: -
--
Expand Down
4 changes: 0 additions & 4 deletions test/fixtures/games.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,10 @@ DEFAULTS: &DEFAULTS
win1:
<<: *DEFAULTS
status: <%= Game.status_alliance_wins %>
started_at: <%= 1.minute.ago %>
ended_at: <%= 30.seconds.ago %>

loss1:
<<: *DEFAULTS
status: <%= Game.status_mines_win %>
started_at: <%= 30.seconds.ago %>
ended_at: <%= 10.seconds.ago %>

standing_by1:
<<: *DEFAULTS
Expand Down
45 changes: 45 additions & 0 deletions test/models/game_create_transaction_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
# frozen_string_literal: true

require "test_helper"

class GameCreateTransactionTest < ActiveSupport::TestCase
describe "GameCreateTransaction" do
let(:unit_class) { GameCreateTransaction }

let(:user1) { users(:user1) }
let(:standing_by1) { games(:standing_by1) }

describe ".create_between" do
subject { unit_class }

context "GIVEN a new, unique pair" do
before do
standing_by1.game_create_transaction.delete
standing_by1.game_create_transaction = nil
end

it "creates the expected record, and returns it" do
result =
_(-> {
subject.create_between(user: user1, game: standing_by1)
}).must_change("GameCreateTransaction.count")
_(result).must_be_instance_of(subject)
_(result.user).must_be_same_as(user1)
_(result.game).must_be_same_as(standing_by1)
end
end

context "GIVEN an existing, non-unique pair" do
it "raises ActiveRecord::RecordInvalid" do
exception =
_(-> {
subject.create_between(user: user1, game: standing_by1)
}).must_raise(ActiveRecord::RecordInvalid)

_(exception.message).must_equal(
"Validation failed: Game has already been created")
end
end
end
end
end
42 changes: 42 additions & 0 deletions test/models/game_end_transaction_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# frozen_string_literal: true

require "test_helper"

class GameEndTransactionTest < ActiveSupport::TestCase
describe "GameEndTransaction" do
let(:unit_class) { GameEndTransaction }

let(:user1) { users(:user1) }

let(:win1) { games(:win1) }
let(:standing_by1) { games(:standing_by1) }

describe ".create_between" do
subject { unit_class }

context "GIVEN a new, unique pair" do
it "creates the expected record, and returns it" do
result =
_(-> {
subject.create_between(user: user1, game: standing_by1)
}).must_change("GameEndTransaction.count")
_(result).must_be_instance_of(subject)
_(result.user).must_be_same_as(user1)
_(result.game).must_be_same_as(standing_by1)
end
end

context "GIVEN an existing, non-unique pair" do
it "raises ActiveRecord::RecordInvalid" do
exception =
_(-> {
subject.create_between(user: user1, game: win1)
}).must_raise(ActiveRecord::RecordInvalid)

_(exception.message).must_equal(
"Validation failed: Game has already been ended")
end
end
end
end
end
42 changes: 42 additions & 0 deletions test/models/game_start_transaction_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
# frozen_string_literal: true

require "test_helper"

class GameStartTransactionTest < ActiveSupport::TestCase
describe "GameStartTransaction" do
let(:unit_class) { GameStartTransaction }

let(:user1) { users(:user1) }

let(:win1) { games(:win1) }
let(:standing_by1) { games(:standing_by1) }

describe ".create_between" do
subject { unit_class }

context "GIVEN a new, unique pair" do
it "creates the expected record, and returns it" do
result =
_(-> {
subject.create_between(user: user1, game: standing_by1)
}).must_change("GameStartTransaction.count")
_(result).must_be_instance_of(subject)
_(result.user).must_be_same_as(user1)
_(result.game).must_be_same_as(standing_by1)
end
end

context "GIVEN an existing, non-unique pair" do
it "raises ActiveRecord::RecordInvalid" do
exception =
_(-> {
subject.create_between(user: user1, game: win1)
}).must_raise(ActiveRecord::RecordInvalid)

_(exception.message).must_equal(
"Validation failed: Game has already been started")
end
end
end
end
end
6 changes: 3 additions & 3 deletions test/models/game_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class GameTest < ActiveSupport::TestCase
context "GIVEN a Game#on? = true Game doesn't exist" do
context "GIVEN a recently-ended Game exists" do
before do
recently_ended_game.touch(:started_at)
recently_ended_game.create_game_start_transaction!(user: user1)
recently_ended_game.end_in_victory(user: user1)
end

Expand Down Expand Up @@ -239,7 +239,7 @@ class GameTest < ActiveSupport::TestCase
context "GIVEN a Game that's still on" do
subject { standing_by1.start(seed_cell: nil, user: user1) }

it "touches Game#ended_at" do
it "updates Game#ended_at" do
_(-> { subject.end_in_victory(user: user1) }).must_change(
"subject.ended_at",
from: nil)
Expand Down Expand Up @@ -282,7 +282,7 @@ class GameTest < ActiveSupport::TestCase
context "GIVEN a Game that's still on" do
subject { standing_by1.start(seed_cell: nil, user: user1) }

it "touches Game#ended_at" do
it "updates Game#ended_at" do
_(-> { subject.end_in_defeat(user: user1) }).must_change(
"subject.ended_at",
from: nil)
Expand Down
34 changes: 7 additions & 27 deletions test/models/game_transaction_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,12 @@ class GameTransactionTest < ActiveSupport::TestCase
let(:standing_by1) { games(:standing_by1) }

describe "DB insertion (GIVEN no Rails validation)" do
subject { GameStartTransaction.new(user: user1, game: win1) }
subject { unit_class.take }

it "raises ActiveRecord::RecordNotUnique" do
exception =
_(-> {
subject.save(validate: false)
subject.dup.save(validate: false)
}).must_raise(ActiveRecord::RecordNotUnique)

_(exception.message).must_include(
Expand All @@ -29,32 +29,12 @@ class GameTransactionTest < ActiveSupport::TestCase
end

describe ".create_between" do
context "GIVEN a new, unique pair" do
subject { GameStartTransaction }

it "creates the expected GameTransaction record, and returns it" do
result =
_(-> {
subject.create_between(user: user2, game: standing_by1)
}).must_change("GameTransaction.count")
_(result).must_be_instance_of(subject)
_(result.user).must_be_same_as(user2)
_(result.game).must_be_same_as(standing_by1)
end
end

context "GIVEN an existing, non-unique pair" do
subject { [GameStartTransaction, GameEndTransaction].sample }

it "raises ActiveRecord::RecordInvalid" do
exception =
_(-> {
subject.create_between(user: user1, game: win1)
}).must_raise(ActiveRecord::RecordInvalid)
subject { unit_class }

_(exception.message).must_equal(
"Validation failed: Game has already been taken")
end
it "raises NotImplementedError" do
_(-> {
subject.create_between(user: user1, game: win1)
}).must_raise(NotImplementedError)
end
end

Expand Down

0 comments on commit 33ccde8

Please sign in to comment.