Update pet state bulk-labeling to skip to next unlabeled if desired

Previously, "Then: Go to unlabeled appearance" would always take you to
the *first* unlabeled appearance in our database.

Now, we go to the *next* unlabeled appearance in the list, relative to
this one.
This commit is contained in:
Emi Matchu 2024-12-08 10:08:39 -08:00
parent 30d42d29c1
commit 7eb209e206
6 changed files with 154 additions and 8 deletions

View file

@ -40,7 +40,8 @@ class PetStatesController < ApplicationController
end end
def next_unlabeled_appearance_path def next_unlabeled_appearance_path
unlabeled_appearance = PetState.next_unlabeled_appearance unlabeled_appearance =
PetState.next_unlabeled_appearance(after_id: params[:after])
if unlabeled_appearance if unlabeled_appearance
edit_pet_type_pet_state_path( edit_pet_type_pet_state_path(

View file

@ -1,7 +1,8 @@
module SupportFormHelper module SupportFormHelper
class SupportFormBuilder < ActionView::Helpers::FormBuilder class SupportFormBuilder < ActionView::Helpers::FormBuilder
attr_reader :template attr_reader :template
delegate :capture, :check_box_tag, :content_tag, :params, :render, delegate :capture, :check_box_tag, :concat, :content_tag,
:hidden_field_tag, :params, :render,
to: :template, private: true to: :template, private: true
def errors def errors
@ -40,8 +41,11 @@ module SupportFormHelper
content_tag(:section, class: "actions", &block) content_tag(:section, class: "actions", &block)
end end
def go_to_next_field(**options, &block) def go_to_next_field(after: nil, **options, &block)
content_tag(:label, class: "go-to-next", **options, &block) content_tag(:label, class: "go-to-next", **options) do
concat hidden_field_tag(:after, after) if after
yield
end
end end
def go_to_next_check_box(value) def go_to_next_check_box(value)

View file

@ -24,6 +24,7 @@ class PetState < ApplicationRecord
scope :newest, -> { order(created_at: :desc) } scope :newest, -> { order(created_at: :desc) }
scope :newest_pet_type, -> { joins(:pet_type).merge(PetType.newest) } scope :newest_pet_type, -> { joins(:pet_type).merge(PetType.newest) }
scope :created_before, ->(time) { where(arel_table[:created_at].lt(time)) }
# A simple ordering that tries to bring reliable pet states to the front. # A simple ordering that tries to bring reliable pet states to the front.
scope :emotion_order, -> { scope :emotion_order, -> {
@ -142,11 +143,39 @@ class PetState < ApplicationRecord
end end
end end
def self.next_unlabeled_appearance def self.next_unlabeled_appearance(after_id: nil)
# Rather than just getting the newest unlabeled pet state, prioritize the # Rather than just getting the newest unlabeled pet state, prioritize the
# newest *pet type*. This better matches the user's perception of what the # newest *pet type*. This better matches the user's perception of what the
# newest state is, because the Rainbow Pool UI is grouped by pet type! # newest state is, because the Rainbow Pool UI is grouped by pet type!
needs_labeling.newest_pet_type.newest.first pet_states = needs_labeling.newest_pet_type.newest
# If `after_id` is given, convert it from a PetState ID to creation
# timestamps, and find the next record prior to those timestamps. This
# enables skipping past records the user doesn't want to label.
if after_id
begin
after_pet_state = PetState.find(after_id)
before_pt_created_at = after_pet_state.pet_type.created_at
before_ps_created_at = after_pet_state.created_at
rescue ActiveRecord::RecordNotFound
Rails.logger.warn "PetState.next_unlabeled_appearance: Could not " +
"find pet state ##{after_id}"
return nil
end
# Because we sort by `newest_pet_type` first, then breaks ties by
# `newest`, our filter needs to operate the same way. Kudos to:
# https://brunoscheufler.com/blog/2022-01-01-paginating-large-ordered-datasets-with-cursor-based-pagination
pet_states.merge!(
PetType.created_before(before_pt_created_at).or(
PetType.created_at(before_pt_created_at).and(
PetState.created_before(before_ps_created_at)
)
)
)
end
pet_states.first
end end
end end

View file

@ -31,6 +31,12 @@ class PetType < ApplicationRecord
# We use DTI's creation timestamp as an estimate of when it was released. # We use DTI's creation timestamp as an estimate of when it was released.
where('created_at <= ?', time) where('created_at <= ?', time)
} }
scope :created_before, ->(time) {
where(arel_table[:created_at].lt(time))
}
scope :created_at, ->(time) {
where(arel_table[:created_at].eq(time))
}
def self.random_basic_per_species(species_ids) def self.random_basic_per_species(species_ids)
random_pet_types = [] random_pet_types = []

View file

@ -39,9 +39,10 @@
= f.actions do = f.actions do
= f.submit "Save changes" = f.submit "Save changes"
= f.go_to_next_field title: "If checked, takes you to the first unlabeled appearance in the database, if any. Useful for labeling in bulk!" do = f.go_to_next_field after: @pet_state.id,
title: "If checked, takes you to the first unlabeled appearance in the database, if any. Useful for labeling in bulk!" do
= f.go_to_next_check_box "unlabeled-appearance" = f.go_to_next_check_box "unlabeled-appearance"
Then: Go to unlabeled appearance Then: Go to next unlabeled appearance
- content_for :stylesheets do - content_for :stylesheets do
= stylesheet_link_tag "application/breadcrumbs" = stylesheet_link_tag "application/breadcrumbs"

View file

@ -0,0 +1,105 @@
require_relative '../rails_helper'
RSpec.describe PetState do
fixtures :colors, :species, :zones
let(:blue) { colors(:blue) }
let(:green) { colors(:green) }
let(:red) { colors(:red) }
let(:acara) { species(:acara) }
describe ".next_unlabeled_appearance" do
before { PetType.destroy_all }
def create_sa
swf_asset = SwfAsset.create!(
type: "biology", remote_id: (SwfAsset.maximum(:remote_id) || 0) + 1,
url: "https://images.neopets.example/hello.swf",
zone: zones(:body), zones_restrict: [], body_id: 0)
end
def create_pt(color, species, created_at = nil)
PetType.create! color:, species:, created_at:,
body_id: (PetType.maximum(:body_id) || 0) + 1
end
def create_ps(pet_type, pose, created_at = nil, **options)
# HACK: PetStates without any assets don't save correctly.
# https://github.com/rails/rails/issues/52340
swf_assets = [create_sa]
PetState.create! pet_type:, pose:, created_at:, swf_assets:,
swf_asset_ids: swf_assets.map(&:id), **options
end
it "returns nil where there are no pet states" do
expect(PetState.next_unlabeled_appearance).to be_nil
end
it "returns nil where there are only labeled pet states" do
pt = PetType.create! color: blue, species: acara, body_id: 1
ps = create_ps(pt, "HAPPY_MASC").tap(&:save!)
expect(PetState.next_unlabeled_appearance).to be_nil
end
it "returns the only pet state when it is unlabeled" do
pt = PetType.create! color: blue, species: acara, body_id: 1
ps = create_ps(pt, "UNKNOWN").tap(&:save!)
expect(PetState.next_unlabeled_appearance).to eq ps
end
describe "with multiple unlabeled pet states" do
before do
# Create three pet types, with ascending order of creation date.
@pt1 = create_pt blue, acara, Date.new(2000)
@pt2 = create_pt green, acara, Date.new(2005)
@pt3 = create_pt red, acara, Date.new(2010)
# Give each a pet state, but created in a different order.
@ps1 = create_ps @pt1, "UNKNOWN", Date.new(2020)
@ps2 = create_ps @pt2, "UNKNOWN", Date.new(2025)
@ps3 = create_ps @pt3, "UNKNOWN", Date.new(2015)
end
it "returns the latest pet type's pet state" do
expect(PetState.next_unlabeled_appearance).to eq @ps3
end
it "excludes fully-labeled pet types" do
# Label the latest pet state, then see it move to the next.
@ps3.update!(pose: "HAPPY_FEM")
expect(PetState.next_unlabeled_appearance).to eq @ps2
end
it "excludes labeled pet states" do
# Create an older pet state on the latest pet type, than label the
# latest pet state, and see it move back to the older one.
ps3_a = create_ps @pt3, "UNKNOWN", Date.new(2014)
@ps3.update!(pose: "HAPPY_FEM")
expect(PetState.next_unlabeled_appearance).to eq ps3_a
end
it "sorts pet states within the latest pet type by newest" do
# Create a few pet types on the latest pet type, and see that we get
# the latest back.
ps3_a = create_ps @pt3, "UNKNOWN", Date.new(2016)
ps3_b = create_ps @pt3, "UNKNOWN", Date.new(2017)
ps3_c = create_ps @pt3, "UNKNOWN", Date.new(2018)
ps3_d = create_ps @pt3, "UNKNOWN", Date.new(2019)
expect(PetState.next_unlabeled_appearance).to eq ps3_d
end
it "can find the next after the latest pet state" do
expect(PetState.next_unlabeled_appearance(after_id: @ps3.id)).to eq @ps2
end
it "can find the next after any given pet state" do
expect(PetState.next_unlabeled_appearance(after_id: @ps2.id)).to eq @ps1
end
it "can find the next after the latest pet state, even within the same pet type" do
ps3_a = create_ps @pt3, "UNKNOWN", Date.new(2014)
expect(PetState.next_unlabeled_appearance(after_id: @ps3.id)).to eq ps3_a
end
end
end
end