Remove redundant WHERE from event queries

The default scope in Event would add a "WHERE author_id IS NOT NULL" clause to every query. Now that "events.author_id" has a NOT NULL clause set this filter is redundant and we can safely remove it. In this commit we also add a validation on events.author_id (previously only defined in PushEvent) just in case something tries to create data without an author ID. This way we can present a nicer error message compared to PostgreSQL's foreign key error messages. Fixes https://gitlab.com/gitlab-org/gitlab-ce/issues/38129

Remove redundant WHERE from event queries
9a3e4b8d · Yorick Peterse · a09d032b · 9a3e4b8d · 9a3e4b8d · 9a3e4b8d
Unverified Commit 9a3e4b8d authored Sep 20, 2017 by Yorick Peterse
Hide whitespace changes
Inline Side-by-side

Showing with 12 additions and 7 deletions

event.rb app/models/event.rb +7 -1

push_event.rb app/models/push_event.rb +0 -6

events-redundant-where.yml changelogs/unreleased/events-redundant-where.yml +5 -0

No files found.
--- a/app/models/event.rb
+++ b/app/models/event.rb
 class Event < ActiveRecord::Base
  include Sortable
  include IgnorableColumn
-  default_scope { reorder(nil).where.not(author_id: nil) }
+  default_scope { reorder(nil) }

  CREATED   = 1
  UPDATED   = 2
@@ -77,6 +77,12 @@ class Event < ActiveRecord::Base

  scope :for_milestone_id, ->(milestone_id) { where(target_type: "Milestone", target_id: milestone_id) }

+  # Authors are required as they're used to display who pushed data.
+  #
+  # We're just validating the presence of the ID here as foreign key constraints
+  # should ensure the ID points to a valid user.
+  validates :author_id, presence: true
+
  self.inheritance_column = 'action'

  # "data" will be removed in 10.0 but it may be possible that JOINs happen that

--- a/app/models/push_event.rb
+++ b/app/models/push_event.rb
@@ -3,12 +3,6 @@ class PushEvent < Event
  # different "action" value.
  validate :validate_push_action

-  # Authors are required as they're used to display who pushed data.
-  #
-  # We're just validating the presence of the ID here as foreign key constraints
-  # should ensure the ID points to a valid user.
-  validates :author_id, presence: true
-
  # The project is required to build links to commits, commit ranges, etc.
  #
  # We're just validating the presence of the ID here as foreign key constraints

--- a/changelogs/unreleased/events-redundant-where.yml
+++ b/changelogs/unreleased/events-redundant-where.yml
+---
+title: Remove redundant WHERE from event queries
+merge_request:
+author:
+type: other