From f6dfbbcdb6868a8b9df59ec6f3e48aea506059a7 Mon Sep 17 00:00:00 2001
From: dadachi <maurois@mac.com>
Date: Mon, 16 Mar 2026 17:08:44 +0900
Subject: [PATCH] Fix nil pointer bugs in version lookups and permissions

- AppVersion/PrivacyVersion/TermsVersion: .first.version crashes with
  NoMethodError when no matching record exists. Use safe navigation
  operator (&.) to return nil instead.
- AppVersion: use `current` instead of `AppVersion.current` so chained
  scopes (e.g. forced_update.current_version) are preserved.
- AccountsShopkeeper#permissions: simplify role-to-permissions lookup
  using active_roles.first with find_by! for clearer error on missing
  role data.
- Add tests for nil-safety edge cases.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
---
 app/models/accounts_shopkeeper.rb   | 16 ++--------------
 app/models/app_version.rb           |  5 ++---
 app/models/privacy_version.rb       |  5 ++---
 app/models/terms_version.rb         |  5 ++---
 test/models/app_version_test.rb     | 14 ++++++++++++++
 test/models/privacy_version_test.rb |  5 +++++
 test/models/terms_version_test.rb   |  5 +++++
 7 files changed, 32 insertions(+), 23 deletions(-)

diff --git a/app/models/accounts_shopkeeper.rb b/app/models/accounts_shopkeeper.rb
index 6d7ae3b..06fff89 100644
--- a/app/models/accounts_shopkeeper.rb
+++ b/app/models/accounts_shopkeeper.rb
@@ -17,20 +17,8 @@ def account_owner?
   end
 
   def permissions
-    role = if admin?
-      Role.find_by(tag: "admin")
-    elsif senior_manager?
-      Role.find_by(tag: "senior_manager")
-    elsif junior_manager?
-      Role.find_by(tag: "junior_manager")
-    elsif senior_member?
-      Role.find_by(tag: "senior_member")
-    elsif junior_member?
-      Role.find_by(tag: "junior_member")
-    elsif guest?
-      Role.find_by(tag: "guest")
-    end
-
+    role_tag = active_roles.first&.to_s
+    role = Role.find_by!(tag: role_tag)
     role.permissions
   end
 
diff --git a/app/models/app_version.rb b/app/models/app_version.rb
index 8e398d9..8696267 100644
--- a/app/models/app_version.rb
+++ b/app/models/app_version.rb
@@ -3,11 +3,10 @@ class AppVersion < ApplicationRecord
   enum :forced_update_type, {unforced_update: 1, forced_update: 2}
 
   def self.current_version(platform:)
-    AppVersion
-      .current
+    current
       .where(platform: platform)
       .order(version: :desc)
       .first
-      .version
+      &.version
   end
 end
diff --git a/app/models/privacy_version.rb b/app/models/privacy_version.rb
index cdc1d50..1b96ad6 100644
--- a/app/models/privacy_version.rb
+++ b/app/models/privacy_version.rb
@@ -2,10 +2,9 @@ class PrivacyVersion < ApplicationRecord
   enum :current_type, {uncurrent: 1, current: 2}
 
   def self.current_version
-    PrivacyVersion
-      .current
+    current
       .order(version: :desc)
       .first
-      .version
+      &.version
   end
 end
diff --git a/app/models/terms_version.rb b/app/models/terms_version.rb
index 0fd3efc..4c97db0 100644
--- a/app/models/terms_version.rb
+++ b/app/models/terms_version.rb
@@ -2,10 +2,9 @@ class TermsVersion < ApplicationRecord
   enum :current_type, {uncurrent: 1, current: 2}
 
   def self.current_version
-    TermsVersion
-      .current
+    current
       .order(version: :desc)
       .first
-      .version
+      &.version
   end
 end
diff --git a/test/models/app_version_test.rb b/test/models/app_version_test.rb
index bcc9427..eb29048 100644
--- a/test/models/app_version_test.rb
+++ b/test/models/app_version_test.rb
@@ -99,6 +99,20 @@ class AppVersionTest < ActiveSupport::TestCase
     assert_equal 5, AppVersion.current_version(platform: "android")
   end
 
+  test "current_version returns nil for nonexistent platform" do
+    assert_nil AppVersion.current_version(platform: "nonexistent")
+  end
+
+  test "current_version respects chained scopes" do
+    version = AppVersion.forced_update.current_version(platform: "ios")
+    assert_not_nil version
+
+    # unforced_update scope should not find forced_update records
+    AppVersion.where(platform: "ios").update_all(forced_update_type: :forced_update)
+    version = AppVersion.unforced_update.current_version(platform: "ios")
+    assert_nil version
+  end
+
   test "should load from fixtures" do
     assert AppVersion.count > 0
 
diff --git a/test/models/privacy_version_test.rb b/test/models/privacy_version_test.rb
index 140c26c..cc739ad 100644
--- a/test/models/privacy_version_test.rb
+++ b/test/models/privacy_version_test.rb
@@ -73,6 +73,11 @@ class PrivacyVersionTest < ActiveSupport::TestCase
     assert_equal 5, PrivacyVersion.current_version
   end
 
+  test "current_version returns nil when no current version exists" do
+    PrivacyVersion.update_all(current_type: :uncurrent)
+    assert_nil PrivacyVersion.current_version
+  end
+
   test "should load from fixtures" do
     assert PrivacyVersion.count > 0
 
diff --git a/test/models/terms_version_test.rb b/test/models/terms_version_test.rb
index 4b97685..b8b753e 100644
--- a/test/models/terms_version_test.rb
+++ b/test/models/terms_version_test.rb
@@ -73,6 +73,11 @@ class TermsVersionTest < ActiveSupport::TestCase
     assert_equal 5, TermsVersion.current_version
   end
 
+  test "current_version returns nil when no current version exists" do
+    TermsVersion.update_all(current_type: :uncurrent)
+    assert_nil TermsVersion.current_version
+  end
+
   test "should load from fixtures" do
     assert TermsVersion.count > 0