diff --git a/.eslintrc.yml b/.eslintrc.yml index 77b1b72fe68..ebf8048f19c 100644 --- a/.eslintrc.yml +++ b/.eslintrc.yml @@ -33,6 +33,15 @@ rules: - error - max: 1 promise/catch-or-return: error + no-param-reassign: + - error + - props: true + ignorePropertyModificationsFor: + - "acc" # for reduce accumulators + - "accumulator" # for reduce accumulators + - "el" # for DOM elements + - "element" # for DOM elements + - "state" # for Vuex mutations no-underscore-dangle: - error - allow: diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 1b4134282c9..c0b622f5abd 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -1,4 +1,4 @@ -image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.4.4-golang-1.9-git-2.18-chrome-67.0-node-8.x-yarn-1.2-postgresql-9.6-graphicsmagick-1.3.29" +image: "dev.gitlab.org:5005/gitlab/gitlab-build-images:ruby-2.4.4-golang-1.9-git-2.18-chrome-69.0-node-8.x-yarn-1.2-postgresql-9.6-graphicsmagick-1.3.29" .dedicated-runner: &dedicated-runner retry: 1 @@ -708,7 +708,6 @@ gitlab:assets:compile: SETUP_DB: "false" SKIP_STORAGE_VALIDATION: "true" WEBPACK_REPORT: "true" - NO_COMPRESSION: "true" # we override the max_old_space_size to prevent OOM errors NODE_OPTIONS: --max_old_space_size=3584 script: @@ -722,6 +721,7 @@ gitlab:assets:compile: expire_in: 31d paths: - webpack-report/ + - public/assets/ karma: <<: *dedicated-no-docs-and-no-qa-pull-cache-job diff --git a/.gitlab/CODEOWNERS b/.gitlab/CODEOWNERS index 5b6e5a719fa..7fd32563696 100644 --- a/.gitlab/CODEOWNERS +++ b/.gitlab/CODEOWNERS @@ -13,3 +13,5 @@ db/ @abrandl @NikolayS # Feature specific owners /ee/lib/gitlab/code_owners/ @reprazent +/ee/lib/ee/gitlab/auth/ldap/ @dblessing @mkozono +/lib/gitlab/auth/ldap/ @dblessing @mkozono diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 1b9e9d4a5a3..7f1da58fa9d 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -36,7 +36,7 @@ _This notice should stay as the first item in the CONTRIBUTING.md file._ - [Release Scoping labels](#release-scoping-labels) - [Priority labels](#priority-labels) - [Severity labels](#severity-labels) - - [Severity impact guidance](#severity-impact-guidance) + - [Severity impact guidance](#severity-impact-guidance) - [Label for community contributors](#label-for-community-contributors) - [Implement design & UI elements](#implement-design--ui-elements) - [Issue tracker](#issue-tracker) diff --git a/Dangerfile b/Dangerfile index 9217610da8b..46e53edcac4 100644 --- a/Dangerfile +++ b/Dangerfile @@ -4,4 +4,6 @@ danger.import_dangerfile(path: 'danger/changelog') danger.import_dangerfile(path: 'danger/specs') danger.import_dangerfile(path: 'danger/gemfile') danger.import_dangerfile(path: 'danger/database') +danger.import_dangerfile(path: 'danger/documentation') danger.import_dangerfile(path: 'danger/frozen_string') +danger.import_dangerfile(path: 'danger/commit_messages') diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 377d8aca07e..99e0d1ed987 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -0.117.2 +0.120.0 diff --git a/GITLAB_SHELL_VERSION b/GITLAB_SHELL_VERSION index 0e79152459e..56b6be4ebb2 100644 --- a/GITLAB_SHELL_VERSION +++ b/GITLAB_SHELL_VERSION @@ -1 +1 @@ -8.1.1 +8.3.1 diff --git a/Gemfile b/Gemfile index fe788370640..27e6ba21ff3 100644 --- a/Gemfile +++ b/Gemfile @@ -170,7 +170,7 @@ gem 'state_machines-activerecord', '~> 0.5.1' gem 'acts-as-taggable-on', '~> 5.0' # Background jobs -gem 'sidekiq', '~> 5.1' +gem 'sidekiq', '~> 5.2.1' gem 'sidekiq-cron', '~> 0.6.0' gem 'redis-namespace', '~> 1.6.0' gem 'sidekiq-limit_fetch', '~> 3.4', require: false @@ -425,7 +425,7 @@ group :ed25519 do end # Gitaly GRPC client -gem 'gitaly-proto', '~> 0.113.0', require: 'gitaly' +gem 'gitaly-proto', '~> 0.117.0', require: 'gitaly' gem 'grpc', '~> 1.11.0' # Locked until https://github.com/google/protobuf/issues/4210 is closed diff --git a/Gemfile.lock b/Gemfile.lock index e7ab97fb299..8c545b7257c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -133,7 +133,7 @@ GEM concurrent-ruby (1.0.5) concurrent-ruby-ext (1.0.5) concurrent-ruby (= 1.0.5) - connection_pool (2.2.1) + connection_pool (2.2.2) crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.4) @@ -208,7 +208,7 @@ GEM fast_blank (1.0.0) fast_gettext (1.6.0) ffaker (2.4.0) - ffi (1.9.18) + ffi (1.9.25) flipper (0.13.0) flipper-active_record (0.13.0) activerecord (>= 3.2, < 6) @@ -276,7 +276,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.113.0) + gitaly-proto (0.117.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -649,7 +649,7 @@ GEM httpclient (>= 2.4) multi_json (>= 1.3.6) rack (>= 1.1) - rack-protection (2.0.1) + rack-protection (2.0.3) rack rack-proxy (0.6.0) rack @@ -843,9 +843,8 @@ GEM rack shoulda-matchers (3.1.2) activesupport (>= 4.0.0) - sidekiq (5.1.3) - concurrent-ruby (~> 1.0) - connection_pool (~> 2.2, >= 2.2.0) + sidekiq (5.2.1) + connection_pool (~> 2.2, >= 2.2.2) rack-protection (>= 1.5.0) redis (>= 3.3.5, < 5) sidekiq-cron (0.6.0) @@ -1038,7 +1037,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.113.0) + gitaly-proto (~> 0.117.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) @@ -1166,7 +1165,7 @@ DEPENDENCIES settingslogic (~> 2.0.9) sham_rack (~> 1.3.6) shoulda-matchers (~> 3.1.2) - sidekiq (~> 5.1) + sidekiq (~> 5.2.1) sidekiq-cron (~> 0.6.0) sidekiq-limit_fetch (~> 3.4) simple_po_parser (~> 1.1.2) diff --git a/Gemfile.rails5.lock b/Gemfile.rails5.lock index 1a1aac9f439..b384592d035 100644 --- a/Gemfile.rails5.lock +++ b/Gemfile.rails5.lock @@ -136,7 +136,7 @@ GEM concurrent-ruby (1.0.5) concurrent-ruby-ext (1.0.5) concurrent-ruby (= 1.0.5) - connection_pool (2.2.1) + connection_pool (2.2.2) crack (0.4.3) safe_yaml (~> 1.0.0) crass (1.0.4) @@ -211,7 +211,7 @@ GEM fast_blank (1.0.0) fast_gettext (1.6.0) ffaker (2.4.0) - ffi (1.9.18) + ffi (1.9.25) flipper (0.13.0) flipper-active_record (0.13.0) activerecord (>= 3.2, < 6) @@ -279,7 +279,7 @@ GEM gettext_i18n_rails (>= 0.7.1) po_to_json (>= 1.0.0) rails (>= 3.2.0) - gitaly-proto (0.113.0) + gitaly-proto (0.117.0) google-protobuf (~> 3.1) grpc (~> 1.10) github-linguist (5.3.3) @@ -653,7 +653,7 @@ GEM httpclient (>= 2.4) multi_json (>= 1.3.6) rack (>= 1.1) - rack-protection (2.0.1) + rack-protection (2.0.3) rack rack-proxy (0.6.0) rack @@ -851,9 +851,8 @@ GEM rack shoulda-matchers (3.1.2) activesupport (>= 4.0.0) - sidekiq (5.1.3) - concurrent-ruby (~> 1.0) - connection_pool (~> 2.2, >= 2.2.0) + sidekiq (5.2.1) + connection_pool (~> 2.2, >= 2.2.2) rack-protection (>= 1.5.0) redis (>= 3.3.5, < 5) sidekiq-cron (0.6.0) @@ -1047,7 +1046,7 @@ DEPENDENCIES gettext (~> 3.2.2) gettext_i18n_rails (~> 1.8.0) gettext_i18n_rails_js (~> 1.3) - gitaly-proto (~> 0.113.0) + gitaly-proto (~> 0.117.0) github-linguist (~> 5.3.3) gitlab-flowdock-git-hook (~> 1.0.1) gitlab-gollum-lib (~> 4.2) @@ -1176,7 +1175,7 @@ DEPENDENCIES settingslogic (~> 2.0.9) sham_rack (~> 1.3.6) shoulda-matchers (~> 3.1.2) - sidekiq (~> 5.1) + sidekiq (~> 5.2.1) sidekiq-cron (~> 0.6.0) sidekiq-limit_fetch (~> 3.4) simple_po_parser (~> 1.1.2) diff --git a/LICENSE b/LICENSE index a76372fad2c..a90ea939517 100644 --- a/LICENSE +++ b/LICENSE @@ -1,7 +1,19 @@ Copyright GitLab B.V. -Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: +Permission is hereby granted, free of charge, to any person obtaining a copy +of this software and associated documentation files (the "Software"), to deal +in the Software without restriction, including without limitation the rights +to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +copies of the Software, and to permit persons to whom the Software is +furnished to do so, subject to the following conditions: -The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. +The above copyright notice and this permission notice shall be included in all +copies or substantial portions of the Software. -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. \ No newline at end of file +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE +SOFTWARE. diff --git a/app/assets/javascripts/boards/components/modal/index.vue b/app/assets/javascripts/boards/components/modal/index.vue index 33e72a6782e..7b33a7573e7 100644 --- a/app/assets/javascripts/boards/components/modal/index.vue +++ b/app/assets/javascripts/boards/components/modal/index.vue @@ -1,6 +1,6 @@ @@ -31,6 +40,7 @@ export default { :render-diff-file="false" :always-expanded="true" :discussions-by-diff-order="true" + @noteDeleted="deleteNoteHandler" /> diff --git a/app/assets/javascripts/diffs/components/diff_file.vue b/app/assets/javascripts/diffs/components/diff_file.vue index 59e9ba08b8b..67e85c4eee3 100644 --- a/app/assets/javascripts/diffs/components/diff_file.vue +++ b/app/assets/javascripts/diffs/components/diff_file.vue @@ -1,5 +1,5 @@ @@ -53,7 +40,7 @@ export default { class="code diff-wrap-lines js-syntax-highlight text-file js-diff-inline-view"> diff --git a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue index 48b8feeb0b4..26417c350cb 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_comment_row.vue @@ -21,51 +21,49 @@ export default { type: Number, required: true, }, - leftDiscussions: { - type: Array, - required: false, - default: () => [], - }, - rightDiscussions: { - type: Array, - required: false, - default: () => [], - }, }, computed: { ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), leftLineCode() { - return this.line.left.lineCode; + return this.line.left && this.line.left.lineCode; }, rightLineCode() { - return this.line.right.lineCode; + return this.line.right && this.line.right.lineCode; }, hasExpandedDiscussionOnLeft() { - const discussions = this.leftDiscussions; - - return discussions ? discussions.every(discussion => discussion.expanded) : false; + return this.line.left && this.line.left.discussions + ? this.line.left.discussions.every(discussion => discussion.expanded) + : false; }, hasExpandedDiscussionOnRight() { - const discussions = this.rightDiscussions; - - return discussions ? discussions.every(discussion => discussion.expanded) : false; + return this.line.right && this.line.right.discussions + ? this.line.right.discussions.every(discussion => discussion.expanded) + : false; }, hasAnyExpandedDiscussion() { return this.hasExpandedDiscussionOnLeft || this.hasExpandedDiscussionOnRight; }, shouldRenderDiscussionsOnLeft() { - return this.leftDiscussions && this.hasExpandedDiscussionOnLeft; + return this.line.left && this.line.left.discussions && this.hasExpandedDiscussionOnLeft; }, shouldRenderDiscussionsOnRight() { - return this.rightDiscussions && this.hasExpandedDiscussionOnRight && this.line.right.type; + return ( + this.line.right && + this.line.right.discussions && + this.hasExpandedDiscussionOnRight && + this.line.right.type + ); }, showRightSideCommentForm() { - return this.line.right.type && this.diffLineCommentForms[this.rightLineCode]; + return ( + this.line.right && this.line.right.type && this.diffLineCommentForms[this.rightLineCode] + ); }, className() { - return this.leftDiscussions.length > 0 || this.rightDiscussions.length > 0 + return (this.left && this.line.left.discussions.length > 0) || + (this.right && this.line.right.discussions.length > 0) ? '' : 'js-temp-notes-holder'; }, @@ -85,8 +83,8 @@ export default { class="content" > import $ from 'jquery'; -import { mapGetters } from 'vuex'; import DiffTableCell from './diff_table_cell.vue'; import { NEW_LINE_TYPE, @@ -10,8 +9,7 @@ import { OLD_NO_NEW_LINE_TYPE, PARALLEL_DIFF_VIEW_TYPE, NEW_NO_NEW_LINE_TYPE, - LINE_POSITION_LEFT, - LINE_POSITION_RIGHT, + EMPTY_CELL_TYPE, } from '../constants'; export default { @@ -36,16 +34,6 @@ export default { required: false, default: false, }, - leftDiscussions: { - type: Array, - required: false, - default: () => [], - }, - rightDiscussions: { - type: Array, - required: false, - default: () => [], - }, }, data() { return { @@ -54,29 +42,26 @@ export default { }; }, computed: { - ...mapGetters('diffs', ['isParallelView']), isContextLine() { - return this.line.left.type === CONTEXT_LINE_TYPE; + return this.line.left && this.line.left.type === CONTEXT_LINE_TYPE; }, classNameMap() { return { [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, - [PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView, + [PARALLEL_DIFF_VIEW_TYPE]: true, }; }, parallelViewLeftLineType() { - if (this.line.right.type === NEW_NO_NEW_LINE_TYPE) { + if (this.line.right && this.line.right.type === NEW_NO_NEW_LINE_TYPE) { return OLD_NO_NEW_LINE_TYPE; } - return this.line.left.type; + return this.line.left ? this.line.left.type : EMPTY_CELL_TYPE; }, }, created() { this.newLineType = NEW_LINE_TYPE; this.oldLineType = OLD_LINE_TYPE; - this.linePositionLeft = LINE_POSITION_LEFT; - this.linePositionRight = LINE_POSITION_RIGHT; this.parallelDiffViewType = PARALLEL_DIFF_VIEW_TYPE; }, methods: { @@ -116,47 +101,57 @@ export default { @mouseover="handleMouseMove" @mouseout="handleMouseMove" > - - - - - - + + + + diff --git a/app/assets/javascripts/diffs/components/parallel_diff_view.vue b/app/assets/javascripts/diffs/components/parallel_diff_view.vue index 24ceb52a04a..501bd4450d8 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_view.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_view.vue @@ -2,8 +2,6 @@ import { mapState, mapGetters } from 'vuex'; import parallelDiffTableRow from './parallel_diff_table_row.vue'; import parallelDiffCommentRow from './parallel_diff_comment_row.vue'; -import { EMPTY_CELL_TYPE } from '../constants'; -import { trimFirstCharOfLineContent } from '../store/utils'; export default { components: { @@ -21,46 +19,17 @@ export default { }, }, computed: { - ...mapGetters('diffs', [ - 'commitId', - 'singleDiscussionByLineCode', - 'shouldRenderParallelCommentRow', - ]), + ...mapGetters('diffs', ['commitId', 'shouldRenderParallelCommentRow']), ...mapState({ diffLineCommentForms: state => state.diffs.diffLineCommentForms, }), - parallelDiffLines() { - return this.diffLines.map(line => { - const parallelLine = Object.assign({}, line); - - if (line.left) { - parallelLine.left = trimFirstCharOfLineContent(line.left); - } else { - parallelLine.left = { type: EMPTY_CELL_TYPE }; - } - - if (line.right) { - parallelLine.right = trimFirstCharOfLineContent(line.right); - } else { - parallelLine.right = { type: EMPTY_CELL_TYPE }; - } - - return parallelLine; - }); - }, diffLinesLength() { - return this.parallelDiffLines.length; + return this.diffLines.length; }, userColorScheme() { return window.gon.user_color_scheme; }, }, - methods: { - discussionsByLine(line, leftOrRight) { - return line[leftOrRight] && line[leftOrRight].lineCode !== undefined ? - this.singleDiscussionByLineCode(line[leftOrRight].lineCode) : []; - }, - }, }; @@ -73,7 +42,7 @@ export default { diff --git a/app/assets/javascripts/diffs/store/actions.js b/app/assets/javascripts/diffs/store/actions.js index 4ab6ceb249a..027df2ec841 100644 --- a/app/assets/javascripts/diffs/store/actions.js +++ b/app/assets/javascripts/diffs/store/actions.js @@ -3,6 +3,7 @@ import axios from '~/lib/utils/axios_utils'; import Cookies from 'js-cookie'; import { handleLocationHash, historyPushState } from '~/lib/utils/common_utils'; import { mergeUrlParams } from '~/lib/utils/url_utility'; +import { getDiffPositionByLineCode } from './utils'; import * as types from './mutation_types'; import { PARALLEL_DIFF_VIEW_TYPE, @@ -29,25 +30,53 @@ export const fetchDiffFiles = ({ state, commit }) => { .then(handleLocationHash); }; -export const startRenderDiffsQueue = ({ state, commit }) => { - const checkItem = () => { - const nextFile = state.diffFiles.find( - file => !file.renderIt && (!file.collapsed || !file.text), - ); - if (nextFile) { - requestAnimationFrame(() => { - commit(types.RENDER_FILE, nextFile); - }); - requestIdleCallback( - () => { - checkItem(); - }, - { timeout: 1000 }, - ); - } - }; +// This is adding line discussions to the actual lines in the diff tree +// once for parallel and once for inline mode +export const assignDiscussionsToDiff = ({ state, commit }, allLineDiscussions) => { + const diffPositionByLineCode = getDiffPositionByLineCode(state.diffFiles); - checkItem(); + Object.values(allLineDiscussions).forEach(discussions => { + if (discussions.length > 0) { + const { fileHash } = discussions[0]; + commit(types.SET_LINE_DISCUSSIONS_FOR_FILE, { + fileHash, + discussions, + diffPositionByLineCode, + }); + } + }); +}; + +export const removeDiscussionsFromDiff = ({ commit }, removeDiscussion) => { + const { fileHash, line_code } = removeDiscussion; + commit(types.REMOVE_LINE_DISCUSSIONS_FOR_FILE, { fileHash, lineCode: line_code }); +}; + +export const startRenderDiffsQueue = ({ state, commit }) => { + const checkItem = () => + new Promise(resolve => { + const nextFile = state.diffFiles.find( + file => !file.renderIt && (!file.collapsed || !file.text), + ); + + if (nextFile) { + requestAnimationFrame(() => { + commit(types.RENDER_FILE, nextFile); + }); + requestIdleCallback( + () => { + checkItem() + .then(resolve) + .catch(() => {}); + }, + { timeout: 1000 }, + ); + } else { + resolve(); + } + }); + + return checkItem(); }; export const setInlineDiffViewType = ({ commit }) => { diff --git a/app/assets/javascripts/diffs/store/getters.js b/app/assets/javascripts/diffs/store/getters.js index 4a47646d7fa..968ba3c5e13 100644 --- a/app/assets/javascripts/diffs/store/getters.js +++ b/app/assets/javascripts/diffs/store/getters.js @@ -17,7 +17,10 @@ export const commitId = state => (state.commit && state.commit.id ? state.commit export const diffHasAllExpandedDiscussions = (state, getters) => diff => { const discussions = getters.getDiffFileDiscussions(diff); - return (discussions.length && discussions.every(discussion => discussion.expanded)) || false; + return ( + (discussions && discussions.length && discussions.every(discussion => discussion.expanded)) || + false + ); }; /** @@ -28,7 +31,10 @@ export const diffHasAllExpandedDiscussions = (state, getters) => diff => { export const diffHasAllCollpasedDiscussions = (state, getters) => diff => { const discussions = getters.getDiffFileDiscussions(diff); - return (discussions.length && discussions.every(discussion => !discussion.expanded)) || false; + return ( + (discussions && discussions.length && discussions.every(discussion => !discussion.expanded)) || + false + ); }; /** @@ -40,7 +46,9 @@ export const diffHasExpandedDiscussions = (state, getters) => diff => { const discussions = getters.getDiffFileDiscussions(diff); return ( - (discussions.length && discussions.find(discussion => discussion.expanded) !== undefined) || + (discussions && + discussions.length && + discussions.find(discussion => discussion.expanded) !== undefined) || false ); }; @@ -64,45 +72,38 @@ export const getDiffFileDiscussions = (state, getters, rootState, rootGetters) = discussion.diff_discussion && _.isEqual(discussion.diff_file.file_hash, diff.fileHash), ) || []; -export const singleDiscussionByLineCode = (state, getters, rootState, rootGetters) => lineCode => { - if (!lineCode || lineCode === undefined) return []; - const discussions = rootGetters.discussionsByLineCode; - return discussions[lineCode] || []; -}; +export const shouldRenderParallelCommentRow = state => line => { + const hasDiscussion = + (line.left && line.left.discussions && line.left.discussions.length) || + (line.right && line.right.discussions && line.right.discussions.length); -export const shouldRenderParallelCommentRow = (state, getters) => line => { - const leftLineCode = line.left.lineCode; - const rightLineCode = line.right.lineCode; - const leftDiscussions = getters.singleDiscussionByLineCode(leftLineCode); - const rightDiscussions = getters.singleDiscussionByLineCode(rightLineCode); - const hasDiscussion = leftDiscussions.length || rightDiscussions.length; - - const hasExpandedDiscussionOnLeft = leftDiscussions.length - ? leftDiscussions.every(discussion => discussion.expanded) - : false; - const hasExpandedDiscussionOnRight = rightDiscussions.length - ? rightDiscussions.every(discussion => discussion.expanded) - : false; + const hasExpandedDiscussionOnLeft = + line.left && line.left.discussions && line.left.discussions.length + ? line.left.discussions.every(discussion => discussion.expanded) + : false; + const hasExpandedDiscussionOnRight = + line.right && line.right.discussions && line.right.discussions.length + ? line.right.discussions.every(discussion => discussion.expanded) + : false; if (hasDiscussion && (hasExpandedDiscussionOnLeft || hasExpandedDiscussionOnRight)) { return true; } - const hasCommentFormOnLeft = state.diffLineCommentForms[leftLineCode]; - const hasCommentFormOnRight = state.diffLineCommentForms[rightLineCode]; + const hasCommentFormOnLeft = line.left && state.diffLineCommentForms[line.left.lineCode]; + const hasCommentFormOnRight = line.right && state.diffLineCommentForms[line.right.lineCode]; return hasCommentFormOnLeft || hasCommentFormOnRight; }; -export const shouldRenderInlineCommentRow = (state, getters) => line => { +export const shouldRenderInlineCommentRow = state => line => { if (state.diffLineCommentForms[line.lineCode]) return true; - const lineDiscussions = getters.singleDiscussionByLineCode(line.lineCode); - if (lineDiscussions.length === 0) { + if (!line.discussions || line.discussions.length === 0) { return false; } - return lineDiscussions.every(discussion => discussion.expanded); + return line.discussions.every(discussion => discussion.expanded); }; // prevent babel-plugin-rewire from generating an invalid default during karma∂ tests diff --git a/app/assets/javascripts/diffs/store/mutation_types.js b/app/assets/javascripts/diffs/store/mutation_types.js index c999d637d50..f61efbe6e1e 100644 --- a/app/assets/javascripts/diffs/store/mutation_types.js +++ b/app/assets/javascripts/diffs/store/mutation_types.js @@ -9,3 +9,5 @@ export const ADD_CONTEXT_LINES = 'ADD_CONTEXT_LINES'; export const ADD_COLLAPSED_DIFFS = 'ADD_COLLAPSED_DIFFS'; export const EXPAND_ALL_FILES = 'EXPAND_ALL_FILES'; export const RENDER_FILE = 'RENDER_FILE'; +export const SET_LINE_DISCUSSIONS_FOR_FILE = 'SET_LINE_DISCUSSIONS_FOR_FILE'; +export const REMOVE_LINE_DISCUSSIONS_FOR_FILE = 'REMOVE_LINE_DISCUSSIONS_FOR_FILE'; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index 0522e32c410..6dc5bf16c65 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -1,8 +1,13 @@ import Vue from 'vue'; -import _ from 'underscore'; import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; -import { findDiffFile, addLineReferences, removeMatchLine, addContextLines } from './utils'; -import { LINES_TO_BE_RENDERED_DIRECTLY, MAX_LINES_TO_BE_RENDERED } from '../constants'; +import { + findDiffFile, + addLineReferences, + removeMatchLine, + addContextLines, + prepareDiffData, + isDiscussionApplicableToLine, +} from './utils'; import * as types from './mutation_types'; export default { @@ -17,38 +22,7 @@ export default { [types.SET_DIFF_DATA](state, data) { const diffData = convertObjectPropsToCamelCase(data, { deep: true }); - let showingLines = 0; - const filesLength = diffData.diffFiles.length; - let i; - for (i = 0; i < filesLength; i += 1) { - const file = diffData.diffFiles[i]; - if (file.parallelDiffLines) { - const linesLength = file.parallelDiffLines.length; - let u = 0; - for (u = 0; u < linesLength; u += 1) { - const line = file.parallelDiffLines[u]; - if (line.left) delete line.left.text; - if (line.right) delete line.right.text; - } - } - - if (file.highlightedDiffLines) { - const linesLength = file.highlightedDiffLines.length; - let u; - for (u = 0; u < linesLength; u += 1) { - const line = file.highlightedDiffLines[u]; - delete line.text; - } - } - - if (file.highlightedDiffLines) { - showingLines += file.parallelDiffLines.length; - } - Object.assign(file, { - renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, - collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, - }); - } + prepareDiffData(diffData); Object.assign(state, { ...diffData, @@ -98,19 +72,93 @@ export default { [types.ADD_COLLAPSED_DIFFS](state, { file, data }) { const normalizedData = convertObjectPropsToCamelCase(data, { deep: true }); + prepareDiffData(normalizedData); const [newFileData] = normalizedData.diffFiles.filter(f => f.fileHash === file.fileHash); - - if (newFileData) { - const index = _.findIndex(state.diffFiles, f => f.fileHash === file.fileHash); - state.diffFiles.splice(index, 1, newFileData); - } + const selectedFile = state.diffFiles.find(f => f.fileHash === file.fileHash); + Object.assign(selectedFile, { ...newFileData }); }, [types.EXPAND_ALL_FILES](state) { - // eslint-disable-next-line no-param-reassign state.diffFiles = state.diffFiles.map(file => ({ ...file, collapsed: false, })); }, + + [types.SET_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, discussions, diffPositionByLineCode }) { + const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); + const firstDiscussion = discussions[0]; + const isDiffDiscussion = firstDiscussion.diff_discussion; + const hasLineCode = firstDiscussion.line_code; + const isResolvable = firstDiscussion.resolvable; + const diffPosition = diffPositionByLineCode[firstDiscussion.line_code]; + + if ( + selectedFile && + isDiffDiscussion && + hasLineCode && + isResolvable && + diffPosition && + isDiscussionApplicableToLine(firstDiscussion, diffPosition) + ) { + const targetLine = selectedFile.parallelDiffLines.find( + line => + (line.left && line.left.lineCode === firstDiscussion.line_code) || + (line.right && line.right.lineCode === firstDiscussion.line_code), + ); + if (targetLine) { + if (targetLine.left && targetLine.left.lineCode === firstDiscussion.line_code) { + Object.assign(targetLine.left, { + discussions, + }); + } else { + Object.assign(targetLine.right, { + discussions, + }); + } + } + + if (selectedFile.highlightedDiffLines) { + const targetInlineLine = selectedFile.highlightedDiffLines.find( + line => line.lineCode === firstDiscussion.line_code, + ); + + if (targetInlineLine) { + Object.assign(targetInlineLine, { + discussions, + }); + } + } + } + }, + + [types.REMOVE_LINE_DISCUSSIONS_FOR_FILE](state, { fileHash, lineCode }) { + const selectedFile = state.diffFiles.find(f => f.fileHash === fileHash); + if (selectedFile) { + const targetLine = selectedFile.parallelDiffLines.find( + line => + (line.left && line.left.lineCode === lineCode) || + (line.right && line.right.lineCode === lineCode), + ); + if (targetLine) { + const side = targetLine.left && targetLine.left.lineCode === lineCode ? 'left' : 'right'; + + Object.assign(targetLine[side], { + discussions: [], + }); + } + + if (selectedFile.highlightedDiffLines) { + const targetInlineLine = selectedFile.highlightedDiffLines.find( + line => line.lineCode === lineCode, + ); + + if (targetInlineLine) { + Object.assign(targetInlineLine, { + discussions: [], + }); + } + } + } + }, }; diff --git a/app/assets/javascripts/diffs/store/utils.js b/app/assets/javascripts/diffs/store/utils.js index 82082ac508a..b7e52a8f37f 100644 --- a/app/assets/javascripts/diffs/store/utils.js +++ b/app/assets/javascripts/diffs/store/utils.js @@ -8,6 +8,8 @@ import { NEW_LINE_TYPE, OLD_LINE_TYPE, MATCH_LINE_TYPE, + LINES_TO_BE_RENDERED_DIRECTLY, + MAX_LINES_TO_BE_RENDERED, } from '../constants'; export function findDiffFile(files, hash) { @@ -161,6 +163,11 @@ export function addContextLines(options) { * @returns {Object} */ export function trimFirstCharOfLineContent(line = {}) { + // eslint-disable-next-line no-param-reassign + delete line.text; + // eslint-disable-next-line no-param-reassign + line.discussions = []; + const parsedLine = Object.assign({}, line); if (line.richText) { @@ -174,7 +181,44 @@ export function trimFirstCharOfLineContent(line = {}) { return parsedLine; } -export function getDiffRefsByLineCode(diffFiles) { +// This prepares and optimizes the incoming diff data from the server +// by setting up incremental rendering and removing unneeded data +export function prepareDiffData(diffData) { + const filesLength = diffData.diffFiles.length; + let showingLines = 0; + for (let i = 0; i < filesLength; i += 1) { + const file = diffData.diffFiles[i]; + + if (file.parallelDiffLines) { + const linesLength = file.parallelDiffLines.length; + for (let u = 0; u < linesLength; u += 1) { + const line = file.parallelDiffLines[u]; + if (line.left) { + line.left = trimFirstCharOfLineContent(line.left); + } + if (line.right) { + line.right = trimFirstCharOfLineContent(line.right); + } + } + } + + if (file.highlightedDiffLines) { + const linesLength = file.highlightedDiffLines.length; + for (let u = 0; u < linesLength; u += 1) { + const line = file.highlightedDiffLines[u]; + Object.assign(line, { ...trimFirstCharOfLineContent(line) }); + } + showingLines += file.parallelDiffLines.length; + } + + Object.assign(file, { + renderIt: showingLines < LINES_TO_BE_RENDERED_DIRECTLY, + collapsed: file.text && showingLines > MAX_LINES_TO_BE_RENDERED, + }); + } +} + +export function getDiffPositionByLineCode(diffFiles) { return diffFiles.reduce((acc, diffFile) => { const { baseSha, headSha, startSha } = diffFile.diffRefs; const { newPath, oldPath } = diffFile; @@ -194,3 +238,12 @@ export function getDiffRefsByLineCode(diffFiles) { return acc; }, {}); } + +// This method will check whether the discussion is still applicable +// to the diff line in question regarding different versions of the MR +export function isDiscussionApplicableToLine(discussion, diffPosition) { + const originalRefs = convertObjectPropsToCamelCase(discussion.original_position.formatter); + const refs = convertObjectPropsToCamelCase(discussion.position.formatter); + + return _.isEqual(refs, diffPosition) || _.isEqual(originalRefs, diffPosition); +} diff --git a/app/assets/javascripts/clusters/components/gcp_signup_offer.js b/app/assets/javascripts/dismissable_callout.js similarity index 83% rename from app/assets/javascripts/clusters/components/gcp_signup_offer.js rename to app/assets/javascripts/dismissable_callout.js index 8bc20a1c09f..5185b019376 100644 --- a/app/assets/javascripts/clusters/components/gcp_signup_offer.js +++ b/app/assets/javascripts/dismissable_callout.js @@ -3,8 +3,8 @@ import axios from '~/lib/utils/axios_utils'; import { __ } from '~/locale'; import Flash from '~/flash'; -export default function gcpSignupOffer() { - const alertEl = document.querySelector('.gcp-signup-offer'); +export default function initDismissableCallout(alertSelector) { + const alertEl = document.querySelector(alertSelector); if (!alertEl) { return; } diff --git a/app/assets/javascripts/dropzone_input.js b/app/assets/javascripts/dropzone_input.js index 5528ad9f38d..ff969bb94a4 100644 --- a/app/assets/javascripts/dropzone_input.js +++ b/app/assets/javascripts/dropzone_input.js @@ -7,6 +7,19 @@ import axios from './lib/utils/axios_utils'; Dropzone.autoDiscover = false; +/** + * Return the error message string from the given response. + * + * @param {String|Object} res + */ +function getErrorMessage(res) { + if (!res || _.isString(res)) { + return res; + } + + return res.message; +} + export default function dropzoneInput(form) { const divHover = '
'; const iconPaperclip = ''; @@ -18,7 +31,7 @@ export default function dropzoneInput(form) { const $uploadingErrorContainer = form.find('.uploading-error-container'); const $uploadingErrorMessage = form.find('.uploading-error-message'); const $uploadingProgressContainer = form.find('.uploading-progress-container'); - const uploadsPath = window.uploads_path || null; + const uploadsPath = form.data('uploads-path') || window.uploads_path || null; const maxFileSize = gon.max_file_size || 10; const formTextarea = form.find('.js-gfm-input'); let handlePaste; @@ -42,7 +55,7 @@ export default function dropzoneInput(form) { if (!uploadsPath) { $formDropzone.addClass('js-invalid-dropzone'); - return; + return null; } const dropzone = $formDropzone.dropzone({ @@ -84,9 +97,7 @@ export default function dropzoneInput(form) { // xhr object (xhr.responseText is error message). // On error we hide the 'Attach' and 'Cancel' buttons // and show an error. - - // If there's xhr error message, let's show it instead of dropzone's one. - const message = xhr ? xhr.responseText : errorMessage; + const message = getErrorMessage(errorMessage || xhr.responseText); $uploadingErrorContainer.removeClass('hide'); $uploadingErrorMessage.html(message); @@ -274,4 +285,6 @@ export default function dropzoneInput(form) { $(this).closest('.gfm-form').find('.div-dropzone').click(); formTextarea.focus(); }); + + return Dropzone.forElement($formDropzone.get(0)); } diff --git a/app/assets/javascripts/fly_out_nav.js b/app/assets/javascripts/fly_out_nav.js index 8b4f3b05ee7..f820f0dc3f0 100644 --- a/app/assets/javascripts/fly_out_nav.js +++ b/app/assets/javascripts/fly_out_nav.js @@ -65,8 +65,8 @@ export const hideMenu = (el) => { const parentEl = el.parentNode; - el.style.display = ''; // eslint-disable-line no-param-reassign - el.style.transform = ''; // eslint-disable-line no-param-reassign + el.style.display = ''; + el.style.transform = ''; el.classList.remove(IS_ABOVE_CLASS); parentEl.classList.remove(IS_OVER_CLASS); parentEl.classList.remove(IS_SHOWING_FLY_OUT_CLASS); diff --git a/app/assets/javascripts/groups/components/app.vue b/app/assets/javascripts/groups/components/app.vue index b0765747a36..69f192ac75e 100644 --- a/app/assets/javascripts/groups/components/app.vue +++ b/app/assets/javascripts/groups/components/app.vue @@ -2,14 +2,15 @@ /* global Flash */ import $ from 'jquery'; -import { s__ } from '~/locale'; +import { s__, sprintf } from '~/locale'; import loadingIcon from '~/vue_shared/components/loading_icon.vue'; import DeprecatedModal from '~/vue_shared/components/deprecated_modal.vue'; +import { HIDDEN_CLASS } from '~/lib/utils/constants'; import { getParameterByName } from '~/lib/utils/common_utils'; import { mergeUrlParams } from '~/lib/utils/url_utility'; import eventHub from '../event_hub'; -import { COMMON_STR } from '../constants'; +import { COMMON_STR, CONTENT_LIST_CLASS } from '../constants'; import groupsComponent from './groups.vue'; export default { @@ -19,6 +20,16 @@ export default { groupsComponent, }, props: { + action: { + type: String, + required: false, + default: '', + }, + containerId: { + type: String, + required: false, + default: '', + }, store: { type: Object, required: true, @@ -56,31 +67,28 @@ export default { ? COMMON_STR.GROUP_SEARCH_EMPTY : COMMON_STR.GROUP_PROJECT_SEARCH_EMPTY; - eventHub.$on('fetchPage', this.fetchPage); - eventHub.$on('toggleChildren', this.toggleChildren); - eventHub.$on('showLeaveGroupModal', this.showLeaveGroupModal); - eventHub.$on('updatePagination', this.updatePagination); - eventHub.$on('updateGroups', this.updateGroups); + eventHub.$on(`${this.action}fetchPage`, this.fetchPage); + eventHub.$on(`${this.action}toggleChildren`, this.toggleChildren); + eventHub.$on(`${this.action}showLeaveGroupModal`, this.showLeaveGroupModal); + eventHub.$on(`${this.action}updatePagination`, this.updatePagination); + eventHub.$on(`${this.action}updateGroups`, this.updateGroups); }, mounted() { this.fetchAllGroups(); + + if (this.containerId) { + this.containerEl = document.getElementById(this.containerId); + } }, beforeDestroy() { - eventHub.$off('fetchPage', this.fetchPage); - eventHub.$off('toggleChildren', this.toggleChildren); - eventHub.$off('showLeaveGroupModal', this.showLeaveGroupModal); - eventHub.$off('updatePagination', this.updatePagination); - eventHub.$off('updateGroups', this.updateGroups); + eventHub.$off(`${this.action}fetchPage`, this.fetchPage); + eventHub.$off(`${this.action}toggleChildren`, this.toggleChildren); + eventHub.$off(`${this.action}showLeaveGroupModal`, this.showLeaveGroupModal); + eventHub.$off(`${this.action}updatePagination`, this.updatePagination); + eventHub.$off(`${this.action}updateGroups`, this.updateGroups); }, methods: { - fetchGroups({ - parentId, - page, - filterGroupsBy, - sortBy, - archived, - updatePagination, - }) { + fetchGroups({ parentId, page, filterGroupsBy, sortBy, archived, updatePagination }) { return this.service .getGroups(parentId, page, filterGroupsBy, sortBy, archived) .then(res => { @@ -165,13 +173,13 @@ export default { } }, showLeaveGroupModal(group, parentGroup) { + const { fullName } = group; this.targetGroup = group; this.targetParentGroup = parentGroup; this.showModal = true; - this.groupLeaveConfirmationMessage = s__( - `GroupsTree|Are you sure you want to leave the "${ - group.fullName - }" group?`, + this.groupLeaveConfirmationMessage = sprintf( + s__('GroupsTree|Are you sure you want to leave the "%{fullName}" group?'), + { fullName }, ); }, hideLeaveGroupModal() { @@ -197,16 +205,35 @@ export default { this.targetGroup.isBeingRemoved = false; }); }, + showEmptyState() { + const { containerEl } = this; + const contentListEl = containerEl.querySelector(CONTENT_LIST_CLASS); + const emptyStateEl = containerEl.querySelector('.empty-state'); + + if (contentListEl) { + contentListEl.remove(); + } + + if (emptyStateEl) { + emptyStateEl.classList.remove(HIDDEN_CLASS); + } + }, updatePagination(headers) { this.store.setPaginationInfo(headers); }, updateGroups(groups, fromSearch) { - this.isSearchEmpty = groups ? groups.length === 0 : false; + const hasGroups = groups && groups.length > 0; + this.isSearchEmpty = !hasGroups; + if (fromSearch) { this.store.setSearchedGroups(groups); } else { this.store.setGroups(groups); } + + if (this.action && !hasGroups && !fromSearch) { + this.showEmptyState(); + } }, }, }; @@ -226,6 +253,7 @@ export default { :search-empty="isSearchEmpty" :search-empty-message="searchEmptyMessage" :page-info="pageInfo" + :action="action" /> ([]), + default: '', }, }, computed: { @@ -37,6 +41,7 @@ export default { :key="index" :group="group" :parent-group="parentGroup" + :action="action" />
  • diff --git a/app/assets/javascripts/groups/components/groups.vue b/app/assets/javascripts/groups/components/groups.vue index 73ae928b0d9..a1beb222950 100644 --- a/app/assets/javascripts/groups/components/groups.vue +++ b/app/assets/javascripts/groups/components/groups.vue @@ -1,39 +1,44 @@ diff --git a/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue b/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue index 391004dcd3c..10c78a80302 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/list_item.vue @@ -2,6 +2,7 @@ import { mapActions } from 'vuex'; import tooltip from '~/vue_shared/directives/tooltip'; import Icon from '~/vue_shared/components/icon.vue'; +import FileIcon from '~/vue_shared/components/file_icon.vue'; import StageButton from './stage_button.vue'; import UnstageButton from './unstage_button.vue'; import { viewerTypes } from '../../constants'; @@ -12,6 +13,7 @@ export default { Icon, StageButton, UnstageButton, + FileIcon, }, directives: { tooltip, @@ -48,7 +50,7 @@ export default { return `${getCommitIconMap(this.file).icon}${suffix}`; }, iconClass() { - return `${getCommitIconMap(this.file).class} append-right-8`; + return `${getCommitIconMap(this.file).class} ml-auto mr-auto`; }, fullKey() { return `${this.keyPrefix}-${this.file.key}`; @@ -105,17 +107,24 @@ export default { @click="openFileInEditor" > - {{ file.name }} +
    +
    + +
    + +
    - diff --git a/app/assets/javascripts/ide/components/commit_sidebar/stage_button.vue b/app/assets/javascripts/ide/components/commit_sidebar/stage_button.vue index e6044401c9f..8a1836a5c92 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/stage_button.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/stage_button.vue @@ -1,11 +1,15 @@ @@ -25,51 +43,50 @@ export default { diff --git a/app/assets/javascripts/ide/components/commit_sidebar/unstage_button.vue b/app/assets/javascripts/ide/components/commit_sidebar/unstage_button.vue index 9cec73ec00e..86c40602074 100644 --- a/app/assets/javascripts/ide/components/commit_sidebar/unstage_button.vue +++ b/app/assets/javascripts/ide/components/commit_sidebar/unstage_button.vue @@ -25,22 +25,23 @@ export default {