diff --git a/app/models/note.rb b/app/models/note.rb index 2d644b03e4dbd4e993acc09b38e048409b44d28f..9ff5e308ed253770d903132282833280ca1288d8 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -7,6 +7,7 @@ class Note < ActiveRecord::Base include Importable include FasterCacheKeys include CacheMarkdownField + include AfterCommitQueue cache_markdown_field :note, pipeline: :note diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index 723cc0e6834f7af3d792495ac3875f016e7404d4..e338792412b8f9e19c42c700530d8f153e4cbfaa 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -26,9 +26,12 @@ module Notes note.note = content end - if !only_commands && note.save + note.run_after_commit do # Finish the harder work in the background - NewNoteWorker.perform_in(2.seconds, note.id, params) + NewNoteWorker.perform_async(note.id) + end + + if !only_commands && note.save todo_service.new_note(note, current_user) end diff --git a/app/workers/new_note_worker.rb b/app/workers/new_note_worker.rb index c3e62bb88c0191a1714bd375f0c195d0463f3002..66574d0fd017b43fb097fc7cbde29f02eded4027 100644 --- a/app/workers/new_note_worker.rb +++ b/app/workers/new_note_worker.rb @@ -2,10 +2,12 @@ class NewNoteWorker include Sidekiq::Worker include DedicatedSidekiqQueue - def perform(note_id, note_params) - note = Note.find(note_id) - - NotificationService.new.new_note(note) - Notes::PostProcessService.new(note).execute + def perform(note_id) + if note = Note.find_by(id: note_id) + NotificationService.new.new_note(note) + Notes::PostProcessService.new(note).execute + else + Rails.logger.error("NewNoteWorker: couldn't find note with ID=#{note_id}, skipping job") + end end end diff --git a/changelogs/unreleased/new-note-worker-record-not-found-fix.yml b/changelogs/unreleased/new-note-worker-record-not-found-fix.yml new file mode 100644 index 0000000000000000000000000000000000000000..abfba640cc0c8a0fbc976ba6a153b7ffd669da94 --- /dev/null +++ b/changelogs/unreleased/new-note-worker-record-not-found-fix.yml @@ -0,0 +1,4 @@ +--- +title: Fix record not found error on NewNoteWorker processing +merge_request: 6863 +author: Oswaldo Ferreira diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 93885c84dc3b9dcc0fa53d71a433a65f357179ae..25804696d2e9ca965f749fefa3354ff20b23fea8 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -14,12 +14,41 @@ describe Notes::CreateService, services: true do end context "valid params" do - before do - @note = Notes::CreateService.new(project, user, opts).execute + it 'returns a valid note' do + note = Notes::CreateService.new(project, user, opts).execute + + expect(note).to be_valid + end + + it 'returns a persisted note' do + note = Notes::CreateService.new(project, user, opts).execute + + expect(note).to be_persisted + end + + it 'note has valid content' do + note = Notes::CreateService.new(project, user, opts).execute + + expect(note.note).to eq(opts[:note]) end - it { expect(@note).to be_valid } - it { expect(@note.note).to eq(opts[:note]) } + it 'TodoService#new_note is called' do + note = build(:note) + allow(project).to receive_message_chain(:notes, :new).with(opts) { note } + + expect_any_instance_of(TodoService).to receive(:new_note).with(note, user) + + Notes::CreateService.new(project, user, opts).execute + end + + it 'enqueues NewNoteWorker' do + note = build(:note, id: 999) + allow(project).to receive_message_chain(:notes, :new).with(opts) { note } + + expect(NewNoteWorker).to receive(:perform_async).with(note.id) + + Notes::CreateService.new(project, user, opts).execute + end end describe 'note with commands' do diff --git a/spec/workers/new_note_worker_spec.rb b/spec/workers/new_note_worker_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..8fdbb35afd0d640b64283b0638c2d1cc7db85f29 --- /dev/null +++ b/spec/workers/new_note_worker_spec.rb @@ -0,0 +1,49 @@ +require "spec_helper" + +describe NewNoteWorker do + context 'when Note found' do + let(:note) { create(:note) } + + it "calls NotificationService#new_note" do + expect_any_instance_of(NotificationService).to receive(:new_note).with(note) + + described_class.new.perform(note.id) + end + + it "calls Notes::PostProcessService#execute" do + notes_post_process_service = double(Notes::PostProcessService) + allow(Notes::PostProcessService).to receive(:new).with(note) { notes_post_process_service } + + expect(notes_post_process_service).to receive(:execute) + + described_class.new.perform(note.id) + end + end + + context 'when Note not found' do + let(:unexistent_note_id) { 999 } + + it 'logs NewNoteWorker process skipping' do + expect(Rails.logger).to receive(:error). + with("NewNoteWorker: couldn't find note with ID=999, skipping job") + + described_class.new.perform(unexistent_note_id) + end + + it 'does not raise errors' do + expect { described_class.new.perform(unexistent_note_id) }.not_to raise_error + end + + it "does not call NotificationService#new_note" do + expect_any_instance_of(NotificationService).not_to receive(:new_note) + + described_class.new.perform(unexistent_note_id) + end + + it "does not call Notes::PostProcessService#execute" do + expect_any_instance_of(Notes::PostProcessService).not_to receive(:execute) + + described_class.new.perform(unexistent_note_id) + end + end +end