diff --git a/app/assets/stylesheets/pages/shop/_admin_shop_orders.scss b/app/assets/stylesheets/pages/shop/_admin_shop_orders.scss index 6cfdee3ee..7cc45856b 100644 --- a/app/assets/stylesheets/pages/shop/_admin_shop_orders.scss +++ b/app/assets/stylesheets/pages/shop/_admin_shop_orders.scss @@ -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; diff --git a/app/controllers/admin/shop/orders_controller.rb b/app/controllers/admin/shop/orders_controller.rb index 8d2076478..a4d377337 100644 --- a/app/controllers/admin/shop/orders_controller.rb +++ b/app/controllers/admin/shop/orders_controller.rb @@ -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] } @@ -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. diff --git a/app/models/concerns/shop/auto_approvable.rb b/app/models/concerns/shop/auto_approvable.rb index 7e1167046..df20215ad 100644 --- a/app/models/concerns/shop/auto_approvable.rb +++ b/app/models/concerns/shop/auto_approvable.rb @@ -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) @@ -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) diff --git a/app/models/shop_order.rb b/app/models/shop_order.rb index 7c4bd78f9..23b3d3db1 100644 --- a/app/models/shop_order.rb +++ b/app/models/shop_order.rb @@ -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, diff --git a/app/views/admin/shop/orders/_stats.html.erb b/app/views/admin/shop/orders/_stats.html.erb index 49ec620bc..f8479a3e6 100644 --- a/app/views/admin/shop/orders/_stats.html.erb +++ b/app/views/admin/shop/orders/_stats.html.erb @@ -1,6 +1,7 @@

Statistics

Click a status to filter orders:

+

Streak stickers excluded. They are still listed below, under their own toggle.

<%= link_to admin_shop_orders_path(preserved_filter_params.merge(status: 'pending')), class: "shop-orders__stat badge badge-pending" do %> diff --git a/test/controllers/admin/shop/orders_controller_test.rb b/test/controllers/admin/shop/orders_controller_test.rb index 515a2d5ca..1153ee17a 100644 --- a/test/controllers/admin/shop/orders_controller_test.rb +++ b/test/controllers/admin/shop/orders_controller_test.rb @@ -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 } diff --git a/test/models/shop_order_auto_approval_test.rb b/test/models/shop_order_auto_approval_test.rb index 0dd03ba49..805269758 100644 --- a/test/models/shop_order_auto_approval_test.rb +++ b/test/models/shop_order_auto_approval_test.rb @@ -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) @@ -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)