From 32d0b1c6cd991d13bf25c16f77ad28941bd94b5f Mon Sep 17 00:00:00 2001 From: Anwar Sadath Date: Thu, 6 Feb 2020 21:41:43 +0530 Subject: [PATCH 01/38] Rebase master --- .../merge_requests/context_controller.rb | 36 ++++++++++++++ app/models/merge_request.rb | 6 +++ lib/api/merge_requests.rb | 47 +++++++++++++++++++ 3 files changed, 89 insertions(+) create mode 100644 app/controllers/projects/merge_requests/context_controller.rb diff --git a/app/controllers/projects/merge_requests/context_controller.rb b/app/controllers/projects/merge_requests/context_controller.rb new file mode 100644 index 00000000000000..ce68f0dae9a57a --- /dev/null +++ b/app/controllers/projects/merge_requests/context_controller.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +class Projects::MergeRequests::ContextController < Projects::MergeRequests::ApplicationController + before_action :authenticate_user! + + def index + render json: @merge_request.context_diffs + end + + def create + commit_ids = params[:commits] + project = @merge_request.target_project + if commit_ids.is_a?(Array) + commits = commit_ids.map do |commit_id| + project.repository.commit_by(oid: commit_id) + end + MergeRequestContextCommit.create_bulk(@merge_request, project.repository.raw_repository, commits) + render json: { status: :success } + else + render json: :bad_request + end + end + + def destroy + commit_ids = params[:commits] + if commit_ids.is_a?(Array) + commits = commit_ids.map do |commit_id| + project.repository.commit_by(oid: commit_id) + end + MergeRequestContextCommit.delete_bulk(@merge_request, commits) + render json: { status: :success } + else + render json: :bad_request + end + end +end diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 62f34ae6525dbd..a97a51dd1c6ef7 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -540,6 +540,12 @@ def discussions_diffs end end + def context_diffs + MergeRequestContextCommitDiffFile.where( + merge_request_context_commit_id: self.merge_request_context_commits + ) + end + def diff_size # Calling `merge_request_diff.diffs.real_size` will also perform # highlighting, which we don't need here. diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 2b1bcc855d2738..cf9951ad7df347 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -362,6 +362,53 @@ def handle_merge_request_errors!(errors) status 204 end + desc 'Get the context commits of a merge request' do + success Entities::MergeRequestContextCommit + end + get ':id/merge_requests/:merge_request_iid/context_commits' do + merge_request = find_merge_request_with_access(params[:merge_request_iid]) + context_diffs = merge_request.context_diffs + present paginate(context_diffs), with: Entities::MergeRequestContextCommit + end + + params do + optional :commits, type: Array, allow_blank: false, desc: 'List of context commits sha' + end + desc 'create context commits of merge request' do + success Entities::Commit + end + post ':id/merge_requests/:merge_request_iid/context_commits' do + commit_ids = params[:commits] + merge_request = find_merge_request_with_access(params[:merge_request_iid]) + project = merge_request.target_project + commits = commit_ids.map do |commit_id| + commit = project.repository.commit_by(oid: commit_id) + render_api_error!("Commit not found for #{commit_id}", 400) unless commit + commit + end + MergeRequestContextCommit.create_bulk(merge_request, project.repository.raw_repository, commits) + present commits, with: Entities::Commit + end + + params do + optional :commits, type: Array, allow_blank: false, desc: 'List of context commits sha' + end + desc 'remove context commits of merge request' do + success Entities::Commit + end + delete ':id/merge_requests/:merge_request_iid/context_commits' do + commit_ids = params[:commits] + merge_request = find_merge_request_with_access(params[:merge_request_iid]) + project = merge_request.target_project + commits = commit_ids.map do |commit_id| + commit = project.repository.commit_by(oid: commit_id) + render_api_error!("Commit not found for #{commit_id}", 400) unless commit + commit + end + MergeRequestContextCommit.delete_bulk(merge_request, commits) + present commits, with: Entities::Commit + end + desc 'Show the merge request changes' do success Entities::MergeRequestChanges end -- GitLab From 37e6c212ae44de7b8fe78cba840cd93f8e51657d Mon Sep 17 00:00:00 2001 From: Lin Xiangyu Date: Sat, 28 Sep 2019 18:56:47 +0800 Subject: [PATCH 02/38] Fix context commits N+1 gitaly query, add transaction for bulk create merge_request_context_commit_diff_files, fix lint issues --- .../merge_requests/context_controller.rb | 36 ------------------- lib/api/merge_requests.rb | 18 +++++----- lib/gitlab/import_export/import_export.yml | 3 ++ 3 files changed, 13 insertions(+), 44 deletions(-) delete mode 100644 app/controllers/projects/merge_requests/context_controller.rb diff --git a/app/controllers/projects/merge_requests/context_controller.rb b/app/controllers/projects/merge_requests/context_controller.rb deleted file mode 100644 index ce68f0dae9a57a..00000000000000 --- a/app/controllers/projects/merge_requests/context_controller.rb +++ /dev/null @@ -1,36 +0,0 @@ -# frozen_string_literal: true - -class Projects::MergeRequests::ContextController < Projects::MergeRequests::ApplicationController - before_action :authenticate_user! - - def index - render json: @merge_request.context_diffs - end - - def create - commit_ids = params[:commits] - project = @merge_request.target_project - if commit_ids.is_a?(Array) - commits = commit_ids.map do |commit_id| - project.repository.commit_by(oid: commit_id) - end - MergeRequestContextCommit.create_bulk(@merge_request, project.repository.raw_repository, commits) - render json: { status: :success } - else - render json: :bad_request - end - end - - def destroy - commit_ids = params[:commits] - if commit_ids.is_a?(Array) - commits = commit_ids.map do |commit_id| - project.repository.commit_by(oid: commit_id) - end - MergeRequestContextCommit.delete_bulk(@merge_request, commits) - render json: { status: :success } - else - render json: :bad_request - end - end -end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index cf9951ad7df347..31223bd34b1bd2 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -381,11 +381,12 @@ def handle_merge_request_errors!(errors) commit_ids = params[:commits] merge_request = find_merge_request_with_access(params[:merge_request_iid]) project = merge_request.target_project - commits = commit_ids.map do |commit_id| - commit = project.repository.commit_by(oid: commit_id) - render_api_error!("Commit not found for #{commit_id}", 400) unless commit - commit + commits = project.repository.commits_by(oids: commit_ids) + + if commits.size != commit_ids.size + render_api_error!("One or more context commits' sha is not valid.", 400) end + MergeRequestContextCommit.create_bulk(merge_request, project.repository.raw_repository, commits) present commits, with: Entities::Commit end @@ -400,11 +401,12 @@ def handle_merge_request_errors!(errors) commit_ids = params[:commits] merge_request = find_merge_request_with_access(params[:merge_request_iid]) project = merge_request.target_project - commits = commit_ids.map do |commit_id| - commit = project.repository.commit_by(oid: commit_id) - render_api_error!("Commit not found for #{commit_id}", 400) unless commit - commit + commits = project.repository.commits_by(oids: commit_ids) + + if commits.size != commit_ids.size + render_api_error!("One or more context commits' sha is not valid.", 400) end + MergeRequestContextCommit.delete_bulk(merge_request, commits) present commits, with: Entities::Commit end diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index afa575241a1000..a5a546282644af 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -55,6 +55,9 @@ tree: - merge_request_diff: - :merge_request_diff_commits - :merge_request_diff_files + - merge_request_context: + - :merge_request_context_commits + - :merge_request_context_commit_diff_files - events: - :push_event_payload - :timelogs -- GitLab From 37afdadbbe9c5dc16197f8b9208348951793324d Mon Sep 17 00:00:00 2001 From: Lin Xiangyu Date: Thu, 3 Oct 2019 15:53:44 +0800 Subject: [PATCH 03/38] Fix import_export test --- lib/gitlab/import_export/import_export.yml | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index a5a546282644af..2dcb397e03b80f 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -55,9 +55,7 @@ tree: - merge_request_diff: - :merge_request_diff_commits - :merge_request_diff_files - - merge_request_context: - - :merge_request_context_commits - - :merge_request_context_commit_diff_files + - :merge_request_context_commits - events: - :push_event_payload - :timelogs -- GitLab From 5cfd37bb73b370b394462872bf347295f43470ba Mon Sep 17 00:00:00 2001 From: Lin Xiangyu Date: Thu, 3 Oct 2019 18:48:03 +0800 Subject: [PATCH 04/38] Fix import_export test --- lib/gitlab/import_export/import_export.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 2dcb397e03b80f..75eb62b569ef57 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -56,6 +56,7 @@ tree: - :merge_request_diff_commits - :merge_request_diff_files - :merge_request_context_commits + - :diff_files - events: - :push_event_payload - :timelogs -- GitLab From 325d02f2a9f58d8bce39f5aab689abf04291e924 Mon Sep 17 00:00:00 2001 From: Lin Xiangyu Date: Wed, 23 Oct 2019 12:39:05 +0800 Subject: [PATCH 05/38] M: fix context commits & add fixtures --- app/controllers/concerns/cached_commit.rb | 17 +++++++++++++++++ lib/api/merge_requests.rb | 9 +++++++++ 2 files changed, 26 insertions(+) create mode 100644 app/controllers/concerns/cached_commit.rb diff --git a/app/controllers/concerns/cached_commit.rb b/app/controllers/concerns/cached_commit.rb new file mode 100644 index 00000000000000..183d572874383c --- /dev/null +++ b/app/controllers/concerns/cached_commit.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module CachedCommit + extend ActiveSupport::Concern + + def to_hash + Gitlab::Git::Commit::SERIALIZE_KEYS.each_with_object({}) do |key, hash| + hash[key] = public_send(key) # rubocop:disable GitlabSecurity/PublicSend + end + end + + # We don't save these, because they would need a table or a serialised + # field. They aren't used anywhere, so just pretend the commit has no parents. + def parent_ids + [] + end +end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index 31223bd34b1bd2..dcceadac0f256a 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -380,6 +380,15 @@ def handle_merge_request_errors!(errors) post ':id/merge_requests/:merge_request_iid/context_commits' do commit_ids = params[:commits] merge_request = find_merge_request_with_access(params[:merge_request_iid]) + existing_oids = merge_request.merge_request_context_commits.map { |commit| commit.sha.to_s } + duplicate_oids = existing_oids.select do |existing_oid| + commit_ids.select { |commit_id| existing_oid.start_with?(commit_id) }.count > 0 + end + + if duplicate_oids.count > 0 + render_api_error!("Context commits: #{duplicate_oids} are already created", 400) + end + project = merge_request.target_project commits = project.repository.commits_by(oids: commit_ids) -- GitLab From 4811bd90acfe493fce0ad7795270af1ec3d17348 Mon Sep 17 00:00:00 2001 From: Anwar Sadath Date: Tue, 29 Oct 2019 14:13:04 +0530 Subject: [PATCH 06/38] Fix pipeline failing --- lib/gitlab/import_export/import_export.yml | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml index 75eb62b569ef57..2dcb397e03b80f 100644 --- a/lib/gitlab/import_export/import_export.yml +++ b/lib/gitlab/import_export/import_export.yml @@ -56,7 +56,6 @@ tree: - :merge_request_diff_commits - :merge_request_diff_files - :merge_request_context_commits - - :diff_files - events: - :push_event_payload - :timelogs -- GitLab From 57f509475ee2826ea97f783a25b1f9d7c3048141 Mon Sep 17 00:00:00 2001 From: Anwar Sadath Date: Tue, 17 Mar 2020 17:02:14 +0530 Subject: [PATCH 07/38] Remove revision picker --- app/assets/javascripts/api.js | 34 +++++ .../components/add_review_items_modal.vue | 124 +++++++++++++++++ .../javascripts/diffs/components/app.vue | 10 ++ .../diffs/components/commit_item.vue | 17 +++ .../diffs/components/compare_versions.vue | 91 +++++++++++- .../diffs/components/review_tab_container.vue | 58 ++++++++ app/assets/javascripts/diffs/index.js | 4 + app/assets/javascripts/diffs/store/actions.js | 75 ++++++++++ .../diffs/store/modules/diff_state.js | 9 ++ .../javascripts/diffs/store/mutation_types.js | 16 +++ .../javascripts/diffs/store/mutations.js | 42 ++++++ app/assets/stylesheets/pages/diff.scss | 8 ++ .../merge_requests/diffs_controller.rb | 10 +- .../projects/merge_requests_controller.rb | 5 +- app/helpers/commits_helper.rb | 7 + app/models/merge_request.rb | 12 +- app/serializers/commit_entity.rb | 47 +------ app/serializers/user_entity.rb | 8 +- app/views/projects/commits/_commit.html.haml | 2 +- .../projects/merge_requests/show.html.haml | 4 +- lib/api/entities/commit_with_link.rb | 53 +++++++ lib/api/entities/user_path.rb | 14 ++ lib/api/merge_requests.rb | 2 +- spec/frontend/api_spec.js | 94 +++++++++++++ .../components/add_review_items_modal_spec.js | 129 ++++++++++++++++++ .../components/review_tab_container_spec.js | 51 +++++++ spec/javascripts/diffs/components/app_spec.js | 2 + spec/javascripts/diffs/store/actions_spec.js | 57 ++++++++ .../javascripts/diffs/store/mutations_spec.js | 111 +++++++++++++++ 29 files changed, 1033 insertions(+), 63 deletions(-) create mode 100644 app/assets/javascripts/diffs/components/add_review_items_modal.vue create mode 100644 app/assets/javascripts/diffs/components/review_tab_container.vue create mode 100644 lib/api/entities/commit_with_link.rb create mode 100644 lib/api/entities/user_path.rb create mode 100644 spec/frontend/diffs/components/add_review_items_modal_spec.js create mode 100644 spec/frontend/diffs/components/review_tab_container_spec.js diff --git a/app/assets/javascripts/api.js b/app/assets/javascripts/api.js index dc6ea148047479..f2509445dec362 100644 --- a/app/assets/javascripts/api.js +++ b/app/assets/javascripts/api.js @@ -48,6 +48,9 @@ const Api = { pipelineSinglePath: '/api/:version/projects/:id/pipelines/:pipeline_id', lsifPath: '/api/:version/projects/:id/commits/:commit_id/lsif/info', environmentsPath: '/api/:version/projects/:id/environments', + contextCommitsPath: + '/api/:version/projects/:id/merge_requests/:merge_request_iid/context_commits', + commitsPath: '/api/:version/projects/:id/merge_requests/:merge_request_iid/commits', group(groupId, callback) { const url = Api.buildUrl(Api.groupPath).replace(':id', groupId); @@ -489,6 +492,37 @@ const Api = { return axios.get(url); }, + createContextCommits(id, mergeRequestIid, data) { + const url = Api.buildUrl(this.contextCommitsPath) + .replace(':id', encodeURIComponent(id)) + .replace(':merge_request_iid', mergeRequestIid); + + return axios.post(url, data); + }, + + mergeRequestCommits(id, mergeRequestIid) { + const url = Api.buildUrl(this.commitsPath) + .replace(':id', encodeURIComponent(id)) + .replace(':merge_request_iid', mergeRequestIid); + return axios.get(url); + }, + + allContextCommits(id, mergeRequestIid) { + const url = Api.buildUrl(this.contextCommitsPath) + .replace(':id', encodeURIComponent(id)) + .replace(':merge_request_iid', mergeRequestIid); + + return axios.get(url); + }, + + removeContextCommits(id, mergeRequestIid, data) { + const url = Api.buildUrl(this.contextCommitsPath) + .replace(':id', id) + .replace(':merge_request_iid', mergeRequestIid); + + return axios.delete(url, { data }); + }, + buildUrl(url) { return joinPaths(gon.relative_url_root || '', url.replace(':version', gon.api_version)); }, diff --git a/app/assets/javascripts/diffs/components/add_review_items_modal.vue b/app/assets/javascripts/diffs/components/add_review_items_modal.vue new file mode 100644 index 00000000000000..fcd57747399c42 --- /dev/null +++ b/app/assets/javascripts/diffs/components/add_review_items_modal.vue @@ -0,0 +1,124 @@ + + + diff --git a/app/assets/javascripts/diffs/components/app.vue b/app/assets/javascripts/diffs/components/app.vue index 77cd2afc10617a..54163560782369 100644 --- a/app/assets/javascripts/diffs/components/app.vue +++ b/app/assets/javascripts/diffs/components/app.vue @@ -88,6 +88,14 @@ export default { required: false, default: false, }, + mergeRequestIid: { + type: Number, + required: true, + }, + projectId: { + type: Number, + required: true, + }, }, data() { const treeWidth = @@ -173,6 +181,8 @@ export default { dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, useSingleDiffStyle: this.glFeatures.singleMrDiffView, + mergeRequestIid: this.mergeRequestIid, + projectId: this.projectId, }); if (this.shouldShow) { diff --git a/app/assets/javascripts/diffs/components/commit_item.vue b/app/assets/javascripts/diffs/components/commit_item.vue index cfffccd54ebcec..5095ee2281b4fe 100644 --- a/app/assets/javascripts/diffs/components/commit_item.vue +++ b/app/assets/javascripts/diffs/components/commit_item.vue @@ -28,10 +28,20 @@ export default { CommitPipelineStatus, }, props: { + isSelectable: { + type: Boolean, + required: false, + default: false, + }, commit: { type: Object, required: true, }, + checked: { + type: Boolean, + required: false, + default: false, + }, }, computed: { author() { @@ -65,6 +75,13 @@ export default { diff --git a/app/assets/javascripts/diffs/components/review_tab_container.vue b/app/assets/javascripts/diffs/components/review_tab_container.vue new file mode 100644 index 00000000000000..db07060808c70e --- /dev/null +++ b/app/assets/javascripts/diffs/components/review_tab_container.vue @@ -0,0 +1,58 @@ + + diff --git a/app/assets/javascripts/diffs/index.js b/app/assets/javascripts/diffs/index.js index 375ac80021f13a..66696551e84d9b 100644 --- a/app/assets/javascripts/diffs/index.js +++ b/app/assets/javascripts/diffs/index.js @@ -77,6 +77,8 @@ export default function initDiffsApp(store) { dismissEndpoint: dataset.dismissEndpoint, showSuggestPopover: parseBoolean(dataset.showSuggestPopover), showWhitespaceDefault: parseBoolean(dataset.showWhitespaceDefault), + mergeRequestIid: Number(dataset.mergeRequestIid), + projectId: Number(dataset.projectId), }; }, computed: { @@ -113,6 +115,8 @@ export default function initDiffsApp(store) { dismissEndpoint: this.dismissEndpoint, showSuggestPopover: this.showSuggestPopover, showWhitespaceDefault: this.showWhitespaceDefault, + mergeRequestIid: this.mergeRequestIid, + projectId: this.projectId, }, }); }, diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index bd85105ccb4c71..8b87d0c31c93d7 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -37,6 +37,7 @@ import { DIFFS_PER_PAGE, } from '../constants'; import { diffViewerModes } from '~/ide/constants'; +import Api from '~/api'; export const setBaseConfig = ({ commit }, options) => { const { @@ -47,6 +48,8 @@ export const setBaseConfig = ({ commit }, options) => { dismissEndpoint, showSuggestPopover, useSingleDiffStyle, + mergeRequestIid, + projectId, } = options; commit(types.SET_BASE_CONFIG, { endpoint, @@ -56,6 +59,8 @@ export const setBaseConfig = ({ commit }, options) => { dismissEndpoint, showSuggestPopover, useSingleDiffStyle, + mergeRequestIid, + projectId, }); }; @@ -606,5 +611,75 @@ export const setSuggestPopoverDismissed = ({ commit, state }) => createFlash(s__('MergeRequest|Error dismissing suggestion popover. Please try again.')); }); +export const setTabIndex = ({ commit }, tabIndex) => commit(types.SET_TABINDEX, tabIndex); + +export const searchCommits = ({ commit, state }, searchText) => { + commit(types.FETCH_COMMITS); + const url = `${state.projectPath}/-/merge_requests/${state.mergeRequestIid}/context_commits.json`; + + let params = {}; + if (searchText) { + params = { + params: { + search: searchText, + per_page: 40, + }, + }; + } + + return axios + .get(url, params) + .then(({ data }) => { + commit(types.FETCH_COMMITS_SUCCESS, data); + }) + .catch(() => { + commit(types.FETCH_COMMITS_ERROR); + }); +}; + +export const setSelectedCommits = ({ commit }, selectedCommits) => + commit(types.SET_SELECTED_COMMITS, selectedCommits); + +export const createContextCommits = ({ state }) => + Api.createContextCommits(state.projectId, state.mergeRequestIid, { + commits: state.selectedCommits, + }) + .then(() => { + window.location.reload(); + }) + .catch(() => { + createFlash(s__('ContextCommits|Failed to create context commits. Please try again.')); + }); + +export const fetchMergeRequestCommits = ({ commit, state }) => + Api.mergeRequestCommits(state.projectId, state.mergeRequestIid).then(({ data }) => { + commit(types.SET_MERGE_REQUEST_COMMITS, data); + }); + +export const fetchContextCommits = ({ commit, state }) => { + commit(types.FETCH_CONTEXT_COMMITS); + return Api.allContextCommits(state.projectId, state.mergeRequestIid) + .then(({ data }) => { + commit(types.FETCH_CONTEXT_COMMITS_SUCCESS, data); + }) + .catch(() => { + commit(types.FETCH_CONTEXT_COMMITS_ERROR); + }); +}; + +export const setSelectedContextCommits = ({ commit }, selectedContextCommits) => + commit(types.SET_SELECTED_CONTEXT_COMMITS, selectedContextCommits); + +export const removeContextCommits = ({ state }) => + Api.removeContextCommits(state.projectId, state.mergeRequestIid, { + commits: state.selectedContextCommits, + }) + .then(() => { + window.location.reload(); + }) + .catch(() => { + createFlash(s__('ContextCommits|Failed to delete context commits. Please try again.')); + }); + // prevent babel-plugin-rewire from generating an invalid default during karma tests export default () => {}; diff --git a/app/assets/javascripts/diffs/store/modules/diff_state.js b/app/assets/javascripts/diffs/store/modules/diff_state.js index 011cd24500a32c..33f2aaedfd316e 100644 --- a/app/assets/javascripts/diffs/store/modules/diff_state.js +++ b/app/assets/javascripts/diffs/store/modules/diff_state.js @@ -33,4 +33,13 @@ export default () => ({ dismissEndpoint: '', showSuggestPopover: true, useSingleDiffStyle: false, + tabIndex: 0, + isLoadingCommits: false, + commits: [], + commitsLoadingError: false, + selectedCommits: [], + mergeRequestCommits: [], + contextCommits: [], + contextCommitsLoadingError: false, + selectedContextCommits: [], }); diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index 2097c8d365540f..94f277fa6f9822 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -40,3 +40,19 @@ export const TOGGLE_DIFF_FILE_RENDERING_MORE = 'TOGGLE_DIFF_FILE_RENDERING_MORE' export const SET_SHOW_SUGGEST_POPOVER = 'SET_SHOW_SUGGEST_POPOVER'; export const TOGGLE_LINE_DISCUSSIONS = 'TOGGLE_LINE_DISCUSSIONS'; + +export const SET_TABINDEX = 'SET_TABINDEX'; + +export const FETCH_COMMITS = 'FETCH_COMMITS'; +export const FETCH_COMMITS_SUCCESS = 'FETCH_COMMITS_SUCCESS'; +export const FETCH_COMMITS_ERROR = 'FETCH_COMMITS_ERROR'; + +export const SET_SELECTED_COMMITS = 'SET_SELECTED_COMMITS'; + +export const SET_MERGE_REQUEST_COMMITS = 'SET_MERGE_REQUEST_COMMITS'; + +export const FETCH_CONTEXT_COMMITS = 'FETCH_CONTEXT_COMMITS'; +export const FETCH_CONTEXT_COMMITS_SUCCESS = 'FETCH_CONTEXT_COMMITS_SUCCESS'; +export const FETCH_CONTEXT_COMMITS_ERROR = 'FETCH_CONTEXT_COMMITS_ERROR'; + +export const SET_SELECTED_CONTEXT_COMMITS = 'SET_SELECTED_CONTEXT_COMMITS'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 086a7872a5d37a..e62fd74490a072 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -20,6 +20,8 @@ export default { dismissEndpoint, showSuggestPopover, useSingleDiffStyle, + mergeRequestIid, + projectId, } = options; Object.assign(state, { endpoint, @@ -29,6 +31,8 @@ export default { dismissEndpoint, showSuggestPopover, useSingleDiffStyle, + mergeRequestIid, + projectId, }); }, @@ -368,4 +372,42 @@ export default { [types.SET_SHOW_SUGGEST_POPOVER](state) { state.showSuggestPopover = false; }, + [types.SET_TABINDEX](state, tabIndex) { + state.tabIndex = tabIndex; + }, + [types.FETCH_COMMITS](state) { + state.isLoadingCommits = true; + state.commitsLoadingError = false; + }, + [types.FETCH_COMMITS_SUCCESS](state, commits) { + state.commits = commits; + state.isLoadingCommits = false; + state.commitsLoadingError = false; + }, + [types.FETCH_COMMITS_ERROR](state) { + state.commitsLoadingError = true; + state.isLoadingCommits = false; + }, + [types.SET_SELECTED_COMMITS](state, selectedCommits) { + state.selectedCommits = selectedCommits; + }, + [types.SET_MERGE_REQUEST_COMMITS](state, mergeRequestCommits) { + state.mergeRequestCommits = mergeRequestCommits; + }, + [types.FETCH_CONTEXT_COMMITS](state) { + state.isLoadingCommits = true; + state.contextCommitsLoadingError = false; + }, + [types.FETCH_CONTEXT_COMMITS_SUCCESS](state, contextCommits) { + state.contextCommits = contextCommits; + state.isLoadingCommits = false; + state.contextCommitsLoadingError = false; + }, + [types.FETCH_CONTEXT_COMMITS_ERROR](state) { + state.contextCommitsLoadingError = true; + state.isLoadingCommits = false; + }, + [types.SET_SELECTED_CONTEXT_COMMITS](state, selectedContextCommits) { + state.selectedContextCommits = selectedContextCommits; + }, }; diff --git a/app/assets/stylesheets/pages/diff.scss b/app/assets/stylesheets/pages/diff.scss index 24c6fec064a8b4..e917420e111eeb 100644 --- a/app/assets/stylesheets/pages/diff.scss +++ b/app/assets/stylesheets/pages/diff.scss @@ -1183,3 +1183,11 @@ table.code { display: none; } } + +.context-commits-container { + ul.content-list { + > li:last-child { + border-bottom: 1px solid $gray-darker; + } + } +} diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 953b2ffeb0b929..6bdfbb924a07f7 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -38,8 +38,14 @@ def diffs_batch def diffs_metadata diffs = @compare.diffs(diff_options) + options = additional_attributes.merge(diff_view: diff_view) + + if @merge_request.project.context_commits_enabled? + options[:context_commits] = @merge_request.recent_context_commits + end + render json: DiffsMetadataSerializer.new(project: @merge_request.project) - .represent(diffs, additional_attributes) + .represent(diffs, options) end private @@ -65,7 +71,7 @@ def render_diffs options = additional_attributes.merge(diff_view: diff_view) if @merge_request.project.context_commits_enabled? - options[:context_commits] = @merge_request.context_commits + options[:context_commits] = @merge_request.recent_context_commits end render json: DiffsSerializer.new(request).represent(diffs, options) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index af185887a8caae..4ec1cf34404b4b 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -27,6 +27,7 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo before_action do push_frontend_feature_flag(:vue_issuable_sidebar, @project.group) + push_frontend_feature_flag(:context_commits, default_enabled: true) end around_action :allow_gitaly_ref_name_caching, only: [:index, :show, :discussions] @@ -59,7 +60,7 @@ def show @note = @project.notes.new(noteable: @merge_request) @noteable = @merge_request - @commits_count = @merge_request.commits_count + @commits_count = @merge_request.commits_count + @merge_request.context_commits_count @issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar') @current_user_data = UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json @show_whitespace_default = current_user.nil? || current_user.show_whitespace_in_diffs @@ -94,7 +95,7 @@ def commits # or from cache if already merged @commits = set_commits_for_rendering( - @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch), + @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch) + @merge_request.recent_context_commits, commits_count: @merge_request.commits_count ) diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index ace8bae03acbeb..ea43662be4faf2 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -122,6 +122,13 @@ def commit_signature_badge_classes(additional_classes) %w(btn gpg-status-box) + Array(additional_classes) end + def context_commit?(merge_request, commit_id) + return false if merge_request.nil? + + mr_context_commits_sha = merge_request.context_commits.map(&:id) + mr_context_commits_sha.include? commit_id + end + protected # Private: Returns a link to a person. If the person has a matching user and diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index cf49f6c8cef8c8..e0e62dc96c9a0a 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -410,8 +410,16 @@ def to_reference(from = nil, full: false) "#{project.to_reference_base(from, full: full)}#{reference}" end - def context_commits - @context_commits ||= merge_request_context_commits.map(&:to_commit) + def context_commits(limit: nil) + @context_commits ||= merge_request_context_commits.limit(limit).map(&:to_commit) + end + + def recent_context_commits + context_commits(limit: MergeRequestDiff::COMMITS_SAFE_SIZE) + end + + def context_commits_count + context_commits.count end def commits(limit: nil) diff --git a/app/serializers/commit_entity.rb b/app/serializers/commit_entity.rb index ae3f1c6bbf5e97..c9c4981682a462 100644 --- a/app/serializers/commit_entity.rb +++ b/app/serializers/commit_entity.rb @@ -1,49 +1,4 @@ # frozen_string_literal: true -class CommitEntity < API::Entities::Commit - include MarkupHelper - include RequestAwareEntity - - expose :author, using: UserEntity - - expose :author_gravatar_url do |commit| - GravatarService.new.execute(commit.author_email) # rubocop: disable CodeReuse/ServiceClass - end - - expose :commit_url do |commit, options| - project_commit_url(request.project, commit, params: options.fetch(:commit_url_params, {})) - end - - expose :commit_path do |commit, options| - project_commit_path(request.project, commit, params: options.fetch(:commit_url_params, {})) - end - - expose :description_html, if: { type: :full } do |commit| - markdown_field(commit, :description) - end - - expose :title_html, if: { type: :full } do |commit| - markdown_field(commit, :title) - end - - expose :signature_html, if: { type: :full } do |commit| - render('projects/commit/_signature', signature: commit.signature) if commit.has_signature? - end - - expose :pipeline_status_path, if: { type: :full } do |commit, options| - pipeline_ref = options[:pipeline_ref] - pipeline_project = options[:pipeline_project] || commit.project - next unless pipeline_ref && pipeline_project - - pipeline = commit.latest_pipeline_for_project(pipeline_ref, pipeline_project) - next unless pipeline&.status - - pipelines_project_commit_path(pipeline_project, commit.id, ref: pipeline_ref) - end - - def render(*args) - return unless request.respond_to?(:render) && request.render.respond_to?(:call) - - request.render.call(*args) - end +class CommitEntity < API::Entities::CommitWithLink end diff --git a/app/serializers/user_entity.rb b/app/serializers/user_entity.rb index 656900bb8af5ae..8909ae8df2cc4c 100644 --- a/app/serializers/user_entity.rb +++ b/app/serializers/user_entity.rb @@ -1,10 +1,4 @@ # frozen_string_literal: true -class UserEntity < API::Entities::UserBasic - include RequestAwareEntity - include UserStatusTooltip - - expose :path do |user| - user_path(user) - end +class UserEntity < API::Entities::UserPath end diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index 8b659034fe6454..a9180f3e8a21f9 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -54,7 +54,7 @@ .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: ref) } } .commit-sha-group.d-none.d-sm-flex - .label.label-monospace.monospace + .label.label-monospace.monospace{ class: ("text-danger" if context_commit?(merge_request, commit.id)) } = commit.short_id = clipboard_button(text: commit.id, title: _("Copy commit SHA"), class: "btn btn-default", container: "body") = link_to_browse_code(project, commit) diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml index d65c874f2453a2..0ce6c2a171e0e1 100644 --- a/app/views/projects/merge_requests/show.html.haml +++ b/app/views/projects/merge_requests/show.html.haml @@ -85,7 +85,9 @@ is_fluid_layout: fluid_layout.to_s, dismiss_endpoint: user_callouts_path, show_suggest_popover: show_suggest_popover?.to_s, - show_whitespace_default: @show_whitespace_default.to_s } + show_whitespace_default: @show_whitespace_default.to_s, + merge_request_iid: @merge_request.iid, + project_id: @merge_request.project.id } .mr-loading-status .loading.hide diff --git a/lib/api/entities/commit_with_link.rb b/lib/api/entities/commit_with_link.rb new file mode 100644 index 00000000000000..31a9efed9bc77e --- /dev/null +++ b/lib/api/entities/commit_with_link.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module API + module Entities + class CommitWithLink < Commit + include MarkupHelper + include RequestAwareEntity + + expose :author, using: Entities::UserPath + + expose :author_gravatar_url do |commit| + GravatarService.new.execute(commit.author_email) + end + + expose :commit_url do |commit, options| + project_commit_url(request.project, commit, params: options.fetch(:commit_url_params, {})) + end + + expose :commit_path do |commit, options| + project_commit_path(request.project, commit, params: options.fetch(:commit_url_params, {})) + end + + expose :description_html, if: { type: :full } do |commit| + markdown_field(commit, :description) + end + + expose :title_html, if: { type: :full } do |commit| + markdown_field(commit, :title) + end + + expose :signature_html, if: { type: :full } do |commit| + render('projects/commit/_signature', signature: commit.signature) if commit.has_signature? + end + + expose :pipeline_status_path, if: { type: :full } do |commit, options| + pipeline_ref = options[:pipeline_ref] + pipeline_project = options[:pipeline_project] || commit.project + next unless pipeline_ref && pipeline_project + + pipeline = commit.latest_pipeline_for_project(pipeline_ref, pipeline_project) + next unless pipeline&.status + + pipelines_project_commit_path(pipeline_project, commit.id, ref: pipeline_ref) + end + + def render(*args) + return unless request.respond_to?(:render) && request.render.respond_to?(:call) + + request.render.call(*args) + end + end + end +end diff --git a/lib/api/entities/user_path.rb b/lib/api/entities/user_path.rb new file mode 100644 index 00000000000000..7d922b39654646 --- /dev/null +++ b/lib/api/entities/user_path.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module API + module Entities + class UserPath < UserBasic + include RequestAwareEntity + include UserStatusTooltip + + expose :path do |user| + user_path(user) + end + end + end +end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index dcceadac0f256a..bc75293c177a76 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -306,7 +306,7 @@ def handle_merge_request_errors!(errors) context_commits = paginate(merge_request.merge_request_context_commits).map(&:to_commit) - present context_commits, with: Entities::Commit + present context_commits, with: Entities::CommitWithLink, type: :full, request: merge_request end params do diff --git a/spec/frontend/api_spec.js b/spec/frontend/api_spec.js index c0126b2330d8de..19ef43b3df7c81 100644 --- a/spec/frontend/api_spec.js +++ b/spec/frontend/api_spec.js @@ -570,4 +570,98 @@ describe('Api', () => { .catch(done.fail); }); }); + + describe('createContextCommits', () => { + it('creates a new context commit', done => { + const projectPath = 'abc'; + const mergeRequestId = '123456'; + const commitsData = ['abcdefg']; + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/${projectPath}/merge_requests/${mergeRequestId}/context_commits`; + const expectedData = { + commits: commitsData, + }; + + jest.spyOn(axios, 'post'); + + mock.onPost(expectedUrl).replyOnce(200, [ + { + id: 'abcdefghijklmnop', + short_id: 'abcdefg', + title: 'Dummy commit', + }, + ]); + + Api.createContextCommits(projectPath, mergeRequestId, expectedData) + .then(({ data }) => { + expect(data[0].title).toBe('Dummy commit'); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('mergeRequestCommits', () => { + it('gets all commits in a merge request', done => { + const projectPath = 'abc'; + const mergeRequestId = '123456'; + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/${projectPath}/merge_requests/${mergeRequestId}/commits`; + + jest.spyOn(axios, 'get'); + + mock + .onGet(expectedUrl) + .replyOnce(200, [{ id: 'abcdef', short_id: 'abcdefghi', title: 'Dummy commit title' }]); + + Api.mergeRequestCommits(projectPath, mergeRequestId) + .then(({ data }) => { + expect(data[0].title).toBe('Dummy commit title'); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('allContextCommits', () => { + it('gets all context commits', done => { + const projectPath = 'abc'; + const mergeRequestId = '123456'; + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/${projectPath}/merge_requests/${mergeRequestId}/context_commits`; + + jest.spyOn(axios, 'get'); + + mock + .onGet(expectedUrl) + .replyOnce(200, [{ id: 'abcdef', short_id: 'abcdefghi', title: 'Dummy commit title' }]); + + Api.allContextCommits(projectPath, mergeRequestId) + .then(({ data }) => { + expect(data[0].title).toBe('Dummy commit title'); + }) + .then(done) + .catch(done.fail); + }); + }); + + describe('removeContextCommits', () => { + it('removes context commits', done => { + const projectPath = 'abc'; + const mergeRequestId = '123456'; + const commitsData = ['abcdefg']; + const expectedUrl = `${dummyUrlRoot}/api/${dummyApiVersion}/projects/${projectPath}/merge_requests/${mergeRequestId}/context_commits`; + const expectedData = { + commits: commitsData, + }; + + jest.spyOn(axios, 'delete'); + + mock.onDelete(expectedUrl).replyOnce(204); + + Api.removeContextCommits(projectPath, mergeRequestId, expectedData) + .then(() => { + expect(axios.delete).toHaveBeenCalledWith(expectedUrl, { data: expectedData }); + }) + .then(done) + .catch(done.fail); + }); + }); }); diff --git a/spec/frontend/diffs/components/add_review_items_modal_spec.js b/spec/frontend/diffs/components/add_review_items_modal_spec.js new file mode 100644 index 00000000000000..05a41fa8a8ef73 --- /dev/null +++ b/spec/frontend/diffs/components/add_review_items_modal_spec.js @@ -0,0 +1,129 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlModal, GlTab } from '@gitlab/ui'; +import AddReviewItemsModal from '~/diffs/components/add_review_items_modal.vue'; +import getDiffWithCommit from '../mock_data/diff_with_commit'; + +describe('AddReviewItemsModal', () => { + let wrapper; + const { commit } = getDiffWithCommit(); + + const createWrapper = (props = {}) => { + wrapper = shallowMount(AddReviewItemsModal, { + propsData: { + tabIndex: 0, + isLoadingCommits: false, + commits: [], + commitsLoadingError: false, + selectedCommits: [], + contextCommits: [], + contextCommitsLoadingError: false, + selectedContextCommits: [], + ...props, + }, + }); + }; + + beforeEach(() => { + createWrapper(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('renders modal', () => { + const modal = wrapper.find(GlModal); + expect(modal.exists()).toBe(true); + expect(modal.attributes('title')).toEqual('Add review items'); + }); + + it('renders modal with 2 tabs', () => { + const modal = wrapper.find(GlModal); + expect(modal.findAll(GlTab).length).toBe(2); + expect( + modal + .findAll(GlTab) + .at(0) + .attributes('title'), + ).toBe('Commits'); + expect( + modal + .findAll(GlTab) + .at(1) + .attributes('title'), + ).toBe('Context Commits'); + }); + + it('emits "onSearchChange" when user starts entering text in input box', () => { + const modal = wrapper.find(GlModal); + const input = modal.find('input'); + input.setValue('abc'); + input.trigger('change'); + + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.emittedByOrder()).toEqual([ + { + name: 'onSearchChange', + args: ['abc'], + }, + ]); + }); + }); + + describe('when in first tab, renders a modal with', () => { + it('an ok button labeled "Add Commits"', () => { + expect(wrapper.find(GlModal).attributes('ok-title')).toEqual('Add Commits'); + }); + + it('an ok button which when clicked emits "createContextCommits" event', () => { + wrapper.find(GlModal).vm.$emit('ok'); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.emitted('createContextCommits')).toBeTruthy(); + }); + }); + + it('disabled ok button when no row is selected', () => { + expect(wrapper.find(GlModal).attributes('ok-disabled')).toBe('true'); + }); + + it('enabled ok button when atleast one row is selected', () => { + createWrapper({ selectedCommits: [commit] }); + expect(wrapper.find(GlModal).attributes('ok-disabled')).toBeFalsy(); + }); + + it('disabled ok button in second tab, when row is selected in first tab', () => { + createWrapper({ tabIndex: 1, selectedCommits: [commit] }); + expect(wrapper.find(GlModal).attributes('ok-disabled')).toBe('true'); + }); + }); + + describe('when in second tab, renders a modal with ', () => { + it('an ok button labeled "Remove Commits"', () => { + createWrapper({ tabIndex: 1 }); + expect(wrapper.find(GlModal).attributes('ok-title')).toEqual('Remove Commits'); + }); + + it('an ok button which when clicked emits "removeContextCommits" event', () => { + createWrapper({ tabIndex: 1 }); + wrapper.find(GlModal).vm.$emit('ok'); + return wrapper.vm.$nextTick().then(() => { + expect(wrapper.emitted('removeContextCommits')).toBeTruthy(); + }); + }); + + it('disabled ok button when no row is selected', () => { + createWrapper({ tabIndex: 1 }); + expect(wrapper.find(GlModal).attributes('ok-disabled')).toBe('true'); + }); + + it('enabled ok button when atleast one row is selected', () => { + createWrapper({ tabIndex: 1, selectedContextCommits: [commit] }); + expect(wrapper.find(GlModal).attributes('ok-disabled')).toBeFalsy(); + }); + + it('disabled ok button in first tab, when row is selected in second tab', () => { + createWrapper({ selectedContextCommits: [commit] }); + expect(wrapper.find(GlModal).attributes('ok-disabled')).toBe('true'); + }); + }); +}); diff --git a/spec/frontend/diffs/components/review_tab_container_spec.js b/spec/frontend/diffs/components/review_tab_container_spec.js new file mode 100644 index 00000000000000..6d35a6a931ef5e --- /dev/null +++ b/spec/frontend/diffs/components/review_tab_container_spec.js @@ -0,0 +1,51 @@ +import { shallowMount } from '@vue/test-utils'; +import { GlLoadingIcon } from '@gitlab/ui'; +import ReviewTabContainer from '~/diffs/components/review_tab_container.vue'; +import CommitItem from '~/diffs/components/commit_item.vue'; +import getDiffWithCommit from '../mock_data/diff_with_commit'; + +describe('ReviewTabContainer', () => { + let wrapper; + const { commit } = getDiffWithCommit(); + + const createWrapper = (props = {}) => { + wrapper = shallowMount(ReviewTabContainer, { + propsData: { + tab: 'commits', + isLoading: false, + loadingError: false, + loadingFailedText: 'Failed to load commits', + commits: [], + selectedRow: [], + ...props, + }, + }); + }; + + beforeEach(() => { + createWrapper(); + }); + + afterEach(() => { + wrapper.destroy(); + }); + + it('shows loading icon when commits are being loaded', () => { + createWrapper({ isLoading: true }); + expect(wrapper.find(GlLoadingIcon).exists()).toBe(true); + }); + + it('shows loading error text when API call fails', () => { + createWrapper({ loadingError: true }); + expect(wrapper.text()).toContain('Failed to load commits'); + }); + + it('shows "No commits present here" when commits are not present', () => { + expect(wrapper.text()).toContain('No commits present here'); + }); + + it('renders all passed commits as list', () => { + createWrapper({ commits: [commit] }); + expect(wrapper.findAll(CommitItem).length).toBe(1); + }); +}); diff --git a/spec/javascripts/diffs/components/app_spec.js b/spec/javascripts/diffs/components/app_spec.js index 5f97182489e076..ac23a0ea51b3e2 100644 --- a/spec/javascripts/diffs/components/app_spec.js +++ b/spec/javascripts/diffs/components/app_spec.js @@ -42,6 +42,8 @@ describe('diffs/components/app', () => { changesEmptyStateIllustration: '', dismissEndpoint: '', showSuggestPopover: true, + mergeRequestIid: 1, + projectId: 1, ...props, }, store, diff --git a/spec/javascripts/diffs/store/actions_spec.js b/spec/javascripts/diffs/store/actions_spec.js index ff17d8ec158023..840e86544389aa 100644 --- a/spec/javascripts/diffs/store/actions_spec.js +++ b/spec/javascripts/diffs/store/actions_spec.js @@ -42,6 +42,9 @@ import actions, { setFileCollapsed, setExpandedDiffLines, setSuggestPopoverDismissed, + setTabIndex, + setSelectedCommits, + setSelectedContextCommits, } from '~/diffs/store/actions'; import eventHub from '~/notes/event_hub'; import * as types from '~/diffs/store/mutation_types'; @@ -77,6 +80,8 @@ describe('DiffsStoreActions', () => { const dismissEndpoint = '/-/user_callouts'; const showSuggestPopover = false; const useSingleDiffStyle = false; + const mergeRequestIid = 1; + const projectId = 1; testAction( setBaseConfig, @@ -88,6 +93,8 @@ describe('DiffsStoreActions', () => { dismissEndpoint, showSuggestPopover, useSingleDiffStyle, + mergeRequestIid, + projectId, }, { endpoint: '', @@ -97,6 +104,8 @@ describe('DiffsStoreActions', () => { dismissEndpoint: '', showSuggestPopover: true, useSingleDiffStyle: true, + mergeRequestIid, + projectId, }, [ { @@ -109,6 +118,8 @@ describe('DiffsStoreActions', () => { dismissEndpoint, showSuggestPopover, useSingleDiffStyle, + mergeRequestIid, + projectId, }, }, ], @@ -1323,4 +1334,50 @@ describe('DiffsStoreActions', () => { ); }); }); + + describe('setTabIndex', () => { + it('commits SET_TABINDEX', done => { + testAction( + setTabIndex, + { tabIndex: 1 }, + { tabIndex: 0 }, + [{ type: types.SET_TABINDEX, payload: { tabIndex: 1 } }], + [], + done, + ); + }); + }); + + describe('setSelectedCommits', () => { + it('commits SET_SELECTED_COMMITS', done => { + const dummyCommit = { id: 1, title: 'dummy commit' }; + testAction( + setSelectedCommits, + { selectedCommits: [dummyCommit] }, + { selectedCommits: [] }, + [{ type: types.SET_SELECTED_COMMITS, payload: { selectedCommits: [dummyCommit] } }], + [], + done, + ); + }); + }); + + describe('setSelectedContextCommits', () => { + it('commits SET_SELECTED_CONTEXT_COMMITS', done => { + const dummyCommit = { id: 1, title: 'dummy commit' }; + testAction( + setSelectedContextCommits, + { selectedContextCommits: [dummyCommit] }, + { selectedContextCommits: [] }, + [ + { + type: types.SET_SELECTED_CONTEXT_COMMITS, + payload: { selectedContextCommits: [dummyCommit] }, + }, + ], + [], + done, + ); + }); + }); }); diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index ffe5d89e615ef6..1081b273de7f50 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -951,4 +951,115 @@ describe('DiffsStoreMutations', () => { expect(state.showSuggestPopover).toBe(false); }); }); + + describe('SET_TABINDEX', () => { + it('sets tabIndex to specific index', () => { + const state = { tabIndex: 0 }; + + mutations[types.SET_TABINDEX](state, 1); + + expect(state.tabIndex).toBe(1); + }); + }); + + describe('FETCH_COMMITS', () => { + it('sets isLoadingCommits to true', () => { + const state = { isLoadingCommits: false }; + + mutations[types.FETCH_COMMITS](state); + + expect(state.isLoadingCommits).toBe(true); + }); + }); + + describe('FETCH_COMMITS_SUCCESS', () => { + it('sets isLoadingCommits to false and sets commits', () => { + const state = { isLoadingCommits: true, commits: [] }; + const commitsMock = [{ id: 'abcdefghijkl', short_id: 'abcde', title: 'Dummy Commit' }]; + + mutations[types.FETCH_COMMITS_SUCCESS](state, commitsMock); + + expect(state.isLoadingCommits).toBe(false); + expect(state.commits[0].id).toBe('abcdefghijkl'); + expect(state.commits[0].short_id).toBe('abcde'); + expect(state.commits[0].title).toBe('Dummy Commit'); + }); + }); + + describe('FETCH_COMMITS_ERROR', () => { + it('sets commitsLoadingError to true', () => { + const state = { commitsLoadingError: false }; + + mutations[types.FETCH_COMMITS_ERROR](state); + + expect(state.commitsLoadingError).toBe(true); + }); + }); + + describe('SET_SELECTED_COMMITS', () => { + it('sets selectedCommits to specified value', () => { + const state = { selectedCommits: [] }; + + mutations[types.SET_SELECTED_COMMITS](state, ['abcdef']); + + expect(state.selectedCommits[0]).toBe('abcdef'); + }); + }); + + describe('SET_MERGE_REQUEST_COMMITS', () => { + it('sets mergeRequestCommits to specified value', () => { + const state = { mergeRequestCommits: [] }; + const commitsMock = [{ id: 'abcdefghijkl', short_id: 'abcde', title: 'Dummy Commit' }]; + + mutations[types.SET_MERGE_REQUEST_COMMITS](state, commitsMock); + + expect(state.mergeRequestCommits[0].id).toBe('abcdefghijkl'); + expect(state.mergeRequestCommits[0].short_id).toBe('abcde'); + expect(state.mergeRequestCommits[0].title).toBe('Dummy Commit'); + }); + }); + + describe('FETCH_CONTEXT_COMMITS', () => { + it('sets isLoadingCommits to true', () => { + const state = { isLoadingCommits: false }; + + mutations[types.FETCH_CONTEXT_COMMITS](state); + + expect(state.isLoadingCommits).toBe(true); + }); + }); + + describe('FETCH_CONTEXT_COMMITS_SUCCESS', () => { + it('sets isLoadingCommits to false and sets contextCommits to specified value', () => { + const state = { isLoadingCommits: true, contextCommits: [] }; + const commitsMock = [{ id: 'abcdefghijkl', short_id: 'abcde', title: 'Dummy Commit' }]; + + mutations[types.FETCH_CONTEXT_COMMITS_SUCCESS](state, commitsMock); + + expect(state.isLoadingCommits).toBe(false); + expect(state.contextCommits[0].id).toBe('abcdefghijkl'); + expect(state.contextCommits[0].short_id).toBe('abcde'); + expect(state.contextCommits[0].title).toBe('Dummy Commit'); + }); + }); + + describe('FETCH_CONTEXT_COMMITS_ERROR', () => { + it('sets contextCommitsLoadingError to true', () => { + const state = { contextCommitsLoadingError: false }; + + mutations[types.FETCH_CONTEXT_COMMITS_ERROR](state); + + expect(state.contextCommitsLoadingError).toBe(true); + }); + }); + + describe('SET_SELECTED_CONTEXT_COMMITS', () => { + it('sets selectedContextCommits to given value', () => { + const state = { selectedContextCommits: [] }; + + mutations[types.SET_SELECTED_CONTEXT_COMMITS](state, ['abcdef']); + + expect(state.selectedContextCommits[0]).toBe('abcdef'); + }); + }); }); -- GitLab From 54967515c9198b45deef0e5349b844aae9d6b06a Mon Sep 17 00:00:00 2001 From: Anwar Sadath Date: Fri, 20 Mar 2020 22:26:42 +0530 Subject: [PATCH 08/38] Remove irreleavant code --- app/controllers/concerns/cached_commit.rb | 17 - .../merge_requests/diffs_controller.rb | 10 +- .../projects/merge_requests_controller.rb | 4 +- app/helpers/commits_helper.rb | 7 - app/models/merge_request.rb | 18 +- app/serializers/commit_entity.rb | 49 ++- app/serializers/user_entity.rb | 8 +- app/views/projects/commits/_commit.html.haml | 2 +- lib/api/entities/commit_with_link.rb | 53 --- lib/api/entities/user_path.rb | 14 - lib/api/merge_requests.rb | 60 +-- lib/gitlab/import_export/import_export.yml | 390 ------------------ 12 files changed, 62 insertions(+), 570 deletions(-) delete mode 100644 app/controllers/concerns/cached_commit.rb delete mode 100644 lib/api/entities/commit_with_link.rb delete mode 100644 lib/api/entities/user_path.rb delete mode 100644 lib/gitlab/import_export/import_export.yml diff --git a/app/controllers/concerns/cached_commit.rb b/app/controllers/concerns/cached_commit.rb deleted file mode 100644 index 183d572874383c..00000000000000 --- a/app/controllers/concerns/cached_commit.rb +++ /dev/null @@ -1,17 +0,0 @@ -# frozen_string_literal: true - -module CachedCommit - extend ActiveSupport::Concern - - def to_hash - Gitlab::Git::Commit::SERIALIZE_KEYS.each_with_object({}) do |key, hash| - hash[key] = public_send(key) # rubocop:disable GitlabSecurity/PublicSend - end - end - - # We don't save these, because they would need a table or a serialised - # field. They aren't used anywhere, so just pretend the commit has no parents. - def parent_ids - [] - end -end diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 6bdfbb924a07f7..953b2ffeb0b929 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -38,14 +38,8 @@ def diffs_batch def diffs_metadata diffs = @compare.diffs(diff_options) - options = additional_attributes.merge(diff_view: diff_view) - - if @merge_request.project.context_commits_enabled? - options[:context_commits] = @merge_request.recent_context_commits - end - render json: DiffsMetadataSerializer.new(project: @merge_request.project) - .represent(diffs, options) + .represent(diffs, additional_attributes) end private @@ -71,7 +65,7 @@ def render_diffs options = additional_attributes.merge(diff_view: diff_view) if @merge_request.project.context_commits_enabled? - options[:context_commits] = @merge_request.recent_context_commits + options[:context_commits] = @merge_request.context_commits end render json: DiffsSerializer.new(request).represent(diffs, options) diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 940692e60254c3..c9e5e092c76f39 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -60,7 +60,7 @@ def show @note = @project.notes.new(noteable: @merge_request) @noteable = @merge_request - @commits_count = @merge_request.commits_count + @merge_request.context_commits_count + @commits_count = @merge_request.commits_count @issuable_sidebar = serializer.represent(@merge_request, serializer: 'sidebar') @current_user_data = UserSerializer.new(project: @project).represent(current_user, {}, MergeRequestUserEntity).to_json @show_whitespace_default = current_user.nil? || current_user.show_whitespace_in_diffs @@ -96,7 +96,7 @@ def commits # or from cache if already merged @commits = set_commits_for_rendering( - @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch) + @merge_request.recent_context_commits, + @merge_request.recent_commits.with_latest_pipeline(@merge_request.source_branch), commits_count: @merge_request.commits_count ) diff --git a/app/helpers/commits_helper.rb b/app/helpers/commits_helper.rb index ea43662be4faf2..ace8bae03acbeb 100644 --- a/app/helpers/commits_helper.rb +++ b/app/helpers/commits_helper.rb @@ -122,13 +122,6 @@ def commit_signature_badge_classes(additional_classes) %w(btn gpg-status-box) + Array(additional_classes) end - def context_commit?(merge_request, commit_id) - return false if merge_request.nil? - - mr_context_commits_sha = merge_request.context_commits.map(&:id) - mr_context_commits_sha.include? commit_id - end - protected # Private: Returns a link to a person. If the person has a matching user and diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb index 0cade6cefdb67a..412d0fa4ec84a1 100644 --- a/app/models/merge_request.rb +++ b/app/models/merge_request.rb @@ -410,16 +410,8 @@ def to_reference(from = nil, full: false) "#{project.to_reference_base(from, full: full)}#{reference}" end - def context_commits(limit: nil) - @context_commits ||= merge_request_context_commits.limit(limit).map(&:to_commit) - end - - def recent_context_commits - context_commits(limit: MergeRequestDiff::COMMITS_SAFE_SIZE) - end - - def context_commits_count - context_commits.count + def context_commits + @context_commits ||= merge_request_context_commits.map(&:to_commit) end def commits(limit: nil) @@ -557,12 +549,6 @@ def discussions_diffs end end - def context_diffs - MergeRequestContextCommitDiffFile.where( - merge_request_context_commit_id: self.merge_request_context_commits - ) - end - def diff_size # Calling `merge_request_diff.diffs.real_size` will also perform # highlighting, which we don't need here. diff --git a/app/serializers/commit_entity.rb b/app/serializers/commit_entity.rb index c9c4981682a462..4090c216472d63 100644 --- a/app/serializers/commit_entity.rb +++ b/app/serializers/commit_entity.rb @@ -1,4 +1,49 @@ # frozen_string_literal: true -class CommitEntity < API::Entities::CommitWithLink -end +class CommitEntity < API::Entities::Commit + include MarkupHelper + include RequestAwareEntity + + expose :author, using: UserEntity + + expose :author_gravatar_url do |commit| + GravatarService.new.execute(commit.author_email) # rubocop: disable CodeReuse/ServiceClass + end + + expose :commit_url do |commit, options| + project_commit_url(request.project, commit, params: options.fetch(:commit_url_params, {})) + end + + expose :commit_path do |commit, options| + project_commit_path(request.project, commit, params: options.fetch(:commit_url_params, {})) + end + + expose :description_html, if: { type: :full } do |commit| + markdown_field(commit, :description) + end + + expose :title_html, if: { type: :full } do |commit| + markdown_field(commit, :title) + end + + expose :signature_html, if: { type: :full } do |commit| + render('projects/commit/_signature', signature: commit.signature) if commit.has_signature? + end + + expose :pipeline_status_path, if: { type: :full } do |commit, options| + pipeline_ref = options[:pipeline_ref] + pipeline_project = options[:pipeline_project] || commit.project + next unless pipeline_ref && pipeline_project + + pipeline = commit.latest_pipeline_for_project(pipeline_ref, pipeline_project) + next unless pipeline&.status + + pipelines_project_commit_path(pipeline_project, commit.id, ref: pipeline_ref) + end + + def render(*args) + return unless request.respond_to?(:render) && request.render.respond_to?(:call) + + request.render.call(*args) + end +end \ No newline at end of file diff --git a/app/serializers/user_entity.rb b/app/serializers/user_entity.rb index 8909ae8df2cc4c..656900bb8af5ae 100644 --- a/app/serializers/user_entity.rb +++ b/app/serializers/user_entity.rb @@ -1,4 +1,10 @@ # frozen_string_literal: true -class UserEntity < API::Entities::UserPath +class UserEntity < API::Entities::UserBasic + include RequestAwareEntity + include UserStatusTooltip + + expose :path do |user| + user_path(user) + end end diff --git a/app/views/projects/commits/_commit.html.haml b/app/views/projects/commits/_commit.html.haml index a9180f3e8a21f9..8b659034fe6454 100644 --- a/app/views/projects/commits/_commit.html.haml +++ b/app/views/projects/commits/_commit.html.haml @@ -54,7 +54,7 @@ .js-commit-pipeline-status{ data: { endpoint: pipelines_project_commit_path(project, commit.id, ref: ref) } } .commit-sha-group.d-none.d-sm-flex - .label.label-monospace.monospace{ class: ("text-danger" if context_commit?(merge_request, commit.id)) } + .label.label-monospace.monospace = commit.short_id = clipboard_button(text: commit.id, title: _("Copy commit SHA"), class: "btn btn-default", container: "body") = link_to_browse_code(project, commit) diff --git a/lib/api/entities/commit_with_link.rb b/lib/api/entities/commit_with_link.rb deleted file mode 100644 index 31a9efed9bc77e..00000000000000 --- a/lib/api/entities/commit_with_link.rb +++ /dev/null @@ -1,53 +0,0 @@ -# frozen_string_literal: true - -module API - module Entities - class CommitWithLink < Commit - include MarkupHelper - include RequestAwareEntity - - expose :author, using: Entities::UserPath - - expose :author_gravatar_url do |commit| - GravatarService.new.execute(commit.author_email) - end - - expose :commit_url do |commit, options| - project_commit_url(request.project, commit, params: options.fetch(:commit_url_params, {})) - end - - expose :commit_path do |commit, options| - project_commit_path(request.project, commit, params: options.fetch(:commit_url_params, {})) - end - - expose :description_html, if: { type: :full } do |commit| - markdown_field(commit, :description) - end - - expose :title_html, if: { type: :full } do |commit| - markdown_field(commit, :title) - end - - expose :signature_html, if: { type: :full } do |commit| - render('projects/commit/_signature', signature: commit.signature) if commit.has_signature? - end - - expose :pipeline_status_path, if: { type: :full } do |commit, options| - pipeline_ref = options[:pipeline_ref] - pipeline_project = options[:pipeline_project] || commit.project - next unless pipeline_ref && pipeline_project - - pipeline = commit.latest_pipeline_for_project(pipeline_ref, pipeline_project) - next unless pipeline&.status - - pipelines_project_commit_path(pipeline_project, commit.id, ref: pipeline_ref) - end - - def render(*args) - return unless request.respond_to?(:render) && request.render.respond_to?(:call) - - request.render.call(*args) - end - end - end -end diff --git a/lib/api/entities/user_path.rb b/lib/api/entities/user_path.rb deleted file mode 100644 index 7d922b39654646..00000000000000 --- a/lib/api/entities/user_path.rb +++ /dev/null @@ -1,14 +0,0 @@ -# frozen_string_literal: true - -module API - module Entities - class UserPath < UserBasic - include RequestAwareEntity - include UserStatusTooltip - - expose :path do |user| - user_path(user) - end - end - end -end diff --git a/lib/api/merge_requests.rb b/lib/api/merge_requests.rb index bc75293c177a76..2b1bcc855d2738 100644 --- a/lib/api/merge_requests.rb +++ b/lib/api/merge_requests.rb @@ -306,7 +306,7 @@ def handle_merge_request_errors!(errors) context_commits = paginate(merge_request.merge_request_context_commits).map(&:to_commit) - present context_commits, with: Entities::CommitWithLink, type: :full, request: merge_request + present context_commits, with: Entities::Commit end params do @@ -362,64 +362,6 @@ def handle_merge_request_errors!(errors) status 204 end - desc 'Get the context commits of a merge request' do - success Entities::MergeRequestContextCommit - end - get ':id/merge_requests/:merge_request_iid/context_commits' do - merge_request = find_merge_request_with_access(params[:merge_request_iid]) - context_diffs = merge_request.context_diffs - present paginate(context_diffs), with: Entities::MergeRequestContextCommit - end - - params do - optional :commits, type: Array, allow_blank: false, desc: 'List of context commits sha' - end - desc 'create context commits of merge request' do - success Entities::Commit - end - post ':id/merge_requests/:merge_request_iid/context_commits' do - commit_ids = params[:commits] - merge_request = find_merge_request_with_access(params[:merge_request_iid]) - existing_oids = merge_request.merge_request_context_commits.map { |commit| commit.sha.to_s } - duplicate_oids = existing_oids.select do |existing_oid| - commit_ids.select { |commit_id| existing_oid.start_with?(commit_id) }.count > 0 - end - - if duplicate_oids.count > 0 - render_api_error!("Context commits: #{duplicate_oids} are already created", 400) - end - - project = merge_request.target_project - commits = project.repository.commits_by(oids: commit_ids) - - if commits.size != commit_ids.size - render_api_error!("One or more context commits' sha is not valid.", 400) - end - - MergeRequestContextCommit.create_bulk(merge_request, project.repository.raw_repository, commits) - present commits, with: Entities::Commit - end - - params do - optional :commits, type: Array, allow_blank: false, desc: 'List of context commits sha' - end - desc 'remove context commits of merge request' do - success Entities::Commit - end - delete ':id/merge_requests/:merge_request_iid/context_commits' do - commit_ids = params[:commits] - merge_request = find_merge_request_with_access(params[:merge_request_iid]) - project = merge_request.target_project - commits = project.repository.commits_by(oids: commit_ids) - - if commits.size != commit_ids.size - render_api_error!("One or more context commits' sha is not valid.", 400) - end - - MergeRequestContextCommit.delete_bulk(merge_request, commits) - present commits, with: Entities::Commit - end - desc 'Show the merge request changes' do success Entities::MergeRequestChanges end diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml deleted file mode 100644 index 2dcb397e03b80f..00000000000000 --- a/lib/gitlab/import_export/import_export.yml +++ /dev/null @@ -1,390 +0,0 @@ -# Model relationships to be included in the project import/export -# -# This list _must_ only contain relationships that are available to both CE and -# EE. EE specific relationships must be defined in the `ee` section further -# down below. -tree: - project: - - labels: - - :priorities - - milestones: - - events: - - :push_event_payload - - issues: - - events: - - :push_event_payload - - :timelogs - - notes: - - :author - - :award_emoji - - events: - - :push_event_payload - - label_links: - - label: - - :priorities - - milestone: - - events: - - :push_event_payload - - issue_milestones: - - :milestone - - resource_label_events: - - label: - - :priorities - - :issue_assignees - - :zoom_meetings - - :sentry_issue - - :award_emoji - - snippets: - - :award_emoji - - notes: - - :author - - :award_emoji - - releases: - - :links - - project_members: - - :user - - merge_requests: - - :metrics - - :award_emoji - - notes: - - :author - - :award_emoji - - events: - - :push_event_payload - - :suggestions - - merge_request_diff: - - :merge_request_diff_commits - - :merge_request_diff_files - - :merge_request_context_commits - - events: - - :push_event_payload - - :timelogs - - label_links: - - label: - - :priorities - - milestone: - - events: - - :push_event_payload - - merge_request_milestones: - - :milestone - - resource_label_events: - - label: - - :priorities - - ci_pipelines: - - notes: - - :author - - events: - - :push_event_payload - - stages: - - :statuses - - :external_pull_request - - :merge_request - - :external_pull_requests - - :auto_devops - - :triggers - - :pipeline_schedules - - :container_expiration_policy - - :services - - protected_branches: - - :merge_access_levels - - :push_access_levels - - protected_tags: - - :create_access_levels - - :project_feature - - :custom_attributes - - :prometheus_metrics - - :project_badges - - :ci_cd_settings - - :error_tracking_setting - - :metrics_setting - - boards: - - lists: - - label: - - :priorities - group_members: - - :user - -# Only include the following attributes for the models specified. -included_attributes: - user: - - :id - - :email - - :username - author: - - :name - ci_cd_settings: - - :group_runners_enabled - -# Do not include the following attributes for the models specified. -excluded_attributes: - project: - - :name - - :path - - :namespace_id - - :creator_id - - :pool_repository_id - - :import_url - - :import_status - - :avatar - - :import_type - - :import_source - - :mirror - - :runners_token - - :runners_token_encrypted - - :repository_storage - - :repository_read_only - - :lfs_enabled - - :created_at - - :updated_at - - :id - - :star_count - - :last_activity_at - - :last_repository_updated_at - - :last_repository_check_at - - :storage_version - - :remote_mirror_available_overridden - - :description_html - - :repository_languages - - :bfg_object_map - - :detected_repository_languages - - :tag_list - - :mirror_user_id - - :mirror_trigger_builds - - :only_mirror_protected_branches - - :pull_mirror_available_overridden - - :pull_mirror_branch_prefix - - :mirror_overwrites_diverged_branches - - :packages_enabled - - :mirror_last_update_at - - :mirror_last_successful_update_at - - :emails_disabled - - :max_pages_size - - :max_artifacts_size - - :marked_for_deletion_at - - :marked_for_deletion_by_user_id - namespaces: - - :runners_token - - :runners_token_encrypted - project_import_state: - - :last_error - - :jid - - :last_update_at - - :last_successful_update_at - prometheus_metrics: - - :common - - :identifier - snippets: - - :expired_at - - :secret - - :encrypted_secret_token - - :encrypted_secret_token_iv - - :repository_storage - merge_request_diff: - - :external_diff - - :stored_externally - - :external_diff_store - - :merge_request_id - merge_request_diff_commits: - - :merge_request_diff_id - merge_request_diff_files: - - :diff - - :external_diff_offset - - :external_diff_size - - :merge_request_diff_id - issues: - - :milestone_id - - :moved_to_id - - :state_id - - :duplicated_to_id - - :promoted_to_epic_id - merge_request: - - :milestone_id - - :ref_fetched - - :merge_jid - - :rebase_jid - - :latest_merge_request_diff_id - - :head_pipeline_id - - :state_id - merge_requests: - - :milestone_id - - :ref_fetched - - :merge_jid - - :rebase_jid - - :latest_merge_request_diff_id - - :head_pipeline_id - - :state_id - issue_milestones: - - :milestone_id - - :issue_id - merge_request_milestones: - - :milestone_id - - :merge_request_id - award_emoji: - - :awardable_id - statuses: - - :trace - - :token - - :token_encrypted - - :when - - :artifacts_file - - :artifacts_metadata - - :artifacts_file_store - - :artifacts_metadata_store - - :artifacts_size - - :commands - - :runner_id - - :trigger_request_id - - :erased_by_id - - :auto_canceled_by_id - - :stage_id - - :upstream_pipeline_id - - :resource_group_id - - :waiting_for_resource_at - - :processed - sentry_issue: - - :issue_id - push_event_payload: - - :event_id - project_badges: - - :group_id - resource_label_events: - - :reference - - :reference_html - - :epic_id - - :issue_id - - :merge_request_id - - :label_id - runners: - - :token - - :token_encrypted - services: - - :template - error_tracking_setting: - - :encrypted_token - - :encrypted_token_iv - - :enabled - service_desk_setting: - - :outgoing_name - priorities: - - :label_id - events: - - :target_id - timelogs: - - :issue_id - - :merge_request_id - notes: - - :noteable_id - - :review_id - label_links: - - :label_id - - :target_id - issue_assignees: - - :issue_id - zoom_meetings: - - :issue_id - design: - - :issue_id - designs: - - :issue_id - design_versions: - - :issue_id - actions: - - :design_id - - :version_id - links: - - :release_id - project_members: - - :source_id - metrics: - - :merge_request_id - - :pipeline_id - suggestions: - - :note_id - ci_pipelines: - - :auto_canceled_by_id - - :pipeline_schedule_id - - :merge_request_id - - :external_pull_request_id - stages: - - :pipeline_id - merge_access_levels: - - :protected_branch_id - push_access_levels: - - :protected_branch_id - unprotect_access_levels: - - :protected_branch_id - create_access_levels: - - :protected_tag_id - deploy_access_levels: - - :protected_environment_id - boards: - - :milestone_id - lists: - - :board_id - - :label_id - - :milestone_id - epic: - - :start_date_sourcing_milestone_id - - :due_date_sourcing_milestone_id - - :parent_id - - :state_id - - :start_date_sourcing_epic_id - - :due_date_sourcing_epic_id -methods: - notes: - - :type - labels: - - :type - label: - - :type - statuses: - - :type - services: - - :type - merge_request_diff_files: - - :utf8_diff - merge_requests: - - :diff_head_sha - - :source_branch_sha - - :target_branch_sha - events: - - :action - push_event_payload: - - :action - project_badges: - - :type - lists: - - :list_type - ci_pipelines: - - :notes - -preloads: - statuses: - # TODO: We cannot preload tags, as they are not part of `GenericCommitStatus` - # tags: # needed by tag_list - project: # deprecated: needed by coverage_regex of Ci::Build - merge_requests: - source_project: # needed by source_branch_sha and diff_head_sha - target_project: # needed by target_branch_sha - assignees: # needed by assigne_id that is implemented by DeprecatedAssignee - -# EE specific relationships and settings to include. All of this will be merged -# into the previous structures if EE is used. -ee: - tree: - project: - - issues: - - designs: - - notes: - - :author - - events: - - :push_event_payload - - design_versions: - - actions: - - :design # Duplicate export of issues.designs in order to link the record to both Issue and Action - - :epic - - protected_branches: - - :unprotect_access_levels - - protected_environments: - - :deploy_access_levels - - :service_desk_setting -- GitLab From 6d35cae7912a3f03bec39326d05acb2e6f1b2fce Mon Sep 17 00:00:00 2001 From: Anwar Sadath Date: Fri, 20 Mar 2020 22:30:38 +0530 Subject: [PATCH 09/38] Fix lint --- app/serializers/commit_entity.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/serializers/commit_entity.rb b/app/serializers/commit_entity.rb index 4090c216472d63..ae3f1c6bbf5e97 100644 --- a/app/serializers/commit_entity.rb +++ b/app/serializers/commit_entity.rb @@ -46,4 +46,4 @@ def render(*args) request.render.call(*args) end -end \ No newline at end of file +end -- GitLab From 8c44bbcbfea5ac3870100ca77ffd700973f4eee3 Mon Sep 17 00:00:00 2001 From: Anwar Sadath Date: Tue, 7 Apr 2020 20:20:53 +0530 Subject: [PATCH 10/38] Move add context commits modal to commits tab and code refactor --- .../add_context_commits_modal_trigger.vue | 38 ++++ .../add_context_commits_modal_wrapper.vue | 164 ++++++++++++++++++ .../components/review_tab_container.vue | 18 +- .../add_context_commits_modal/event_hub.js | 3 + .../add_context_commits_modal/index.js | 58 +++++++ .../store/actions.js | 88 ++++++++++ .../add_context_commits_modal/store/index.js | 15 ++ .../store/mutation_types.js | 17 ++ .../store/mutations.js | 54 ++++++ .../add_context_commits_modal/store/state.js | 13 ++ .../components/add_review_items_modal.vue | 124 ------------- .../javascripts/diffs/components/app.vue | 10 -- .../diffs/components/commit_item.vue | 30 ++-- .../diffs/components/compare_versions.vue | 91 +--------- app/assets/javascripts/diffs/index.js | 4 - app/assets/javascripts/diffs/store/actions.js | 75 -------- .../diffs/store/modules/diff_state.js | 9 - .../javascripts/diffs/store/mutation_types.js | 16 -- .../javascripts/diffs/store/mutations.js | 42 ----- app/assets/javascripts/merge_request_tabs.js | 2 + app/assets/stylesheets/pages/commits.scss | 5 + app/assets/stylesheets/pages/diff.scss | 8 - .../stylesheets/pages/merge_requests.scss | 4 + .../projects/merge_requests_controller.rb | 8 +- app/views/projects/commits/_commits.html.haml | 15 ++ .../merge_requests/_commits.html.haml | 12 +- .../projects/merge_requests/show.html.haml | 4 +- spec/frontend/diffs/components/app_spec.js | 2 - 28 files changed, 520 insertions(+), 409 deletions(-) create mode 100644 app/assets/javascripts/add_context_commits_modal/components/add_context_commits_modal_trigger.vue create mode 100644 app/assets/javascripts/add_context_commits_modal/components/add_context_commits_modal_wrapper.vue rename app/assets/javascripts/{diffs => add_context_commits_modal}/components/review_tab_container.vue (74%) create mode 100644 app/assets/javascripts/add_context_commits_modal/event_hub.js create mode 100644 app/assets/javascripts/add_context_commits_modal/index.js create mode 100644 app/assets/javascripts/add_context_commits_modal/store/actions.js create mode 100644 app/assets/javascripts/add_context_commits_modal/store/index.js create mode 100644 app/assets/javascripts/add_context_commits_modal/store/mutation_types.js create mode 100644 app/assets/javascripts/add_context_commits_modal/store/mutations.js create mode 100644 app/assets/javascripts/add_context_commits_modal/store/state.js delete mode 100644 app/assets/javascripts/diffs/components/add_review_items_modal.vue diff --git a/app/assets/javascripts/add_context_commits_modal/components/add_context_commits_modal_trigger.vue b/app/assets/javascripts/add_context_commits_modal/components/add_context_commits_modal_trigger.vue new file mode 100644 index 00000000000000..9bfe1e18aa07d7 --- /dev/null +++ b/app/assets/javascripts/add_context_commits_modal/components/add_context_commits_modal_trigger.vue @@ -0,0 +1,38 @@ + + + diff --git a/app/assets/javascripts/add_context_commits_modal/components/add_context_commits_modal_wrapper.vue b/app/assets/javascripts/add_context_commits_modal/components/add_context_commits_modal_wrapper.vue new file mode 100644 index 00000000000000..fd5e11e2a1e4b5 --- /dev/null +++ b/app/assets/javascripts/add_context_commits_modal/components/add_context_commits_modal_wrapper.vue @@ -0,0 +1,164 @@ + + + diff --git a/app/assets/javascripts/diffs/components/review_tab_container.vue b/app/assets/javascripts/add_context_commits_modal/components/review_tab_container.vue similarity index 74% rename from app/assets/javascripts/diffs/components/review_tab_container.vue rename to app/assets/javascripts/add_context_commits_modal/components/review_tab_container.vue index db07060808c70e..31ca21c19e7de4 100644 --- a/app/assets/javascripts/diffs/components/review_tab_container.vue +++ b/app/assets/javascripts/add_context_commits_modal/components/review_tab_container.vue @@ -1,6 +1,7 @@