[openstreetmap/openstreetmap-website] Enable active_record.belongs_to_required_by_default (PR #3470)

Tom Hughes notifications at github.com
Wed Feb 23 15:57:51 UTC 2022


@tomhughes commented on this pull request.



> @@ -36,8 +36,8 @@
 #
 
 class AccessToken < OauthToken
-  belongs_to :user
-  belongs_to :client_application
+  belongs_to :user, :optional => true
+  belongs_to :client_application, :optional => true

Are these really optional? A token that isn't associated with a user or application?

> @@ -31,7 +31,7 @@
 #
 
 class ClientApplication < ApplicationRecord
-  belongs_to :user
+  belongs_to :user, :optional => true

Again I'm pretty sure all applications should be associated with a user.

> @@ -32,9 +32,9 @@
 
 class Issue < ApplicationRecord
   belongs_to :reportable, :polymorphic => true
-  belongs_to :reported_user, :class_name => "User"
-  belongs_to :user_resolved, :class_name => "User", :foreign_key => :resolved_by
-  belongs_to :user_updated, :class_name => "User", :foreign_key => :updated_by
+  belongs_to :reported_user, :class_name => "User", :optional => true

Is this not set for all issues?

> @@ -32,9 +32,9 @@
 
 class Issue < ApplicationRecord
   belongs_to :reportable, :polymorphic => true
-  belongs_to :reported_user, :class_name => "User"
-  belongs_to :user_resolved, :class_name => "User", :foreign_key => :resolved_by
-  belongs_to :user_updated, :class_name => "User", :foreign_key => :updated_by
+  belongs_to :reported_user, :class_name => "User", :optional => true
+  belongs_to :user_resolved, :class_name => "User", :foreign_key => :resolved_by, :optional => true
+  belongs_to :user_updated, :class_name => "User", :foreign_key => :updated_by, :optional => true

Not sure about this one - it probably depends on whether the initial creation counts as an update...

> @@ -36,8 +36,8 @@
 #
 
 class OauthToken < ApplicationRecord
-  belongs_to :client_application
-  belongs_to :user
+  belongs_to :client_application, :optional => true
+  belongs_to :user, :optional => true

As with `AccessToken` these both look like they should be required?

-- 
Reply to this email directly or view it on GitHub:
https://github.com/openstreetmap/openstreetmap-website/pull/3470#pullrequestreview-891295016
You are receiving this because you are subscribed to this thread.

Message ID: <openstreetmap/openstreetmap-website/pull/3470/review/891295016 at github.com>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.openstreetmap.org/pipermail/rails-dev/attachments/20220223/12dc42a3/attachment.htm>


More information about the rails-dev mailing list