From 4b6b8eccedb49014d4cd329a336015fb8d7bcff9 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Thu, 11 Jan 2018 17:22:00 +0100 Subject: Return last edited time instead of update time For issuable models we keep two timestamps: updated_at which is updated whenever any model attribute is changed, last_edited_at which is changed when only title or description is changed. In UI bellow description we display who and when updated the item. But last_edited_by (used for 'who') is mistakenly combined with updated_at (when), last_edited_at should be used instead. Closes #41247 --- app/controllers/concerns/issuable_actions.rb | 2 +- app/helpers/issuables_helper.rb | 2 +- changelogs/unreleased/41247-timestamp.yml | 6 ++ .../controllers/projects/issues_controller_spec.rb | 66 +++++++++++++++------- spec/helpers/issuables_helper_spec.rb | 6 +- 5 files changed, 58 insertions(+), 24 deletions(-) create mode 100644 changelogs/unreleased/41247-timestamp.yml diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb index 74a4f437dc8..337957c366d 100644 --- a/app/controllers/concerns/issuable_actions.rb +++ b/app/controllers/concerns/issuable_actions.rb @@ -45,7 +45,7 @@ module IssuableActions } if issuable.edited? - response[:updated_at] = issuable.updated_at + response[:updated_at] = issuable.last_edited_at.to_time.iso8601 response[:updated_by_name] = issuable.last_edited_by.name response[:updated_by_path] = user_path(issuable.last_edited_by) end diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb index 2668cf78afe..16f39ac96bd 100644 --- a/app/helpers/issuables_helper.rb +++ b/app/helpers/issuables_helper.rb @@ -241,7 +241,7 @@ module IssuablesHelper return {} unless issuable.edited? { - updatedAt: issuable.updated_at.to_time.iso8601, + updatedAt: issuable.last_edited_at.to_time.iso8601, updatedBy: { name: issuable.last_edited_by.name, path: user_path(issuable.last_edited_by) diff --git a/changelogs/unreleased/41247-timestamp.yml b/changelogs/unreleased/41247-timestamp.yml new file mode 100644 index 00000000000..65f1a7485ad --- /dev/null +++ b/changelogs/unreleased/41247-timestamp.yml @@ -0,0 +1,6 @@ +--- +title: For issues display time of last edit of title or description instead of time + of any attribute change +merge_request: +author: +type: fixed diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index 6b7db947216..4a2998b4ccd 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -301,6 +301,53 @@ describe Projects::IssuesController do end end + describe 'GET #realtime_changes' do + def go(id:) + get :realtime_changes, + namespace_id: project.namespace.to_param, + project_id: project, + id: id + end + + context 'when an issue was edited' do + before do + project.add_developer(user) + + issue.update!(last_edited_by: user, last_edited_at: issue.created_at + 1.minute) + + sign_in(user) + end + + it 'returns last edited time' do + go(id: issue.iid) + + data = JSON.parse(response.body) + + expect(data).to include('updated_at') + expect(data['updated_at']).to eq(issue.last_edited_at.to_time.iso8601) + end + end + + context 'when an issue was edited by a deleted user' do + let(:deleted_user) { create(:user) } + + before do + project.add_developer(user) + + issue.update!(last_edited_by: deleted_user, last_edited_at: Time.now) + + deleted_user.destroy + sign_in(user) + end + + it 'returns 200' do + go(id: issue.iid) + + expect(response).to have_gitlab_http_status(200) + end + end + end + describe 'Confidential Issues' do let(:project) { create(:project_empty_repo, :public) } let(:assignee) { create(:assignee) } @@ -589,25 +636,6 @@ describe Projects::IssuesController do project_id: project, id: id end - - context 'when an issue was edited by a deleted user' do - let(:deleted_user) { create(:user) } - - before do - project.add_developer(user) - - issue.update!(last_edited_by: deleted_user, last_edited_at: Time.now) - - deleted_user.destroy - sign_in(user) - end - - it 'returns 200' do - go(id: issue.iid) - - expect(response).to have_gitlab_http_status(200) - end - end end describe 'GET #edit' do diff --git a/spec/helpers/issuables_helper_spec.rb b/spec/helpers/issuables_helper_spec.rb index d601cbdb39b..8a01f7439d6 100644 --- a/spec/helpers/issuables_helper_spec.rb +++ b/spec/helpers/issuables_helper_spec.rb @@ -125,10 +125,10 @@ describe IssuablesHelper do describe '#updated_at_by' do let(:user) { create(:user) } let(:unedited_issuable) { create(:issue) } - let(:edited_issuable) { create(:issue, last_edited_by: user, created_at: 3.days.ago, updated_at: 2.days.ago, last_edited_at: 2.days.ago) } + let(:edited_issuable) { create(:issue, last_edited_by: user, created_at: 3.days.ago, updated_at: 1.day.ago, last_edited_at: 2.days.ago) } let(:edited_updated_at_by) do { - updatedAt: edited_issuable.updated_at.to_time.iso8601, + updatedAt: edited_issuable.last_edited_at.to_time.iso8601, updatedBy: { name: user.name, path: user_path(user) @@ -142,7 +142,7 @@ describe IssuablesHelper do context 'when updated by a deleted user' do let(:edited_updated_at_by) do { - updatedAt: edited_issuable.updated_at.to_time.iso8601, + updatedAt: edited_issuable.last_edited_at.to_time.iso8601, updatedBy: { name: User.ghost.name, path: user_path(User.ghost) -- cgit v1.2.1