Skip to content
Open
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions app/controllers/admin/shop_items_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ def available_shop_item_types
"ShopItem::HCBGrant",
"ShopItem::HCBPreauthGrant",
"ShopItem::HQMailItem",
"ShopItem::InkthreadableItem",
"ShopItem::LetterMail",
"ShopItem::ThirdPartyPhysical",
"ShopItem::ThirdPartyDigital",
Expand Down
77 changes: 77 additions & 0 deletions app/jobs/shop/send_inkthreadable_order_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
# frozen_string_literal: true

class Shop::SendInkthreadableOrderJob < ApplicationJob
queue_as :default

def perform(order_id)
order = ShopOrder.find(order_id)
shop_item = order.shop_item

unless shop_item.is_a?(ShopItem::InkthreadableItem)
Rails.logger.warn "[Inkthreadable] Order #{order_id} is not an Inkthreadable item, skipping"
return
end

address = order.frozen_address
if address.blank?
Rails.logger.error "[Inkthreadable] Order #{order_id} missing address"
return
end

payload = build_order_payload(order, shop_item, address)

response = InkthreadableService.create_order(payload)

inkthreadable_order_id = response.dig("order", "id")
if inkthreadable_order_id.present?
order.update!(external_ref: "INK-#{inkthreadable_order_id}")
Rails.logger.info "[Inkthreadable] Order #{order_id} submitted successfully as #{inkthreadable_order_id}"
else
Rails.logger.error "[Inkthreadable] Order #{order_id} response missing order.id: #{response.inspect}"
raise "Inkthreadable response missing order.id"
end
rescue Faraday::Error => e
Rails.logger.error "[Inkthreadable] Failed to send order #{order_id}: #{e.message}"
Rails.logger.error e.response&.dig(:body) if e.respond_to?(:response)
raise
end

private

def build_order_payload(order, shop_item, address)
payload = {
external_id: "FT-#{order.id}",
shipping_address: {
firstName: address["first_name"] || address["firstName"] || order.user.display_name.split.first,
lastName: address["last_name"] || address["lastName"] || order.user.display_name.split.last,
Comment on lines +42 to +46
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lastName fallback uses display_name.split.last which will return the entire display_name if there are no spaces. For single-word display names, this could result in the same value being used for both firstName and lastName. Consider handling this edge case more gracefully.

Suggested change
payload = {
external_id: "FT-#{order.id}",
shipping_address: {
firstName: address["first_name"] || address["firstName"] || order.user.display_name.split.first,
lastName: address["last_name"] || address["lastName"] || order.user.display_name.split.last,
user_display_name = order.user&.display_name.to_s
name_parts = user_display_name.strip.split(/\s+/)
fallback_first_name = name_parts.first
fallback_last_name = name_parts.length > 1 ? name_parts.last : nil
payload = {
external_id: "FT-#{order.id}",
shipping_address: {
firstName: address["first_name"] || address["firstName"] || fallback_first_name,
lastName: address["last_name"] || address["lastName"] || fallback_last_name,

Copilot uses AI. Check for mistakes.
company: address["company"],
address1: address["address1"] || address["line1"],
address2: address["address2"] || address["line2"],
city: address["city"],
county: address["state"] || address["county"],
postcode: address["postcode"] || address["zip"] || address["postal_code"],
country: address["country"],
phone1: address["phone"] || address["phone1"]
}.compact,
Comment on lines +44 to +55
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The address field handling logic tries multiple field name variations (e.g., "first_name" vs "firstName", "postcode" vs "zip" vs "postal_code"). However, there's no validation to ensure that required fields are actually present after this normalization. If critical fields like address1, city, or country are missing after checking all variations, the API call will likely fail. Consider adding validation to check that all required fields are present before making the API call.

Copilot uses AI. Check for mistakes.
shipping: {
shippingMethod: shop_item.shipping_method
},
items: build_order_items(order, shop_item)
}

payload[:brandName] = shop_item.brand_name if shop_item.brand_name.present?
payload[:comment] = "Flavortown order ##{order.id}" if Rails.env.production?
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment is only added in production environment, which may make testing and debugging more difficult in non-production environments. Consider including the comment in all environments or document why it should only be added in production.

Suggested change
payload[:comment] = "Flavortown order ##{order.id}" if Rails.env.production?
payload[:comment] = "Flavortown order ##{order.id}"

Copilot uses AI. Check for mistakes.

payload
end

def build_order_items(order, shop_item)
[
{
pn: shop_item.product_number,
quantity: order.quantity,
designs: shop_item.design_urls
}.compact
]
end
end
Comment on lines +1 to +77
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Shop::SendInkthreadableOrderJob lacks test coverage. Consider adding tests to verify payload construction with various address formats, error handling when addresses are missing required fields, and proper handling of Faraday errors.

Copilot uses AI. Check for mistakes.
76 changes: 76 additions & 0 deletions app/jobs/shop/sync_inkthreadable_status_job.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
class Shop::SyncInkthreadableStatusJob < ApplicationJob
queue_as :default

SHIPPED_STATUSES = [ "quality control" ].freeze
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The constant SHIPPED_STATUSES is defined but never used in the code. This appears to be dead code that should either be removed or the logic should be updated to use it.

Copilot uses AI. Check for mistakes.
ALERT_SLACK_ID = "U08FDLWUZM4" # @transcental

def perform
pending_inkthreadable_orders.find_each do |order|
sync_order_status(order)
rescue => e
Rails.logger.error "[InkthreadableSync] Failed to sync order #{order.id}: #{e.message}"
end
end

private

def pending_inkthreadable_orders
ShopOrder
.joins(:shop_item)
.where(shop_items: { type: "ShopItem::InkthreadableItem" })
.where.not(aasm_state: %w[fulfilled rejected refunded])
.where("external_ref LIKE 'INK-%'")
end

def sync_order_status(order)
inkthreadable_id = order.external_ref.delete_prefix("INK-")
response = InkthreadableService.get_order(inkthreadable_id)

ink_order = response["order"]
return unless ink_order

status = ink_order["status"]&.downcase
shipping = ink_order["shipping"] || {}
tracking_number = shipping["trackingNumber"]
shipped_at = shipping["shiped_at"]

if shipped_at.present? || tracking_number.present?
mark_as_fulfilled(order, tracking_number)
Comment on lines +38 to +39
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic checks for shipped_at OR tracking_number to mark an order as fulfilled, but then only the tracking_number is used to update the external_ref. If shipped_at is present but tracking_number is null, the order will be marked as fulfilled but the external_ref will remain unchanged, potentially maintaining an INK-[id] format rather than INK-[tracking_number]. Consider whether this is the intended behavior or if there should be separate handling for cases where only shipped_at is available.

Copilot uses AI. Check for mistakes.
elsif status == "refunded" || ink_order["deleted"] == "true"
handle_cancelled(order)
else
handle_unexpected_status(order, status)
end
end

def handle_unexpected_status(order, status)
message = "[InkthreadableSync] Order #{order.id} has unexpected status: #{status}"
Rails.logger.warn message

Sentry.capture_message(message, level: :warning, extra: { order_id: order.id, status: status })

send_slack_alert(order, status)
end

Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All pending Inkthreadable orders with unexpected statuses will trigger a Slack alert every hour when this job runs. This could lead to alert fatigue if an order stays in an unexpected status for an extended period. Consider adding logic to track which orders have already been alerted about or implementing exponential backoff to avoid repeatedly alerting for the same issue.

Suggested change
send_slack_alert(order, status)
end
send_slack_alert(order, status) if alert_allowed?(order, status)
end
def alert_allowed?(order, status)
cache_key = "inkthreadable_unexpected_status_alert:#{order.id}:#{status}"
data = Rails.cache.read(cache_key) || {}
last_alert_at = data["last_alert_at"] && Time.zone.parse(data["last_alert_at"].to_s)
count = (data["count"] || 0).to_i
# Exponential backoff: 1h, 2h, 4h, 8h, ... capped at 24h
base_interval = 1.hour
interval = [base_interval * (2**count), 24.hours].min
now = Time.current
if last_alert_at.nil? || now - last_alert_at >= interval
Rails.cache.write(
cache_key,
{
"last_alert_at" => now.iso8601,
"count" => count + 1
},
expires_in: 7.days
)
true
else
false
end
end

Copilot uses AI. Check for mistakes.
def send_slack_alert(order, status)
slack_id = order.assigned_to_user&.slack_id || ALERT_SLACK_ID
client = Slack::Web::Client.new(token: Rails.application.credentials.dig(:slack, :bot_token))

client.chat_postMessage(
channel: slack_id,
text: "⚠️ Inkthreadable order needs attention!\n\nOrder ##{order.id} has unexpected status: *#{status}*\n\nPlease review: #{Rails.application.routes.url_helpers.admin_shop_order_url(order, host: "flavortown.hackclub.com")}"
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The URL uses a hardcoded hostname "flavortown.hackclub.com" which may not be correct for non-production environments. Consider using a configurable hostname or Rails.application.config.action_mailer.default_url_options[:host] to ensure the correct domain is used in all environments.

Suggested change
client.chat_postMessage(
channel: slack_id,
text: "⚠️ Inkthreadable order needs attention!\n\nOrder ##{order.id} has unexpected status: *#{status}*\n\nPlease review: #{Rails.application.routes.url_helpers.admin_shop_order_url(order, host: "flavortown.hackclub.com")}"
host = Rails.application.config.action_mailer.default_url_options[:host]
client.chat_postMessage(
channel: slack_id,
text: "⚠️ Inkthreadable order needs attention!\n\nOrder ##{order.id} has unexpected status: *#{status}*\n\nPlease review: #{Rails.application.routes.url_helpers.admin_shop_order_url(order, host: host)}"

Copilot uses AI. Check for mistakes.
)
rescue Slack::Web::Api::Errors::SlackError => e
Rails.logger.error "[InkthreadableSync] Failed to send Slack alert: #{e.message}"
end

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's move this to the shop_order.rb model and use it whenever issues come up! Also going to make it a channel instead of DMing me - invited you on Slack

def mark_as_fulfilled(order, tracking_number)
external_ref = tracking_number.present? ? "INK-#{tracking_number}" : order.external_ref
order.mark_fulfilled!(external_ref)
Rails.logger.info "[InkthreadableSync] Order #{order.id} marked as fulfilled with tracking: #{tracking_number}"
end

def handle_cancelled(order)
Rails.logger.warn "[InkthreadableSync] Order #{order.id} was refunded/cancelled in Inkthreadable"
end
Comment on lines 62 to 65
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The handle_cancelled method logs a warning but takes no action on the order. If an order is refunded or deleted in Inkthreadable, the order should likely be refunded or rejected in the local system as well. Consider implementing proper handling such as calling order.refund! or order.mark_rejected! with an appropriate reason.

Copilot uses AI. Check for mistakes.
end
Comment on lines 1 to 66
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Shop::SyncInkthreadableStatusJob lacks test coverage. Consider adding tests for various order states (shipped, refunded, unexpected status), Slack alert behavior, and the query filtering logic for pending orders.

Copilot uses AI. Check for mistakes.
1 change: 1 addition & 0 deletions app/models/shop_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/accessory.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/free_stickers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/h_c_b_grant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/h_c_b_preauth_grant.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/h_q_mail_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/hack_clubber_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
100 changes: 100 additions & 0 deletions app/models/shop_item/inkthreadable_item.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,100 @@
# == Schema Information
#
# Table name: shop_items
#
# id :bigint not null, primary key
# accessory_tag :string
# agh_contents :jsonb
# attached_shop_item_ids :bigint default([]), is an Array
# buyable_by_self :boolean default(TRUE)
# default_assigned_user_id_au :bigint
# default_assigned_user_id_ca :bigint
# default_assigned_user_id_eu :bigint
# default_assigned_user_id_in :bigint
# default_assigned_user_id_uk :bigint
# default_assigned_user_id_us :bigint
# default_assigned_user_id_xx :bigint
# description :string
# enabled :boolean
# enabled_au :boolean
# enabled_ca :boolean
# enabled_eu :boolean
# enabled_in :boolean
# enabled_uk :boolean
# enabled_us :boolean
# enabled_xx :boolean
# hacker_score :integer
# hcb_category_lock :string
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
# max_qty :integer
# name :string
# old_prices :integer default([]), is an Array
# one_per_person_ever :boolean
# payout_percentage :integer default(0)
# price_offset_au :decimal(, )
# price_offset_ca :decimal(, )
# price_offset_eu :decimal(, )
# price_offset_in :decimal(, )
# price_offset_uk :decimal(10, 2)
# price_offset_us :decimal(, )
# price_offset_xx :decimal(, )
# required_ships_count :integer default(1)
# required_ships_end_date :date
# required_ships_start_date :date
# requires_ship :boolean default(FALSE)
# sale_percentage :integer
# show_in_carousel :boolean
# site_action :integer
# special :boolean
# stock :integer
# ticket_cost :decimal(, )
# type :string
# unlock_on :date
# usd_cost :decimal(, )
# created_at :datetime not null
# updated_at :datetime not null
# default_assigned_user_id :bigint
# user_id :bigint
#
# Indexes
#
# index_shop_items_on_default_assigned_user_id (default_assigned_user_id)
# index_shop_items_on_user_id (user_id)
#
# Foreign Keys
#
# fk_rails_... (default_assigned_user_id => users.id) ON DELETE => nullify
# fk_rails_... (user_id => users.id)
#
class ShopItem::InkthreadableItem < ShopItem
def fulfill!(order)
Shop::SendInkthreadableOrderJob.perform_later(order.id)
order.queue_for_fulfillment!
end

def inkthreadable_config
super || {}
end

def product_number
inkthreadable_config["pn"]
end

def design_urls
inkthreadable_config["designs"] || {}
end

def shipping_method
inkthreadable_config["shipping_method"] || "regular"
end

def brand_name
inkthreadable_config["brand_name"]
end
end
Comment on lines 75 to 100
Copy link

Copilot AI Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new InkthreadableItem model lacks test coverage. Consider adding tests to verify the fulfill! method enqueues the correct job, and to test the configuration accessor methods (product_number, design_urls, shipping_method, brand_name) with various configurations including edge cases like missing or empty inkthreadable_config.

Copilot uses AI. Check for mistakes.
1 change: 1 addition & 0 deletions app/models/shop_item/letter_mail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/pile_of_stickers_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/special_fulfillment_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/third_party_digital.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/third_party_physical.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
1 change: 1 addition & 0 deletions app/models/shop_item/warehouse_item.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
# hcb_keyword_lock :string
# hcb_merchant_lock :string
# hcb_preauthorization_instructions :text
# inkthreadable_config :jsonb
# internal_description :string
# limited :boolean
# long_description :text
Expand Down
Loading
Loading