diff --git a/apps/api/app/admin/stores.rb b/apps/api/app/admin/stores.rb index 19e7b72..a07d2d0 100644 --- a/apps/api/app/admin/stores.rb +++ b/apps/api/app/admin/stores.rb @@ -1,8 +1,14 @@ ActiveAdmin.register Store do - permit_params :name, :catalog_url + permit_params :name, :catalog_url, :active menu priority: 5, label: "🛒 Stores" + # Active first and by default: the registry is what clients actually read, so the + # useful question on opening this page is "what is being offered right now". + scope("Active", default: true) { |scope| scope.active } + scope("Inactive") { |scope| scope.where(active: false) } + scope :all + index do selectable_column id_column @@ -10,12 +16,38 @@ ActiveAdmin.register Store do column :catalog_url do |store| link_to store.catalog_url, store.catalog_url, target: "_blank", rel: "noopener" end + column :active do |store| + status_tag store.active? ? "listed" : "hidden", class: store.active? ? :ok : :warning + end column :updated_at - actions + actions defaults: true do |store| + # One click, because this is the thing an admin comes to this page to do — and + # the alternative is Edit, a checkbox and Save for a single boolean. + link_to store.active? ? "Hide" : "List", + toggle_admin_store_path(store), + method: :put + end end filter :name filter :catalog_url + filter :active + + member_action :toggle, method: :put do + resource.update!(active: !resource.active?) + redirect_back fallback_location: admin_stores_path, + notice: "#{resource.name} is now #{resource.active? ? 'listed' : 'hidden'}" + end + + batch_action :list do |ids| + Store.where(id: ids).update_all(active: true, updated_at: Time.current) + redirect_to admin_stores_path, notice: "#{ids.size} store(s) listed" + end + + batch_action :hide do |ids| + Store.where(id: ids).update_all(active: false, updated_at: Time.current) + redirect_to admin_stores_path, notice: "#{ids.size} store(s) hidden" + end show do attributes_table do @@ -24,6 +56,9 @@ ActiveAdmin.register Store do row :catalog_url do |store| link_to store.catalog_url, store.catalog_url, target: "_blank", rel: "noopener" end + row :active do |store| + status_tag store.active? ? "listed" : "hidden", class: store.active? ? :ok : :warning + end row :created_at row :updated_at end @@ -31,7 +66,8 @@ ActiveAdmin.register Store do "Listed by GET /api/stores, which the graphical client reads on first run. A name " \ "and a catalog are the whole record: the client carries its own store engine " \ "and configures itself from this much, deriving the store's slug from the " \ - "catalog host." + "catalog host. An inactive store is left out of that answer entirely — the " \ + "client has no state for 'there but switched off'." end end @@ -39,6 +75,7 @@ ActiveAdmin.register Store do f.inputs do f.input :name, hint: "What the client shows in its store picker" f.input :catalog_url, hint: "Base URL of the WarpEngine catalog, e.g. https://teletypegames.org" + f.input :active, hint: "Off keeps the record but leaves it out of GET /api/stores" end f.actions end diff --git a/apps/api/app/models/store.rb b/apps/api/app/models/store.rb index ee067e3..6b89cfe 100644 --- a/apps/api/app/models/store.rb +++ b/apps/api/app/models/store.rb @@ -17,9 +17,14 @@ class Store < ApplicationRecord default_scope { where(deleted_at: nil) } + # Two different "not listed". Soft deletion is "this store is gone"; `active` is + # "not right now" — a catalog still being set up, or one pulled from the picker for + # a while. The client cannot tell the difference and should not have to: it lists + # whatever /api/stores hands it. + scope :active, -> { where(active: true) } scope :ordered, -> { order(:name) } def self.ransackable_attributes(auth_object = nil) - %w[id name catalog_url created_at updated_at] + %w[id name catalog_url active created_at updated_at] end end diff --git a/apps/api/app/services/store_service.rb b/apps/api/app/services/store_service.rb index 30a498d..0ab0c7f 100644 --- a/apps/api/app/services/store_service.rb +++ b/apps/api/app/services/store_service.rb @@ -1,5 +1,8 @@ class StoreService + # Only the active ones. An inactive store is simply absent from the registry — the + # client has no state for "there but switched off", and inventing one would mean + # every client release having an opinion about it. def index - StoreSerializer.render_as_hash(Store.ordered) + StoreSerializer.render_as_hash(Store.active.ordered) end end diff --git a/apps/api/db/migrate/20260819121824_add_active_to_stores.rb b/apps/api/db/migrate/20260819121824_add_active_to_stores.rb new file mode 100644 index 0000000..7825dcd --- /dev/null +++ b/apps/api/db/migrate/20260819121824_add_active_to_stores.rb @@ -0,0 +1,17 @@ +class AddActiveToStores < ActiveRecord::Migration[8.1] + # A store that exists but should not be offered yet. + # + # Soft deletion already covered "this store is gone"; what was missing is "not yet" — + # a catalog being set up, or one taken out of the picker for a while without losing + # the row and its history. The client has no notion of either: it lists whatever + # /api/stores hands it, so the filtering has to happen here. + # + # Default true, so every store that exists today keeps being listed. A migration that + # silently emptied the registry would be a client with nothing to install from. + # + # No index: this table holds a handful of rows and is read once per client on first + # run. An index on a two-valued column would be ceremony. + def change + add_column :stores, :active, :boolean, null: false, default: true + end +end diff --git a/apps/api/db/schema.rb b/apps/api/db/schema.rb index 273869f..1325fc5 100644 --- a/apps/api/db/schema.rb +++ b/apps/api/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[8.1].define(version: 2026_08_19_120000) do +ActiveRecord::Schema[8.1].define(version: 2026_08_19_121824) do create_table "admin_users", charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| t.datetime "created_at", null: false t.datetime "deleted_at", precision: 3 @@ -300,6 +300,7 @@ ActiveRecord::Schema[8.1].define(version: 2026_08_19_120000) do end create_table "stores", id: { type: :bigint, unsigned: true }, charset: "utf8mb4", collation: "utf8mb4_0900_ai_ci", force: :cascade do |t| + t.boolean "active", default: true, null: false t.string "catalog_url", null: false t.datetime "created_at", precision: 3, null: false t.datetime "deleted_at", precision: 3 diff --git a/apps/api/spec/controllers/stores_controller_spec.rb b/apps/api/spec/controllers/stores_controller_spec.rb index 8fb0c05..fdbe6eb 100644 --- a/apps/api/spec/controllers/stores_controller_spec.rb +++ b/apps/api/spec/controllers/stores_controller_spec.rb @@ -32,5 +32,25 @@ RSpec.describe Api::StoresController, type: :request do expect(JSON.parse(response.body)).to be_empty end + + # Two different "not listed", and a client can tell neither apart from the store + # never having existed — which is the point. It has no state for "there but + # switched off", so an inactive store is simply absent. + it "leaves out inactive stores" do + create(:store, name: "Listed") + create(:store, name: "Not yet", catalog_url: "https://soon.example", active: false) + + get "/api/stores" + + expect(JSON.parse(response.body).map { |s| s["name"] }).to eq([ "Listed" ]) + end + + it "still says nothing about the flag itself" do + create(:store) + + get "/api/stores" + + expect(JSON.parse(response.body).first.keys).to contain_exactly("name", "catalogUrl") + end end end diff --git a/apps/api/spec/factories/stores.rb b/apps/api/spec/factories/stores.rb index 02f0acb..3c30943 100644 --- a/apps/api/spec/factories/stores.rb +++ b/apps/api/spec/factories/stores.rb @@ -2,5 +2,9 @@ FactoryBot.define do factory :store do name { "Teletype Games" } catalog_url { "https://teletypegames.org" } + + trait :inactive do + active { false } + end end end diff --git a/apps/api/spec/requests/admin_stores_spec.rb b/apps/api/spec/requests/admin_stores_spec.rb new file mode 100644 index 0000000..b0d5b99 --- /dev/null +++ b/apps/api/spec/requests/admin_stores_spec.rb @@ -0,0 +1,91 @@ +require "rails_helper" +require "warden/test/helpers" + +# The admin is where the flag is actually used, and none of it is reachable from a model +# spec: the scopes, the toggle action and the batch actions are ActiveAdmin plumbing that +# only exists once the host's admin is running. The pipelines resource shipped with a +# missing `permit_params` and every edit raised — same layer, same lesson. +RSpec.describe "Admin stores", type: :request do + include Warden::Test::Helpers + + let(:admin) { AdminUser.create!(email: "stores-spec@example.org", password: "password123") } + let!(:listed) { create(:store, name: "Listed", catalog_url: "https://listed.example") } + let!(:hidden) { create(:store, :inactive, name: "Hidden", catalog_url: "https://hidden.example") } + + before do + Warden.test_mode! + login_as(admin, scope: :admin_user) + end + + after { Warden.test_reset! } + + # Same reasoning as the pipelines spec: a request spec has no rendered form to take a + # CSRF token from, and the token is not what is under test. + around do |example| + protection = ActionController::Base.allow_forgery_protection + ActionController::Base.allow_forgery_protection = false + example.run + ActionController::Base.allow_forgery_protection = protection + end + + it "opens with the active ones" do + get "/admin/stores" + + expect(response).to have_http_status(:ok) + expect(response.body).to include("Listed") + expect(response.body).not_to include("Hidden") + end + + it "shows the hidden ones under their own scope" do + get "/admin/stores", params: { scope: "inactive" } + + expect(response.body).to include("Hidden") + end + + it "renders a store's page" do + get "/admin/stores/#{hidden.id}" + + expect(response).to have_http_status(:ok) + end + + # The one this page exists for: the form saves the flag rather than dropping it as an + # unpermitted attribute, which fails silently — the page redirects and nothing changes. + it "saves the flag from the form" do + put "/admin/stores/#{listed.id}", params: { store: { active: "0" } } + + expect(response).to have_http_status(:found) + expect(listed.reload).not_to be_active + end + + it "toggles from the index in one click" do + put "/admin/stores/#{listed.id}/toggle" + + expect(response).to have_http_status(:found) + expect(listed.reload).not_to be_active + + put "/admin/stores/#{listed.id}/toggle" + expect(listed.reload).to be_active + end + + it "lists and hides in bulk" do + post "/admin/stores/batch_action", + params: { batch_action: "list", collection_selection: [ hidden.id ] } + + expect(hidden.reload).to be_active + + post "/admin/stores/batch_action", + params: { batch_action: "hide", collection_selection: [ hidden.id ] } + + expect(hidden.reload).not_to be_active + end + + # The registry is the whole point of the flag, so the two are checked together: a + # change made here has to be what the client sees. + it "is what /api/stores answers with" do + put "/admin/stores/#{listed.id}/toggle" + + get "/api/stores" + + expect(JSON.parse(response.body)).to be_empty + end +end