Skip to content

Commit 2e93213

Browse files
dadachiclaude
andcommitted
Fix two latent bugs in permissions and sign-in flows
PermissionsController#index compared `confirmed_*_version < current_*_version`, where `current_version` returns nil if no row is flagged current. `string < nil` raises ArgumentError → 500. Add a `version_outdated?` helper that treats a missing current version as "nothing to update". ShopkeeperAuth::SessionsController#create unconditionally assigned `request.headers["source"]` to current_platform and called `save!(validate: false)`. Sign-ins without the source header overwrote the user's stored platform with nil, bypassing the presence/inclusion validation. Now skip the assignment when the header is blank. Adds regression tests for both paths. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 08c840f commit 2e93213

4 files changed

Lines changed: 51 additions & 3 deletions

File tree

app/controllers/api/v1/shopkeeper/permissions_controller.rb

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,8 @@ def index
88
current_privacy_version = PrivacyVersion.current_version
99
current_terms_version = TermsVersion.current_version
1010

11-
should_update_privacy = current_shopkeeper.confirmed_privacy_version < current_privacy_version
12-
should_update_terms = current_shopkeeper.confirmed_terms_version < current_terms_version
11+
should_update_privacy = version_outdated?(current_shopkeeper.confirmed_privacy_version, current_privacy_version)
12+
should_update_terms = version_outdated?(current_shopkeeper.confirmed_terms_version, current_terms_version)
1313

1414
options = {}
1515
options[:meta] = {
@@ -25,4 +25,13 @@ def index
2525
permissions = current_accounts_shopkeeper.permissions
2626
render json: PermissionSerializer.new(permissions, options).serializable_hash
2727
end
28+
29+
private
30+
31+
# nil current → nothing published yet, nothing to update.
32+
def version_outdated?(confirmed, current)
33+
return false if current.nil?
34+
35+
confirmed < current
36+
end
2837
end

app/controllers/shopkeeper_auth/sessions_controller.rb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,10 @@ def create
33
super
44
return if @resource.blank?
55

6-
@resource.current_platform = request.headers["source"]
6+
source = request.headers["source"]
7+
return if source.blank?
8+
9+
@resource.current_platform = source
710
@resource.save!(validate: false)
811
end
912

test/controllers/api/v1/shopkeeper/permissions_controller_test.rb

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,4 +64,15 @@ class Api::V1::Shopkeeper::PermissionsControllerTest < ActionDispatch::Integrati
6464

6565
assert_response :unauthorized
6666
end
67+
68+
test "index does not crash when there is no current published privacy or terms version" do
69+
PrivacyVersion.update_all(current_type: PrivacyVersion.current_types[:uncurrent])
70+
TermsVersion.update_all(current_type: TermsVersion.current_types[:uncurrent])
71+
72+
get api_v1_shopkeeper_permissions_url, headers: @shopkeeper.create_new_auth_token
73+
74+
assert_response :success
75+
assert_equal false, response.parsed_body["meta"]["should_update_privacy"]
76+
assert_equal false, response.parsed_body["meta"]["should_update_terms"]
77+
end
6778
end

test/controllers/shopkeeper_auth/sessions_controller_test.rb

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,4 +28,29 @@ class ShopkeeperAuth::SessionsControllerTest < ActionDispatch::IntegrationTest
2828
delete destroy_shopkeeper_session_url, headers: shopkeeper.create_new_auth_token
2929
assert_response :success
3030
end
31+
32+
test "successful sign-in updates current_platform to the source header value" do
33+
shopkeeper = shopkeepers(:one)
34+
shopkeeper.create_default_account
35+
shopkeeper.update_column(:current_platform, "ios")
36+
37+
post shopkeeper_session_url,
38+
params: {email: shopkeeper.email, password: "password"},
39+
headers: {source: "android"}
40+
41+
assert_response :success
42+
assert_equal "android", shopkeeper.reload.current_platform
43+
end
44+
45+
test "successful sign-in without source header preserves the existing current_platform" do
46+
shopkeeper = shopkeepers(:one)
47+
shopkeeper.create_default_account
48+
shopkeeper.update_column(:current_platform, "ios")
49+
50+
post shopkeeper_session_url,
51+
params: {email: shopkeeper.email, password: "password"}
52+
53+
assert_response :success
54+
assert_equal "ios", shopkeeper.reload.current_platform
55+
end
3156
end

0 commit comments

Comments
 (0)