Make Web::PushSubscription#user and Web::PushSubscription#access_token relationships non-optional (#34498)

Co-authored-by: Emelia Smith <ThisIsMissEm@users.noreply.github.com>

app/models/web/push_subscription.rb

@@ -12,13 +12,13 @@
 #  standard        :boolean          default(FALSE), not null
 #  created_at      :datetime         not null
 #  updated_at      :datetime         not null
-#  access_token_id :bigint(8)
+#  access_token_id :bigint(8)        not null
-#  user_id         :bigint(8)
+#  user_id         :bigint(8)        not null
 #
 
 class Web::PushSubscription < ApplicationRecord
-  belongs_to :user, optional: true
+  belongs_to :user
-  belongs_to :access_token, class_name: 'Doorkeeper::AccessToken', optional: true
+  belongs_to :access_token, class_name: 'Doorkeeper::AccessToken'
 
   has_one :session_activation, foreign_key: 'web_push_subscription_id', inverse_of: :web_push_subscription, dependent: nil
 
@@ -28,7 +28,7 @@ class Web::PushSubscription < ApplicationRecord
 
   validates_with WebPushKeyValidator
 
-  delegate :locale, to: :associated_user
+  delegate :locale, to: :user
 
   generates_token_for :unsubscribe, expires_in: Web::PushNotificationWorker::TTL
 
@@ -36,24 +36,8 @@ class Web::PushSubscription < ApplicationRecord
     policy_allows_notification?(notification) && alert_enabled_for_notification_type?(notification)
   end
 
-  def associated_user
-    return @associated_user if defined?(@associated_user)
-
-    @associated_user = if user_id.nil?
-                         session_activation.user
-                       else
-                         user
-                       end
-  end
-
   def associated_access_token
-    return @associated_access_token if defined?(@associated_access_token)
+    access_token.token
-
-    @associated_access_token = if access_token_id.nil?
-                                 find_or_create_access_token.token
-                               else
-                                 access_token.token
-                               end
   end
 
   class << self
@@ -65,16 +49,6 @@ class Web::PushSubscription < ApplicationRecord
 
   private
 
-  def find_or_create_access_token
-    Doorkeeper::AccessToken.find_or_create_for(
-      application: Doorkeeper::Application.find_by(superapp: true),
-      resource_owner: user_id || session_activation.user_id,
-      scopes: Doorkeeper::OAuth::Scopes.from_string('read write follow push'),
-      expires_in: Doorkeeper.configuration.access_token_expires_in,
-      use_refresh_token: Doorkeeper.configuration.refresh_token_enabled?
-    )
-  end
-
   def alert_enabled_for_notification_type?(notification)
     truthy?(data&.dig('alerts', notification.type.to_s))
   end

app/serializers/web/notification_serializer.rb

@@ -13,7 +13,7 @@ class Web::NotificationSerializer < ActiveModel::Serializer
   end
 
   def preferred_locale
-    current_push_subscription.associated_user&.locale || I18n.default_locale
+    current_push_subscription.user&.locale || I18n.default_locale
   end
 
   def notification_id

db/migrate/20250422083912_add_not_null_to_web_push_subscription_user.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddNotNullToWebPushSubscriptionUser < ActiveRecord::Migration[8.0]
+  def change
+    add_check_constraint :web_push_subscriptions, 'user_id IS NOT NULL', name: 'web_push_subscriptions_user_id_null', validate: false
+  end
+end

db/migrate/20250422084214_validate_add_not_null_to_web_push_subscription_user.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class ValidateAddNotNullToWebPushSubscriptionUser < ActiveRecord::Migration[8.0]
+  def up
+    connection.execute(<<~SQL.squish)
+      DELETE FROM web_push_subscriptions
+      WHERE user_id IS NULL
+    SQL
+
+    validate_check_constraint :web_push_subscriptions, name: 'web_push_subscriptions_user_id_null'
+    change_column_null :web_push_subscriptions, :user_id, false
+    remove_check_constraint :web_push_subscriptions, name: 'web_push_subscriptions_user_id_null'
+  end
+
+  def down
+    add_check_constraint :web_push_subscriptions, 'user_id IS NOT NULL', name: 'web_push_subscriptions_user_id_null', validate: false
+    change_column_null :web_push_subscriptions, :user_id, true
+  end
+end

db/migrate/20250422085027_add_not_null_to_web_push_subscription_access_token.rb

@@ -0,0 +1,7 @@
+# frozen_string_literal: true
+
+class AddNotNullToWebPushSubscriptionAccessToken < ActiveRecord::Migration[8.0]
+  def change
+    add_check_constraint :web_push_subscriptions, 'access_token_id IS NOT NULL', name: 'web_push_subscriptions_access_token_id_null', validate: false
+  end
+end

db/migrate/20250422085303_validate_add_not_null_to_web_push_subscription_access_token.rb

@@ -0,0 +1,19 @@
+# frozen_string_literal: true
+
+class ValidateAddNotNullToWebPushSubscriptionAccessToken < ActiveRecord::Migration[8.0]
+  def up
+    connection.execute(<<~SQL.squish)
+      DELETE FROM web_push_subscriptions
+      WHERE access_token_id IS NULL
+    SQL
+
+    validate_check_constraint :web_push_subscriptions, name: 'web_push_subscriptions_access_token_id_null'
+    change_column_null :web_push_subscriptions, :access_token_id, false
+    remove_check_constraint :web_push_subscriptions, name: 'web_push_subscriptions_access_token_id_null'
+  end
+
+  def down
+    add_check_constraint :web_push_subscriptions, 'access_token_id IS NOT NULL', name: 'web_push_subscriptions_access_token_id_null', validate: false
+    change_column_null :web_push_subscriptions, :access_token_id, true
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema[8.0].define(version: 2025_04_11_095859) do
+ActiveRecord::Schema[8.0].define(version: 2025_04_22_085303) do
   # These are extensions that must be enabled in order to support this database
   enable_extension "pg_catalog.plpgsql"
 
@@ -1237,8 +1237,8 @@ ActiveRecord::Schema[8.0].define(version: 2025_04_11_095859) do
     t.json "data"
     t.datetime "created_at", precision: nil, null: false
     t.datetime "updated_at", precision: nil, null: false
-    t.bigint "access_token_id"
+    t.bigint "access_token_id", null: false
-    t.bigint "user_id"
+    t.bigint "user_id", null: false
     t.boolean "standard", default: false, null: false
     t.index ["access_token_id"], name: "index_web_push_subscriptions_on_access_token_id", where: "(access_token_id IS NOT NULL)"
     t.index ["user_id"], name: "index_web_push_subscriptions_on_user_id"

spec/fabricators/web_push_subscription_fabricator.rb

@@ -8,4 +8,6 @@ Fabricator(:web_push_subscription, from: Web::PushSubscription) do
     Base64.urlsafe_encode64(ecdh_key)
   end
   key_auth { Base64.urlsafe_encode64(Random.new.bytes(16)) }
+  user { Fabricate(:user) }
+  access_token { |attrs| Fabricate.build(:accessible_access_token, resource_owner_id: attrs[:user].id) }
 end

spec/requests/api/v1/push/subscriptions_spec.rb

@@ -207,7 +207,8 @@ RSpec.describe 'API V1 Push Subscriptions' do
     Fabricate(
       :web_push_subscription,
       endpoint: create_payload[:subscription][:endpoint],
-      access_token_id: token.id
+      access_token: token,
+      user: user
     )
   end
 end