1
0
Fork 0
forked from OpenNeo/impress

Save last trade activity time onto User

In impress-2020, we do a big slow query to figure out which users have
been active in trades recently. Now, we cache that timestamp on the
User model.

This won't have any immediate effect; it's to clear the way for Classic
DTI to receive the better trade ratios feature people like from 2020.

I also added some unit testing infra because I finally wanted it! for
all the ways you can trigger this timestamp lol

Note too that this is a bit of an unusually complex migration, but my
hope is that the batching and query structure and such helps it run
surprisingly fast! 🤞
This commit is contained in:
Emi Matchu 2024-01-19 00:00:46 -08:00
parent b84942d77c
commit 470c805880
15 changed files with 394 additions and 25 deletions

View file

@ -27,6 +27,8 @@
// we partially override `config/database.yml` to connect to `db`!
"DATABASE_URL_PRIMARY_DEV": "mysql2://db",
"DATABASE_URL_OPENNEO_ID_DEV": "mysql2://db",
"DATABASE_URL_PRIMARY_TEST": "mysql2://db",
"DATABASE_URL_OPENNEO_ID_TEST": "mysql2://db",
// HACK: Out of the box, this dev container doesn't allow installation to
// the default GEM_HOME, because of a weird thing going on with RVM.

View file

@ -79,3 +79,8 @@ gem "stackprof", "~> 0.2.25"
# For monitoring errors in production.
gem "sentry-ruby", "~> 5.12"
gem "sentry-rails", "~> 5.12"
# For automated testing.
group :test do
gem 'sqlite3', '~> 1.7'
end

View file

@ -297,6 +297,8 @@ GEM
actionpack (>= 5.2)
activesupport (>= 5.2)
sprockets (>= 3.0.0)
sqlite3 (1.7.0)
mini_portile2 (~> 2.8.0)
stackprof (0.2.25)
stringio (3.0.8)
temple (0.8.2)
@ -359,6 +361,7 @@ DEPENDENCIES
sentry-rails (~> 5.12)
sentry-ruby (~> 5.12)
sprockets (~> 4.2)
sqlite3 (~> 1.7)
stackprof (~> 0.2.25)
terser (~> 1.1, >= 1.1.17)
thread-local (~> 1.1)

View file

@ -4,6 +4,7 @@ class ClosetHanger < ApplicationRecord
belongs_to :user
delegate :name, to: :item, prefix: true
delegate :log_trade_activity, to: :user
validates :item_id, :uniqueness => {:scope => [:user_id, :owned, :list_id]}
validates :quantity, :numericality => {:greater_than => 0}
@ -16,6 +17,32 @@ class ClosetHanger < ApplicationRecord
joins(:item => :translations).where(it[:locale].eq(I18n.locale)).
order(it[:name].asc)
}
scope :trading, -> {
ch = arel_table
cl = ClosetList.arel_table
u = User.arel_table
joins(:user, :list).where(
# sigh… our default-lists continue to be a pain
(
ch[:list_id].not_eq(nil).and(cl[:visibility].gteq(
ClosetVisibility[:trading].id))
).or(
(
ch[:list_id].eq(nil).and(ch[:owned].eq(true))
).and(
u[:owned_closet_hangers_visibility].gteq(
ClosetVisibility[:trading].id)
)
).or(
(
ch[:list_id].eq(nil).and(ch[:owned].eq(false))
).and(
u[:wanted_closet_hangers_visibility].gteq(
ClosetVisibility[:trading].id)
)
)
)
}
scope :newest, -> { order(arel_table[:created_at].desc) }
scope :owned_before_wanted, -> { order(arel_table[:owned].desc) }
scope :unlisted, -> { where(:list_id => nil) }
@ -36,6 +63,9 @@ class ClosetHanger < ApplicationRecord
before_validation :merge_quantities, :set_owned_by_list
after_save :log_trade_activity, if: :trading?
after_destroy :log_trade_activity, if: :trading?
def possibly_null_closet_list
list || user.null_closet_list(owned)
end

View file

@ -6,10 +6,15 @@ class ClosetList < ApplicationRecord
validates :user, :presence => true
validates :hangers_owned, :inclusion => {:in => [true, false], :message => "can't be blank"}
delegate :log_trade_activity, to: :user
scope :alphabetical, -> { order(:name) }
scope :publicly_visible, -> {
where(arel_table[:visibility].gteq(ClosetVisibility[:public].id))
}
scope :trading, -> {
where(arel_table[:visibility].gteq(ClosetVisibility[:trading].id))
}
scope :visible_to, ->(user) {
condition = arel_table[:visibility].gteq(ClosetVisibility[:public].id)
condition = condition.or(arel_table[:user_id].eq(user.id)) if user
@ -17,6 +22,12 @@ class ClosetList < ApplicationRecord
}
after_save :sync_hangers_owned!
after_save :log_trade_activity, if: :trading?
after_destroy :log_trade_activity, if: :trading?
def trading?
visibility >= ClosetVisibility[:trading].id
end
def sync_hangers_owned!
if hangers_owned_changed?

View file

@ -25,6 +25,12 @@ class User < ApplicationRecord
scope :top_contributors, -> { order('points DESC').where('points > 0') }
after_update :sync_name_with_auth_user!, if: :saved_change_to_name?
after_update :log_trade_activity, if: -> user {
(user.saved_change_to_owned_closet_hangers_visibility? &&
user.owned_closet_hangers_visibility >= ClosetVisibility[:trading].id) ||
(user.saved_change_to_wanted_closet_hangers_visibility? &&
user.wanted_closet_hangers_visibility >= ClosetVisibility[:trading].id)
}
def sync_name_with_auth_user!
auth_user.name = name
@ -169,6 +175,10 @@ class User < ApplicationRecord
contact_neopets_connection.try(:neopets_username)
end
def log_trade_activity
touch(:last_trade_activity_at)
end
def self.points_required_to_pass_top_contributor(offset)
user = User.top_contributors.select(:points).limit(1).offset(offset).first
user ? user.points : 0

View file

@ -24,6 +24,32 @@ development:
sql_mode: TRADITIONAL
migrations_paths: db/openneo_id_migrate
test:
primary:
# You can override these default settings with this environment variable,
# fully or partially. We do this in the .devcontainer setup!
url: <%= ENV['DATABASE_URL_PRIMARY_TEST'] %>
adapter: mysql2
database: openneo_impress_test
username: impress_dev
password: impress_dev
pool: 5
variables:
sql_mode: TRADITIONAL
openneo_id:
# You can override these default settings with this environment variable,
# fully or partially. We do this in the .devcontainer setup!
url: <%= ENV['DATABASE_URL_OPENNEO_ID_TEST'] %>
adapter: mysql2
database: openneo_id_test
username: impress_dev
password: impress_dev
pool: 2
variables:
sql_mode: TRADITIONAL
migrations_paths: db/openneo_id_migrate
production:
primary:
url: <%= ENV['DATABASE_URL_PRIMARY'] %>

View file

@ -35,7 +35,7 @@ Rails.application.configure do
config.action_controller.allow_forgery_protection = false
# Store uploaded files on the local file system in a temporary directory.
config.active_storage.service = :test
# config.active_storage.service = :test
config.action_mailer.perform_caching = false

View file

@ -9,7 +9,7 @@
# Make sure your secret_key_base is kept private
# if you're sharing your code publicly.
if Rails.env.development?
if Rails.env.development? || Rails.env.test?
# In development, we use a hardcoded secret key, because it doesn't actually
# need to be secret!
OpenneoImpressItems::Application.config.secret_key_base = "7584841652f89044a8b5a428efa6dfac2461449eb24741a33668cd642130d79f93b0347766ebf4a4d7d5033a263c36431594ad56b5735a7325c8cdda991219c2"

View file

@ -0,0 +1,36 @@
class AddLastTradeActivityAtToUsers < ActiveRecord::Migration[7.1]
def change
add_column :users, :last_trade_activity_at, :timestamp
reversible do |direction|
direction.up do
User.find_in_batches do |users|
# Find the last ClosetList/ClosetHanger updated_at timestamp for each
# user, for trading lists/hangers only.
max_closet_list_updated_at_by_user_id = ClosetList.
trading.
group(:user_id).
where(user_id: users.map(&:id)).
maximum(:updated_at)
max_closet_hanger_updated_at_by_user_id = ClosetHanger.
trading.
group(:user_id).
where(user_id: users.map(&:id)).
maximum(:updated_at)
# Set `last_trade_activity_at` to the largest such `updated_at` for
# that user, or nil if there's none.
User.transaction do
users.each do |user|
user.last_trade_activity_at = [
max_closet_list_updated_at_by_user_id[user.id],
max_closet_hanger_updated_at_by_user_id[user.id],
].filter(&:present?).max
user.save!
end
end
end
end
end
end
end

View file

@ -11,7 +11,7 @@
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.1].define(version: 2023_08_07_005748) do
create_table "users", id: { type: :integer, unsigned: true }, charset: "utf8mb3", force: :cascade do |t|
create_table "users", id: { type: :integer, unsigned: true }, charset: "utf8mb3", collation: "utf8mb3_general_ci", force: :cascade do |t|
t.string "name", limit: 20, null: false
t.string "encrypted_password", limit: 64, null: false
t.string "email", limit: 50, null: false

View file

@ -10,8 +10,8 @@
#
# It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
create_table "auth_servers", id: :integer, charset: "latin1", force: :cascade do |t|
ActiveRecord::Schema[7.1].define(version: 2024_01_19_061745) do
create_table "auth_servers", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.string "short_name", limit: 10, null: false
t.string "name", limit: 40, null: false
t.text "icon", size: :medium, null: false
@ -19,7 +19,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.string "secret", limit: 64, null: false
end
create_table "campaigns", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "campaigns", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "progress", default: 0, null: false
t.integer "goal", null: false
t.boolean "active", null: false
@ -33,7 +33,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.string "name"
end
create_table "closet_hangers", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "closet_hangers", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "item_id"
t.integer "user_id"
t.integer "quantity"
@ -49,7 +49,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.index ["user_id"], name: "index_closet_hangers_on_user_id"
end
create_table "closet_lists", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "closet_lists", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.string "name"
t.text "description"
t.integer "user_id"
@ -60,7 +60,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.index ["user_id"], name: "index_closet_lists_on_user_id"
end
create_table "color_translations", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "color_translations", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "color_id"
t.string "locale"
t.string "name"
@ -70,13 +70,13 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.index ["locale"], name: "index_color_translations_on_locale"
end
create_table "colors", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "colors", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.boolean "basic"
t.boolean "standard"
t.boolean "prank", default: false, null: false
end
create_table "contributions", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "contributions", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.string "contributed_type", limit: 8, null: false
t.integer "contributed_id", null: false
t.integer "user_id", null: false
@ -85,14 +85,14 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.index ["user_id"], name: "index_contributions_on_user_id"
end
create_table "donation_features", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "donation_features", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "donation_id", null: false
t.integer "outfit_id"
t.datetime "created_at", precision: nil, null: false
t.datetime "updated_at", precision: nil, null: false
end
create_table "donations", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "donations", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "amount", null: false
t.string "charge_id", null: false
t.integer "user_id"
@ -104,7 +104,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.integer "campaign_id", null: false
end
create_table "item_outfit_relationships", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "item_outfit_relationships", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "item_id"
t.integer "outfit_id"
t.boolean "is_worn"
@ -149,7 +149,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.index ["modeling_status_hint"], name: "items_modeling_status_hint"
end
create_table "login_cookies", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "login_cookies", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "user_id", null: false
t.integer "series", null: false
t.integer "token", null: false
@ -157,20 +157,20 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.index ["user_id"], name: "login_cookies_user_id"
end
create_table "modeling_logs", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "modeling_logs", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.datetime "created_at", precision: nil, null: false
t.text "log_json", null: false
t.string "pet_name", limit: 128, null: false
end
create_table "neopets_connections", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "neopets_connections", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "user_id"
t.string "neopets_username"
t.datetime "created_at", precision: nil, null: false
t.datetime "updated_at", precision: nil, null: false
end
create_table "outfits", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "outfits", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "pet_state_id"
t.integer "user_id"
t.datetime "created_at", precision: nil
@ -193,7 +193,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.index ["swf_asset_id"], name: "parents_swf_assets_swf_asset_id"
end
create_table "pet_loads", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "pet_loads", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.string "pet_name", limit: 20, null: false
t.text "amf", size: :medium, null: false
t.datetime "created_at", precision: nil, null: false
@ -225,17 +225,17 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.index ["species_id", "color_id"], name: "pet_types_species_color", unique: true
end
create_table "pets", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "pets", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.string "name", limit: 20, null: false
t.integer "pet_type_id", limit: 3, null: false
t.index ["name"], name: "pets_name", unique: true
t.index ["pet_type_id"], name: "pets_pet_type_id"
end
create_table "species", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "species", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
end
create_table "species_translations", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "species_translations", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "species_id"
t.string "locale"
t.string "name"
@ -267,7 +267,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.index ["zone_id"], name: "idx_swf_assets_zone_id"
end
create_table "users", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "users", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.string "name", limit: 20, null: false
t.integer "auth_server_id", limit: 1, null: false
t.integer "remote_id", null: false
@ -278,9 +278,10 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.integer "owned_closet_hangers_visibility", default: 1, null: false
t.integer "wanted_closet_hangers_visibility", default: 1, null: false
t.integer "contact_neopets_connection_id"
t.timestamp "last_trade_activity_at"
end
create_table "zone_translations", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "zone_translations", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "zone_id"
t.string "locale"
t.string "label"
@ -291,7 +292,7 @@ ActiveRecord::Schema[7.1].define(version: 2023_11_11_234255) do
t.index ["zone_id"], name: "index_zone_translations_on_zone_id"
end
create_table "zones", id: :integer, charset: "latin1", force: :cascade do |t|
create_table "zones", id: :integer, charset: "latin1", collation: "latin1_swedish_ci", force: :cascade do |t|
t.integer "depth"
t.integer "type_id"
end

13
test/test_helper.rb Normal file
View file

@ -0,0 +1,13 @@
ENV["RAILS_ENV"] = "test"
require File.expand_path('../../config/environment', __FILE__)
require 'rails/test_help'
class ActiveSupport::TestCase
# Setup all fixtures in test/fixtures/*.(yml|csv) for all tests in alphabetical order.
#
# Note: You'll currently still have to declare fixtures explicitly in integration tests
# -- they do not yet inherit this setting
fixtures :all
# Add more helper methods to be used by all tests here...
end

232
test/trade_activity_test.rb Normal file
View file

@ -0,0 +1,232 @@
require 'test_helper'
class TradeActivityTest < ActiveSupport::TestCase
test "New user's last trade activity is nil" do
user = create_user
assert_nil user.last_trade_activity_at
end
test "Adding or removing items in a Trading list updates last trade activity" do
user = create_user
list = create_closet_list(
user: user, visibility: ClosetVisibility[:trading].id)
hanger = create_closet_hanger(user: user, list: list)
created_at = Time.now
assert_equal created_at, user.last_trade_activity_at
travel 1.day
hanger.destroy!
assert_equal created_at + 1.day, user.last_trade_activity_at
end
test "Adding or removing items in a Public list does not update last trade activity" do
user = create_user
list = create_closet_list(
user: user, visibility: ClosetVisibility[:public].id)
hanger = create_closet_hanger(user: user, list: list)
assert_nil user.last_trade_activity_at
travel 1.day
hanger.destroy!
assert_nil user.last_trade_activity_at
end
test "Adding or removing items in a Private list does not update last trade activity" do
user = create_user
list = create_closet_list(
user: user, visibility: ClosetVisibility[:private].id)
hanger = create_closet_hanger(user: user, list: list)
assert_nil user.last_trade_activity_at
travel 1.day
hanger.destroy!
assert_nil user.last_trade_activity_at
end
test "Adding or removing items in a Trading default-list updates last trade activity" do
user = create_user(
owned_closet_hangers_visibility: ClosetVisibility[:trading].id,
wanted_closet_hangers_visibility: ClosetVisibility[:private].id,
)
hanger = create_closet_hanger(user: user, owned: true)
created_at = Time.now
assert_equal created_at, user.last_trade_activity_at
travel 1.day
hanger.destroy!
assert_equal created_at + 1.day, user.last_trade_activity_at
end
test "Adding or removing items in a Public default-list does not update last trade activity" do
user = create_user(
owned_closet_hangers_visibility: ClosetVisibility[:public].id,
wanted_closet_hangers_visibility: ClosetVisibility[:private].id,
)
hanger = create_closet_hanger(user: user, owned: true)
assert_nil user.last_trade_activity_at
travel 1.day
hanger.destroy!
assert_nil user.last_trade_activity_at
end
test "Adding or removing items in a Private default-list does not update last trade activity" do
user = create_user(
owned_closet_hangers_visibility: ClosetVisibility[:private].id,
wanted_closet_hangers_visibility: ClosetVisibility[:private].id,
)
hanger = create_closet_hanger(user: user, owned: true)
assert_nil user.last_trade_activity_at
travel 1.day
hanger.destroy!
assert_nil user.last_trade_activity_at
end
test "Creating, editing, or deleting a Trading list updates last trade activity" do
user = create_user
list = create_closet_list(
user: user, visibility: ClosetVisibility[:trading].id
)
created_at = Time.now
assert_equal created_at, user.last_trade_activity_at
travel 1.day
list.update!(description: "Hello, world!")
assert_equal created_at + 1.day, user.last_trade_activity_at
travel 1.day
list.destroy!
assert_equal created_at + 2.day, user.last_trade_activity_at
end
test "Creating, editing, or deleting a Public list does not update last trade activity" do
user = create_user
list = create_closet_list(
user: user, visibility: ClosetVisibility[:public].id
)
assert_nil user.last_trade_activity_at
travel 1.day
list.update!(description: "Hello, world!")
assert_nil user.last_trade_activity_at
travel 1.day
list.destroy!
assert_nil user.last_trade_activity_at
end
test "Creating, editing, or deleting a Private list does not update last trade activity" do
user = create_user
list = create_closet_list(
user: user, visibility: ClosetVisibility[:private].id
)
assert_nil user.last_trade_activity_at
travel 1.day
list.update!(description: "Hello, world!")
assert_nil user.last_trade_activity_at
travel 1.day
list.destroy!
assert_nil user.last_trade_activity_at
end
test "Updating default-list visibility to Trading updates last trade activity" do
user = create_user(
owned_closet_hangers_visibility: ClosetVisibility[:private].id,
)
assert_nil user.last_trade_activity_at
user.update!(
owned_closet_hangers_visibility: ClosetVisibility[:trading].id,
)
assert_equal Time.now, user.last_trade_activity_at
end
test "Updating default-list visibility to Public does not update last trade activity" do
user = create_user(
owned_closet_hangers_visibility: ClosetVisibility[:private].id,
)
assert_nil user.last_trade_activity_at
user.update!(
owned_closet_hangers_visibility: ClosetVisibility[:public].id,
)
assert_nil user.last_trade_activity_at
end
test "Updating default-list visibility to Private does not update last trade activity" do
user = create_user(
owned_closet_hangers_visibility: ClosetVisibility[:public].id,
)
assert_nil user.last_trade_activity_at
user.update!(
owned_closet_hangers_visibility: ClosetVisibility[:private].id,
)
assert_nil user.last_trade_activity_at
end
setup do
freeze_time # to compare timestamps accurately
Item.create!(
thumbnail_url: "https://images.neopets.com/foo.png",
zones_restrict: "",
price: 123,
)
end
private
def create_user(**args)
auth_user = AuthUser.create!(
name: 'test', email: 'test@example.com', password: 'test123!'
)
auth_user.user.update!(**args) unless args.empty?
auth_user.user
end
def create_closet_list(**args)
num = ClosetList.count + 1
ClosetList.create!(name: "Test List #{num}", hangers_owned: true, **args)
end
def create_closet_hanger(**args)
ClosetHanger.create!(item: Item.first, quantity: 1, **args)
end
end

Binary file not shown.