Automatic merge of gitlab-org/gitlab-ce master

app/assets/javascripts/jobs/components/job_log_controllers.vue

@@ -3,7 +3,7 @@ import { GlTooltipDirective, GlLink, GlButton } from '@gitlab/ui';
 import { polyfillSticky } from '~/lib/utils/sticky';
 import Icon from '~/vue_shared/components/icon.vue';
 import { numberToHumanSize } from '~/lib/utils/number_utils';
-import { sprintf } from '~/locale';
+import { __, sprintf } from '~/locale';
 import scrollDown from '../svg/scroll_down.svg';
 
 export default {
@@ -50,7 +50,7 @@ export default {
   },
   computed: {
     jobLogSize() {
-      return sprintf('Showing last %{size} of log -', {
+      return sprintf(__('Showing last %{size} of log -'), {
         size: numberToHumanSize(this.size),
       });
     },
@@ -74,14 +74,12 @@ export default {
     <div class="js-truncated-info truncated-info d-none d-sm-block float-left">
       <template v-if="isTraceSizeVisible">
         {{ jobLogSize }}
-
         <gl-link
           v-if="rawPath"
           :href="rawPath"
           class="js-raw-link text-plain text-underline prepend-left-5"
+          >{{ s__('Job|Complete Raw') }}</gl-link
         >
-          {{ s__('Job|Complete Raw') }}
-        </gl-link>
       </template>
     </div>
     <!-- eo truncate information -->

app/assets/javascripts/jobs/components/sidebar.vue

 <script>
+import { __, sprintf } from '~/locale';
 import _ from 'underscore';
 import { mapActions, mapState } from 'vuex';
 import { GlLink, GlButton } from '@gitlab/ui';
@@ -63,7 +64,9 @@ export default {
 
       let t = this.job.metadata.timeout_human_readable;
       if (this.job.metadata.timeout_source !== '') {
-        t += ` (from ${this.job.metadata.timeout_source})`;
+        t += sprintf(__(` (from %{timeoutSource})`), {
+          timeoutSource: this.job.metadata.timeout_source,
+        });
       }
 
       return t;

app/controllers/dashboard/todos_controller.rb

@@ -10,6 +10,7 @@ class Dashboard::TodosController < Dashboard::ApplicationController
   def index
     @sort = params[:sort]
     @todos = @todos.page(params[:page])
+    @todos = @todos.with_entity_associations
 
     return if redirect_out_of_range(@todos)
   end

app/helpers/todos_helper.rb

@@ -170,7 +170,7 @@ module TodosHelper
   end
 
   def todo_group_options
-    groups = current_user.authorized_groups.map do |group|
+    groups = current_user.authorized_groups.with_route.map do |group|
       { id: group.id, text: group.full_name }
     end
 

app/models/todo.rb

@@ -60,7 +60,7 @@ class Todo < ApplicationRecord
   scope :for_type, -> (type) { where(target_type: type) }
   scope :for_target, -> (id) { where(target_id: id) }
   scope :for_commit, -> (id) { where(commit_id: id) }
-  scope :with_api_entity_associations, -> { preload(:target, :author, :note, group: :route, project: [:route, { namespace: :route }]) }
+  scope :with_entity_associations, -> { preload(:target, :author, :note, group: :route, project: [:route, { namespace: :route }]) }
   scope :joins_issue_and_assignees, -> { left_joins(issue: :assignees) }
 
   state_machine :state, initial: :pending do

changelogs/unreleased/sh-optimize-todos-controller.yml

+---
+title: Eliminate N+1 queries in Dashboard::TodosController
+merge_request: 29954
+author:
+type: performance

lib/api/todos.rb

@@ -78,7 +78,7 @@ module API
         use :pagination
       end
       get do
-        todos = paginate(find_todos.with_api_entity_associations)
+        todos = paginate(find_todos.with_entity_associations)
         options = { with: Entities::Todo, current_user: current_user }
         batch_load_issuable_metadata(todos, options)
 

locale/gitlab.pot

@@ -16,6 +16,9 @@ msgstr ""
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n"
 
+msgid " (from %{timeoutSource})"
+msgstr ""
+
 msgid " Please sign in."
 msgstr ""
 

spec/controllers/dashboard/todos_controller_spec.rb

@@ -44,6 +44,34 @@ describe Dashboard::TodosController do
       end
     end
 
+    context "with render_views" do
+      render_views
+
+      it 'avoids N+1 queries', :request_store do
+        merge_request = create(:merge_request, source_project: project)
+        create(:todo, project: project, author: author, user: user, target: merge_request)
+        create(:issue, project: project, assignees: [user])
+
+        group = create(:group)
+        group.add_owner(user)
+
+        get :index
+
+        control = ActiveRecord::QueryRecorder.new { get :index }
+
+        create(:issue, project: project, assignees: [user])
+        group_2 = create(:group)
+        group_2.add_owner(user)
+        project_2 = create(:project)
+        project_2.add_developer(user)
+        merge_request_2 = create(:merge_request, source_project: project_2)
+        create(:todo, project: project, author: author, user: user, target: merge_request_2)
+
+        expect { get :index }.not_to exceed_query_limit(control)
+        expect(response.status).to eq(200)
+      end
+    end
+
     context 'group authorization' do
       it 'renders 404 when user does not have read access on given group' do
         unauthorized_group = create(:group, :private)