Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions app/assets/stylesheets/pages/shop/_admin_shop_orders.scss
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,13 @@
margin: 0;
}

&__stats-note {
color: s.$set-fg-secondary;
font-size: var(--font-size-xs);
font-style: italic;
margin: 0;
}

&__stat-grid {
display: flex;
flex-wrap: wrap;
Expand Down
14 changes: 13 additions & 1 deletion app/controllers/admin/shop/orders_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,23 @@ class Admin::Shop::OrdersController < Admin::ApplicationController
# stays visible — HQ mail and letter mail share a single toggle because
# they're worked as one pile. New LetterMail subclasses join the mail group
# through ShopItem::LetterMail::TYPES, so they fold with the rest of it.
# Sticky Streak stickers get their own toggle rather than folding in with the
# mail: they are sent one at a time from the order, not swept up in a batch,
# so the mail pile's count means nothing with them mixed in.
ITEM_TYPE_TOGGLES = [
{ label: "Free Stickers", hidden_by_default: true, types: %w[ShopItem::FreeStickers] },
{ label: "Warehouse Item", hidden_by_default: true, types: %w[ShopItem::WarehouseItem] },
{ label: "HQ Mail", hidden_by_default: false, types: %w[ShopItem::HQMailItem] + ShopItem::LetterMail::TYPES }
{ label: "HQ Mail", hidden_by_default: false, types: %w[ShopItem::HQMailItem] + ShopItem::LetterMail::BULK_BATCHED_TYPES },
{ label: "Streak Stickers", hidden_by_default: false, types: %w[ShopItem::StickyStreakSticker] }
].freeze

# Sticky Streak stickers clear review on their own and ship outside the batch
# sweep, and one 21-day run adds 21 of them per person, so counting them
# alongside the orders a human actually works buries that work. They stay in
# the queue below and keep their own toggle count; they just leave the
# headline numbers alone.
STATS_EXCLUDED_ITEM_TYPES = %w[ShopItem::StickyStreakSticker].freeze

TOGGLEABLE_ITEM_TYPES = ITEM_TYPE_TOGGLES.flat_map { |toggle| toggle[:types] }.freeze

DEFAULT_HIDDEN_ITEM_TYPES = ITEM_TYPE_TOGGLES.select { |toggle| toggle[:hidden_by_default] }
Expand Down Expand Up @@ -154,6 +165,7 @@ def index
# Apply shared filters to both the orders query and the stats base query
orders = apply_shared_filters(orders)
base = apply_shared_filters(ShopOrder.includes(:shop_item, :user))
.where.not(shop_item_id: ShopItem.where(type: STATS_EXCLUDED_ITEM_TYPES))

# Folded-away item types only come off the list itself — the stats below and
# the counts on the toggles stay whole so nothing vanishes silently.
Expand Down
13 changes: 13 additions & 0 deletions app/models/concerns/shop/auto_approvable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ module AutoApprovable

WHODUNNIT = "Shop::AutoApprovable".freeze

# States a Sticky Streak sticker moves through unattended, which the
# claimer is never notified about. See #silent_status_change?.
SILENT_STREAK_STATES = %w[pending awaiting_periodical_fulfillment].freeze

# Raised when an unattended fulfilment blew up, so the job can back off and
# try again rather than abandoning the order on a transient HCB fault.
FulfilmentFailed = Class.new(StandardError)
Expand Down Expand Up @@ -68,6 +72,15 @@ def sticky_streak_sticker?
shop_item.is_a?(ShopItem::StickyStreakSticker)
end

# A streak sticker is claimed and cleared in one motion, so neither the
# order landing in review nor clearing it is news: the claimer already
# watched the sticker land in the streak calendar, and there is nothing for
# them to do in either state. Everything that does need them (verification,
# fulfilment, rejection) still notifies, as does every other item type.
def silent_status_change?
sticky_streak_sticker? && SILENT_STREAK_STATES.include?(aasm_state)
end

# `ships` lets a caller hand over a buyer's already-loaded ship history;
# left out, the order fetches its own.
def auto_approvable?(ships: nil)
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_order.rb
Original file line number Diff line number Diff line change
Expand Up @@ -414,6 +414,7 @@ def get_agh_contents = shop_item.get_agh_contents(self)
def notify_user_of_status_change
# Don't notify the user when an order is placed on hold — they shouldn't know
return if aasm_state == "on_hold"
return if silent_status_change?

Notifications::ShopOrders::StatusChanged.notify(
recipient: user,
Expand Down
1 change: 1 addition & 0 deletions app/views/admin/shop/orders/_stats.html.erb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<div class="shop-orders__stats">
<h2 class="shop-orders__stats-title">Statistics</h2>
<p class="shop-orders__stats-hint">Click a status to filter orders:</p>
<p class="shop-orders__stats-note">Streak stickers excluded. They are still listed below, under their own toggle.</p>
<div class="shop-orders__stat-grid">
<%= link_to admin_shop_orders_path(preserved_filter_params.merge(status: 'pending')),
class: "shop-orders__stat badge badge-pending" do %>
Expand Down
47 changes: 47 additions & 0 deletions test/controllers/admin/shop/orders_controller_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,55 @@ class Admin::Shop::OrdersControllerTest < ActionDispatch::IntegrationTest
assert_select "h1", text: /Fraud Review/
end

test "streak stickers are left out of the fulfillment stats but stay in the queue" do
packer = buyer("packer", integrity: :auto_passed)
real_order = order_for(packer, at: 2.hours.ago)
real_order.update_columns(aasm_state: "awaiting_periodical_fulfillment")

sticker_order = packer.shop_orders.create!(
shop_item: streak_sticker, quantity: 1, frozen_address: @address
)
sticker_order.update_columns(aasm_state: "awaiting_periodical_fulfillment")

sign_in @admin
get admin_shop_orders_path(view: "fulfillment")

assert_response :success
assert_select ".badge-awaiting_periodical_fulfillment .shop-orders__stat-count",
text: "1", message: "the headline count must ignore the sticker"
assert_includes rendered_order_ids, sticker_order.id,
"the sticker still has to be packed, so it stays in the queue"
assert_select ".shop-orders__stats-note"
end

test "streak stickers get their own toggle instead of inflating the mail pile" do
packer = buyer("mailer", integrity: :auto_passed)
packer.shop_orders.create!(shop_item: streak_sticker, quantity: 1, frozen_address: @address)
.update_columns(aasm_state: "awaiting_periodical_fulfillment")

sign_in @admin
get admin_shop_orders_path(view: "fulfillment", hidden_types: [ "ShopItem::StickyStreakSticker" ])

assert_select ".shop-orders__type-toggle-count", text: "1",
message: "the folded-away stickers report their own count"
assert_not_includes Admin::Shop::OrdersController::ITEM_TYPE_TOGGLES
.find { |toggle| toggle[:label] == "HQ Mail" }[:types],
"ShopItem::StickyStreakSticker"
end

private

def streak_sticker
item = ShopItem.new(
name: "Streak Sticker", description: "Earned by keeping the streak",
ticket_cost: 0, usd_cost: 1, type: "ShopItem::StickyStreakSticker", enabled: true,
mission_prize_only: false
)
item.image.attach(io: StringIO.new(Base64.decode64(PIXEL)), filename: "px.png", content_type: "image/png")
item.save!
item
end

# Ids in the order the table rendered them.
def rendered_order_ids
css_select("tbody tr td:first-child").map { |cell| cell.text[/#(\d+)/, 1].to_i }
Expand Down
28 changes: 28 additions & 0 deletions test/models/shop_order_auto_approval_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,30 @@ class ShopOrderAutoApprovalTest < ActiveSupport::TestCase
assert_equal order.shop_item_id, version.object_changes["shop_item_id"]
end

test "a streak claim tells the claimer nothing on its way to fulfilment" do
order = claim_streak_day

assert_empty status_notifications_for(order),
"neither landing in review nor clearing it is news to the claimer"
end

test "a streak sticker still notifies once it is fulfilled" do
order = claim_streak_day

perform_enqueued_jobs { order.mark_fulfilled! }

assert_equal [ "fulfilled" ], status_notifications_for(order).map { |n| n.params["state"] }
end

test "an ordinary approval still notifies" do
ship_with_integrity(:auto_passed)

order = place_order

assert_equal [ "awaiting_periodical_fulfillment", "pending" ],
status_notifications_for(order).map { |n| n.params["state"] }.sort
end

test "the batch predicate agrees with the per-order one" do
ship_with_integrity(:auto_passed)
clean = @user.shop_orders.create!(shop_item: @item, quantity: 1, frozen_address: @address)
Expand Down Expand Up @@ -279,6 +303,10 @@ def build_streak_sticker
build_item(usd_cost: 1, type: "ShopItem::StickyStreakSticker")
end

def status_notifications_for(order)
Notifications::ShopOrders::StatusChanged.where(record: order)
end

def ship_at(time)
project = Project.create!(title: "Ship #{SecureRandom.hex(4)}")
Project::Membership.create!(project: project, user: @user, role: :owner)
Expand Down
Loading