Broaden ShopPolicy to admin || member per collaborative model

The substrate v2 design (overview §2.1 table) grants create_shops,
update_shops, and delete_shops to both admin and member tiers.
Pre-refactor policy was stricter than the design called for:
- create? required owner? (so even non-owner admins couldn't create)
- update? required admin? (so members couldn't update)
- destroy? delegated to create? (so only owner could destroy)

Aligned with the design and with ItemTagPolicy's pattern. Tests
rewritten — two old assertions inverted (non-owner admin can now
create; member can now update), and two read-only true-for-all tests
specialised into admin and member variants for symmetry.

Author: dadachi

app/policies/api/shopkeeper/shop_policy.rb

@@ -2,22 +2,22 @@ class Api::Shopkeeper::ShopPolicy < Api::Shopkeeper::BasePolicy
   include Api::Shopkeeper::Concerns::Authorization
 
   def index?
-    true
+    admin? || member?
   end
 
-  def create?
-    owner?
+  def show?
+    admin? || member?
   end
 
-  def show?
-    true
+  def create?
+    admin? || member?
   end
 
   def update?
-    admin?
+    admin? || member?
   end
 
   def destroy?
-    create?
+    admin? || member?
   end
 end

test/policies/api/shopkeeper/shop_policy_test.rb

@@ -8,62 +8,66 @@ def setup
     @shop = @account.shops.first
   end
 
-  test "index? returns true for all users" do
-    accounts_shopkeeper = @account.accounts_shopkeepers.first
-    policy = Api::Shopkeeper::ShopPolicy.new(accounts_shopkeeper, @shop)
+  def admin_accounts_shopkeeper
+    @account.accounts_shopkeepers.first.tap { |as| as.update!(admin: true) }
+  end
+
+  def member_accounts_shopkeeper
+    other_shopkeeper = shopkeepers(:two)
+    AccountsShopkeeper.create!(
+      account: @account,
+      shopkeeper: other_shopkeeper,
+      member: true
+    )
+  end
+
+  test "index? returns true for admin" do
+    policy = Api::Shopkeeper::ShopPolicy.new(admin_accounts_shopkeeper, @shop)
     assert policy.index?
   end
 
-  test "show? returns true for all users" do
-    accounts_shopkeeper = @account.accounts_shopkeepers.first
-    policy = Api::Shopkeeper::ShopPolicy.new(accounts_shopkeeper, @shop)
+  test "index? returns true for member" do
+    policy = Api::Shopkeeper::ShopPolicy.new(member_accounts_shopkeeper, @shop)
+    assert policy.index?
+  end
+
+  test "show? returns true for admin" do
+    policy = Api::Shopkeeper::ShopPolicy.new(admin_accounts_shopkeeper, @shop)
     assert policy.show?
   end
 
-  test "create? returns true for account owner" do
-    accounts_shopkeeper = @account.accounts_shopkeepers.first
-    assert accounts_shopkeeper.account_owner?
+  test "show? returns true for member" do
+    policy = Api::Shopkeeper::ShopPolicy.new(member_accounts_shopkeeper, @shop)
+    assert policy.show?
+  end
 
-    policy = Api::Shopkeeper::ShopPolicy.new(accounts_shopkeeper, @shop)
+  test "create? returns true for admin" do
+    policy = Api::Shopkeeper::ShopPolicy.new(admin_accounts_shopkeeper, @shop)
     assert policy.create?
   end
 
-  test "create? returns false for non-owner" do
-    other_shopkeeper = shopkeepers(:two)
-    accounts_shopkeeper = AccountsShopkeeper.create!(
-      account: @account,
-      shopkeeper: other_shopkeeper,
-      admin: true
-    )
-    assert_not accounts_shopkeeper.account_owner?
-
-    policy = Api::Shopkeeper::ShopPolicy.new(accounts_shopkeeper, @shop)
-    assert_not policy.create?
+  test "create? returns true for member" do
+    policy = Api::Shopkeeper::ShopPolicy.new(member_accounts_shopkeeper, @shop)
+    assert policy.create?
   end
 
   test "update? returns true for admin" do
-    accounts_shopkeeper = @account.accounts_shopkeepers.first
-    accounts_shopkeeper.update!(admin: true)
-
-    policy = Api::Shopkeeper::ShopPolicy.new(accounts_shopkeeper, @shop)
+    policy = Api::Shopkeeper::ShopPolicy.new(admin_accounts_shopkeeper, @shop)
     assert policy.update?
   end
 
-  test "update? returns false for non-admin" do
-    other_shopkeeper = shopkeepers(:two)
-    accounts_shopkeeper = AccountsShopkeeper.create!(
-      account: @account,
-      shopkeeper: other_shopkeeper,
-      member: true
-    )
+  test "update? returns true for member" do
+    policy = Api::Shopkeeper::ShopPolicy.new(member_accounts_shopkeeper, @shop)
+    assert policy.update?
+  end
 
-    policy = Api::Shopkeeper::ShopPolicy.new(accounts_shopkeeper, @shop)
-    assert_not policy.update?
+  test "destroy? returns true for admin" do
+    policy = Api::Shopkeeper::ShopPolicy.new(admin_accounts_shopkeeper, @shop)
+    assert policy.destroy?
   end
 
-  test "destroy? delegates to create?" do
-    accounts_shopkeeper = @account.accounts_shopkeepers.first
-    policy = Api::Shopkeeper::ShopPolicy.new(accounts_shopkeeper, @shop)
-    assert_equal policy.create?, policy.destroy?
+  test "destroy? returns true for member" do
+    policy = Api::Shopkeeper::ShopPolicy.new(member_accounts_shopkeeper, @shop)
+    assert policy.destroy?
   end
 end