Merge branch 'add_note_id_to_timelogs' into 'master'

Add note_id to timelogs

Closes #223615

See merge request gitlab-org/gitlab!35916

app/models/concerns/time_trackable.rb

@@ -26,6 +26,7 @@ module TimeTrackable
   # rubocop:disable Gitlab/ModuleWithInstanceVariables
   def spend_time(options)
     @time_spent = options[:duration]
+    @time_spent_note_id = options[:note_id]
     @time_spent_user = User.find(options[:user_id])
     @spent_at = options[:spent_at]
     @original_total_time_spent = nil
@@ -67,6 +68,7 @@ module TimeTrackable
   def add_or_subtract_spent_time
     timelogs.new(
       time_spent: time_spent,
+      note_id: @time_spent_note_id,
       user: @time_spent_user,
       spent_at: @spent_at
     )

app/services/notes/quick_actions_service.rb

@@ -50,6 +50,11 @@ module Notes
       return if update_params.empty?
       return unless supported?(note)
 
+      # We need the `id` after the note is persisted
+      if update_params[:spend_time]
+        update_params[:spend_time][:note_id] = note.id
+      end
+
       self.class.noteable_update_service(note).new(note.resource_parent, current_user, update_params).execute(note.noteable)
     end
   end

changelogs/unreleased/add_note_id_to_timelogs.yml

+---
+title: Add note_id to timelogs
+merge_request: 35916
+author: Lee Tickett
+type: added

db/migrate/20200703035021_add_notes_to_timelogs.rb

+# frozen_string_literal: true
+
+class AddNotesToTimelogs < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    with_lock_retries do
+      add_column :timelogs, :note_id, :integer
+    end
+    add_concurrent_index :timelogs, :note_id
+    add_concurrent_foreign_key :timelogs, :notes, column: :note_id
+  end
+
+  def down
+    remove_foreign_key_if_exists :timelogs, column: :note_id
+    remove_concurrent_index :timelogs, :note_id
+    with_lock_retries do
+      remove_column :timelogs, :note_id
+    end
+  end
+end

db/structure.sql

@@ -15672,7 +15672,8 @@ CREATE TABLE public.timelogs (
     updated_at timestamp without time zone NOT NULL,
     issue_id integer,
     merge_request_id integer,
-    spent_at timestamp without time zone
+    spent_at timestamp without time zone,
+    note_id integer
 );
 
 CREATE SEQUENCE public.timelogs_id_seq
@@ -20579,6 +20580,8 @@ CREATE INDEX index_timelogs_on_issue_id ON public.timelogs USING btree (issue_id
 
 CREATE INDEX index_timelogs_on_merge_request_id ON public.timelogs USING btree (merge_request_id);
 
+CREATE INDEX index_timelogs_on_note_id ON public.timelogs USING btree (note_id);
+
 CREATE INDEX index_timelogs_on_spent_at ON public.timelogs USING btree (spent_at) WHERE (spent_at IS NOT NULL);
 
 CREATE INDEX index_timelogs_on_user_id ON public.timelogs USING btree (user_id);
@@ -21431,6 +21434,9 @@ ALTER TABLE ONLY public.issues
 ALTER TABLE ONLY public.protected_branch_merge_access_levels
     ADD CONSTRAINT fk_8a3072ccb3 FOREIGN KEY (protected_branch_id) REFERENCES public.protected_branches(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY public.timelogs
+    ADD CONSTRAINT fk_8d058cd571 FOREIGN KEY (note_id) REFERENCES public.notes(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY public.releases
     ADD CONSTRAINT fk_8e4456f90f FOREIGN KEY (author_id) REFERENCES public.users(id) ON DELETE SET NULL;
 
@@ -24042,6 +24048,7 @@ COPY "schema_migrations" (version) FROM STDIN;
 20200701221303
 20200702123805
 20200702201039
+20200703035021
 20200703064117
 20200703121557
 20200703124823

lib/gitlab/import_export/project/import_export.yml

@@ -274,6 +274,7 @@ excluded_attributes:
   timelogs:
     - :issue_id
     - :merge_request_id
+    - :note_id
   notes:
     - :noteable_id
     - :review_id

spec/services/notes/quick_actions_service_spec.rb

@@ -58,14 +58,29 @@ RSpec.describe Notes::QuickActionsService do
       end
 
       describe '/spend' do
-        let(:note_text) { '/spend 1h' }
+        context 'when note is not persisted' do
+          let(:note_text) { '/spend 1h' }
 
-        it 'updates the spent time on the noteable' do
-          content, update_params = service.execute(note)
-          service.apply_updates(update_params, note)
+          it 'adds time to noteable, adds timelog with nil note_id and has no content' do
+            content, update_params = service.execute(note)
+            service.apply_updates(update_params, note)
 
-          expect(content).to eq ''
-          expect(note.noteable.time_spent).to eq(3600)
+            expect(content).to eq ''
+            expect(note.noteable.time_spent).to eq(3600)
+            expect(Timelog.last.note_id).to be_nil
+          end
+        end
+
+        context 'when note is persisted' do
+          let(:note_text) { "a note \n/spend 1h" }
+
+          it 'updates the spent time and populates timelog with note_id' do
+            new_content, update_params = service.execute(note)
+            note.update!(note: new_content)
+            service.apply_updates(update_params, note)
+
+            expect(Timelog.last.note_id).to eq(note.id)
+          end
         end
       end
     end