Merge branch 'rs-blocks-json-serialization-ee' into 'master'

[EE] Add BlocksJsonSerialization model concern and include it in User

See merge request gitlab-org/gitlab-ee!3835

app/controllers/autocomplete_controller.rb

@@ -8,7 +8,7 @@ class AutocompleteController < ApplicationController
   def users
     @users = AutocompleteUsersFinder.new(params: params, current_user: current_user, project: @project, group: @group).execute
 
-    render json: @users, only: [:name, :username, :id], methods: [:avatar_url]
+    render json: UserSerializer.new.represent(@users)
   end
 
   def project_groups
@@ -17,7 +17,7 @@ class AutocompleteController < ApplicationController
 
   def user
     @user = User.find(params[:id])
-    render json: @user, only: [:name, :username, :id], methods: [:avatar_url]
+    render json: UserSerializer.new.represent(@user)
   end
 
   def projects

app/helpers/form_helper.rb

@@ -37,7 +37,7 @@ module FormHelper
         multi_select: true,
         'input-meta': 'name',
         'always-show-selectbox': true,
-        current_user_info: current_user.to_json(only: [:id, :name])
+        current_user_info: UserSerializer.new.represent(current_user)
       }
     }
   end

app/helpers/issuables_helper.rb

@@ -365,7 +365,7 @@ module IssuablesHelper
       moveIssueEndpoint: move_namespace_project_issue_path(namespace_id: issuable.project.namespace.to_param, project_id: issuable.project, id: issuable),
       projectsAutocompleteEndpoint: autocomplete_projects_path(project_id: @project.id),
       editable: can_edit_issuable,
-      currentUser: current_user.as_json(only: [:username, :id, :name], methods: :avatar_url),
+      currentUser: UserSerializer.new.represent(current_user),
       rootPath: root_path,
       fullPath: @project.full_path,
       weightOptions: Issue.weight_options,

app/helpers/search_helper.rb

@@ -149,7 +149,7 @@ module SearchHelper
         id: "filtered-search-#{type}",
         placeholder: 'Search or filter results...',
         data: {
-          'username-params' => @users.to_json(only: [:id, :username])
+          'username-params' => UserSerializer.new.represent(@users)
         },
         autocomplete: 'off'
       }

app/models/concerns/blocks_json_serialization.rb

+# Overrides `as_json` and `to_json` to raise an exception when called in order
+# to prevent accidentally exposing attributes
+#
+# Not that that would ever happen... but just in case.
+module BlocksJsonSerialization
+  extend ActiveSupport::Concern
+
+  JsonSerializationError = Class.new(StandardError)
+
+  def to_json(*)
+    raise JsonSerializationError,
+      "JSON serialization has been disabled on #{self.class.name}"
+  end
+
+  alias_method :as_json, :to_json
+end

app/models/concerns/time_trackable.rb

@@ -24,7 +24,7 @@ module TimeTrackable
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def spend_time(options)
     @time_spent = options[:duration]
-    @time_spent_user = options[:user]
+    @time_spent_user = User.find(options[:user_id])
     @spent_at = options[:spent_at]
     @original_total_time_spent = nil
 

app/models/user.rb

@@ -18,6 +18,7 @@ class User < ActiveRecord::Base
   include CreatedAtFilterable
   include IgnorableColumn
   include BulkMemberAccessLoad
+  include BlocksJsonSerialization
 
   prepend EE::GeoAwareAvatar
   prepend EE::User

app/services/quick_actions/interpret_service.rb

@@ -406,7 +406,7 @@ module QuickActions
       if time_spent
         @updates[:spend_time] = {
           duration: time_spent,
-          user: current_user,
+          user_id: current_user.id,
           spent_at: time_spent_date
         }
       end
@@ -429,7 +429,7 @@ module QuickActions
         current_user.can?(:"admin_#{issuable.to_ability_name}", project)
     end
     command :remove_time_spent do
-      @updates[:spend_time] = { duration: :reset, user: current_user }
+      @updates[:spend_time] = { duration: :reset, user_id: current_user.id }
     end
 
     desc "Append the comment with #{SHRUG}"

app/views/shared/boards/components/_sidebar.html.haml

-%board-sidebar{ "inline-template" => true,
-  ":current-user" => "#{current_user ? current_user.to_json(only: [:username, :id, :name], methods: [:avatar_url]) : {}}" }
+%board-sidebar{ "inline-template" => true,  ":current-user" => (UserSerializer.new.represent(current_user) || {}).to_json }
   %transition{ name: "boards-sidebar-slide" }
     %aside.right-sidebar.right-sidebar-expanded.issue-boards-sidebar{ "v-show" => "showSidebar" }
       .issuable-sidebar

ee/app/views/projects/issues/service_desk.html.haml

@@ -12,7 +12,7 @@
 - content_for :breadcrumbs_extra do
   = render "projects/issues/nav_btns", show_export_button: false, show_rss_button: false
 
-- support_bot_attrs = User.support_bot.to_json(only: [:id, :name, :username, :avatar_url])
+- support_bot_attrs = UserSerializer.new.represent(User.support_bot).to_json
 
 %div{ class: "#{container_class} js-service-desk-issues service-desk-issues", data: { support_bot: support_bot_attrs } }
   .top-area

lib/api/time_tracking_endpoints.rb

@@ -85,7 +85,7 @@ module API
 
         update_issuable(spend_time: {
           duration: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)),
-          user: current_user
+          user_id: current_user.id
         })
       end
 
@@ -97,7 +97,7 @@ module API
         authorize! update_issuable_key, load_issuable
 
         status :ok
-        update_issuable(spend_time: { duration: :reset, user: current_user })
+        update_issuable(spend_time: { duration: :reset, user_id: current_user.id })
       end
 
       desc "Show time stats for a project #{issuable_name}"

lib/api/v3/time_tracking_endpoints.rb

@@ -86,7 +86,7 @@ module API
 
           update_issuable(spend_time: {
                             duration: Gitlab::TimeTrackingFormatter.parse(params.delete(:duration)),
-                            user: current_user
+                            user_id: current_user.id
                           })
         end
 
@@ -98,7 +98,7 @@ module API
           authorize! update_issuable_key, load_issuable
 
           status :ok
-          update_issuable(spend_time: { duration: :reset, user: current_user })
+          update_issuable(spend_time: { duration: :reset, user_id: current_user.id })
         end
 
         desc "Show time stats for a project #{issuable_name}"

spec/ee/spec/lib/gitlab/geo/oauth_session_spec.rb

@@ -7,7 +7,6 @@ describe Gitlab::Geo::OauthSession do
   let(:dummy_state) { 'salt:hmac:return_to' }
   let(:valid_state) { described_class.new(return_to: oauth_return_to).generate_oauth_state }
   let(:access_token) { FactoryBot.create(:doorkeeper_access_token).token }
-  let(:user) { FactoryBot.build(:user) }
 
   before do
     allow(subject).to receive(:oauth_app) { oauth_app }
@@ -122,7 +121,7 @@ describe Gitlab::Geo::OauthSession do
     context 'on success' do
       it 'returns hashed user data' do
         allow(response).to receive(:status) { 200 }
-        allow(response).to receive(:parsed) { user.to_json }
+        allow(response).to receive(:parsed) { attributes_for(:user) }
 
         subject.authenticate_with_gitlab(access_token)
       end

spec/features/milestone_spec.rb

@@ -82,9 +82,9 @@ feature 'Milestone' do
       milestone = create(:milestone, project: project, title: 8.7)
       issue1 = create(:issue, project: project, milestone: milestone)
       issue2 = create(:issue, project: project, milestone: milestone)
-      issue1.spend_time(duration: 3600, user: user)
+      issue1.spend_time(duration: 3600, user_id: user.id)
       issue1.save!
-      issue2.spend_time(duration: 7200, user: user)
+      issue2.spend_time(duration: 7200, user_id: user.id)
       issue2.save!
 
       visit project_milestone_path(project, milestone)

spec/models/concerns/blocks_json_serialization_spec.rb

+require 'rails_helper'
+
+describe BlocksJsonSerialization do
+  DummyModel = Class.new do
+    include BlocksJsonSerialization
+  end
+
+  it 'blocks as_json' do
+    expect { DummyModel.new.as_json }
+      .to raise_error(described_class::JsonSerializationError, /DummyModel/)
+  end
+
+  it 'blocks to_json' do
+    expect { DummyModel.new.to_json }
+      .to raise_error(described_class::JsonSerializationError, /DummyModel/)
+  end
+end

spec/models/concerns/issuable_spec.rb

@@ -308,7 +308,7 @@ describe Issuable do
 
     context 'total_time_spent is updated' do
       before do
-        issue.spend_time(duration: 2, user: user, spent_at: Time.now)
+        issue.spend_time(duration: 2, user_id: user.id, spent_at: Time.now)
         issue.save
         expect(Gitlab::HookData::IssuableBuilder)
           .to receive(:new).with(issue).and_return(builder)
@@ -519,7 +519,7 @@ describe Issuable do
     let(:issue) { create(:issue) }
 
     def spend_time(seconds)
-      issue.spend_time(duration: seconds, user: user)
+      issue.spend_time(duration: seconds, user_id: user.id)
       issue.save!
     end
 

spec/models/concerns/milestoneish_spec.rb

@@ -189,9 +189,9 @@ describe Milestone, 'Milestoneish' do
 
   describe '#total_issue_time_spent' do
     it 'calculates total issue time spent' do
-      closed_issue_1.spend_time(duration: 300, user: author)
+      closed_issue_1.spend_time(duration: 300, user_id: author.id)
       closed_issue_1.save!
-      closed_issue_2.spend_time(duration: 600, user: assignee)
+      closed_issue_2.spend_time(duration: 600, user_id: assignee.id)
       closed_issue_2.save!
 
       expect(milestone.total_issue_time_spent).to eq(900)

spec/models/user_spec.rb

@@ -12,6 +12,7 @@ describe User do
     it { is_expected.to include_module(Referable) }
     it { is_expected.to include_module(Sortable) }
     it { is_expected.to include_module(TokenAuthenticatable) }
+    it { is_expected.to include_module(BlocksJsonSerialization) }
   end
 
   describe 'delegations' do

spec/services/quick_actions/interpret_service_spec.rb

@@ -211,7 +211,7 @@ describe QuickActions::InterpretService do
 
         expect(updates).to eq(spend_time: {
                                 duration: 3600,
-                                user: developer,
+                                user_id: developer.id,
                                 spent_at: DateTime.now.to_date
                               })
       end
@@ -223,7 +223,7 @@ describe QuickActions::InterpretService do
 
         expect(updates).to eq(spend_time: {
                                 duration: -1800,
-                                user: developer,
+                                user_id: developer.id,
                                 spent_at: DateTime.now.to_date
                               })
       end
@@ -235,7 +235,7 @@ describe QuickActions::InterpretService do
 
         expect(updates).to eq(spend_time: {
                                 duration: 1800,
-                                user: developer,
+                                user_id: developer.id,
                                 spent_at: Date.parse(date)
                               })
       end
@@ -269,7 +269,7 @@ describe QuickActions::InterpretService do
       it 'populates spend_time: :reset if content contains /remove_time_spent' do
         _, updates = service.execute(content, issuable)
 
-        expect(updates).to eq(spend_time: { duration: :reset, user: developer })
+        expect(updates).to eq(spend_time: { duration: :reset, user_id: developer.id })
       end
     end
 

spec/services/system_note_service_spec.rb

@@ -961,53 +961,6 @@ describe SystemNoteService do
     end
   end
 
-  describe '.change_time_spent' do
-    # We need a custom noteable in order to the shared examples to be green.
-    let(:noteable) do
-      mr = create(:merge_request, source_project: project)
-      mr.spend_time(duration: 360000, user: author)
-      mr.save!
-      mr
-    end
-
-    subject do
-      described_class.change_time_spent(noteable, project, author)
-    end
-
-    it_behaves_like 'a system note' do
-      let(:action) { 'time_tracking' }
-    end
-
-    context 'when time was added' do
-      it 'sets the note text' do
-        spend_time!(277200)
-
-        expect(subject.note).to eq "added 1w 4d 5h of time spent"
-      end
-    end
-
-    context 'when time was subtracted' do
-      it 'sets the note text' do
-        spend_time!(-277200)
-
-        expect(subject.note).to eq "subtracted 1w 4d 5h of time spent"
-      end
-    end
-
-    context 'when time was removed' do
-      it 'sets the note text' do
-        spend_time!(:reset)
-
-        expect(subject.note).to eq "removed time spent"
-      end
-    end
-
-    def spend_time!(seconds)
-      noteable.spend_time(duration: seconds, user: author)
-      noteable.save!
-    end
-  end
-
   describe '.discussion_continued_in_issue' do
     let(:discussion) { create(:diff_note_on_merge_request, project: project).to_discussion }
     let(:merge_request) { discussion.noteable }
@@ -1060,7 +1013,7 @@ describe SystemNoteService do
     # We need a custom noteable in order to the shared examples to be green.
     let(:noteable) do
       mr = create(:merge_request, source_project: project)
-      mr.spend_time(duration: 360000, user: author)
+      mr.spend_time(duration: 360000, user_id: author.id)
       mr.save!
       mr
     end
@@ -1098,7 +1051,7 @@ describe SystemNoteService do
     end
 
     def spend_time!(seconds)
-      noteable.spend_time(duration: seconds, user: author)
+      noteable.spend_time(duration: seconds, user_id: author.id)
       noteable.save!
     end
   end

spec/support/api/time_tracking_shared_examples.rb

@@ -79,7 +79,7 @@ shared_examples 'time tracking endpoints' do |issuable_name|
 
     context 'when subtracting time' do
       it 'subtracts time of the total spent time' do
-        issuable.update_attributes!(spend_time: { duration: 7200, user: user })
+        issuable.update_attributes!(spend_time: { duration: 7200, user_id: user.id })
 
         post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", user),
              duration: '-1h'
@@ -91,7 +91,7 @@ shared_examples 'time tracking endpoints' do |issuable_name|
 
     context 'when time to subtract is greater than the total spent time' do
       it 'does not modify the total time spent' do
-        issuable.update_attributes!(spend_time: { duration: 7200, user: user })
+        issuable.update_attributes!(spend_time: { duration: 7200, user_id: user.id })
 
         post api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/add_spent_time", user),
              duration: '-1w'
@@ -119,7 +119,7 @@ shared_examples 'time tracking endpoints' do |issuable_name|
 
   describe "GET /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_stats" do
     it "returns the time stats for #{issuable_name}" do
-      issuable.update_attributes!(spend_time: { duration: 1800, user: user },
+      issuable.update_attributes!(spend_time: { duration: 1800, user_id: user.id },
                                   time_estimate: 3600)
 
       get api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.iid}/time_stats", user)

spec/support/api/v3/time_tracking_shared_examples.rb

@@ -75,7 +75,7 @@ shared_examples 'V3 time tracking endpoints' do |issuable_name|
 
     context 'when subtracting time' do
       it 'subtracts time of the total spent time' do
-        issuable.update_attributes!(spend_time: { duration: 7200, user: user })
+        issuable.update_attributes!(spend_time: { duration: 7200, user_id: user.id })
 
         post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user),
              duration: '-1h'
@@ -87,7 +87,7 @@ shared_examples 'V3 time tracking endpoints' do |issuable_name|
 
     context 'when time to subtract is greater than the total spent time' do
       it 'does not modify the total time spent' do
-        issuable.update_attributes!(spend_time: { duration: 7200, user: user })
+        issuable.update_attributes!(spend_time: { duration: 7200, user_id: user.id })
 
         post v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/add_spent_time", user),
              duration: '-1w'
@@ -115,7 +115,7 @@ shared_examples 'V3 time tracking endpoints' do |issuable_name|
 
   describe "GET /projects/:id/#{issuable_collection_name}/:#{issuable_name}_id/time_stats" do
     it "returns the time stats for #{issuable_name}" do
-      issuable.update_attributes!(spend_time: { duration: 1800, user: user },
+      issuable.update_attributes!(spend_time: { duration: 1800, user_id: user.id },
                                   time_estimate: 3600)
 
       get v3_api("/projects/#{project.id}/#{issuable_collection_name}/#{issuable.id}/time_stats", user)