Merge branch 'ce-to-ee-2018-12-05' into 'master'

CE upstream - 2018-12-05 21:21 UTC

Closes gitlab-ce#54868, gitlab-ce#50662, and #8195

See merge request gitlab-org/gitlab-ee!8715

.babelrc.js

@@ -35,4 +35,10 @@ if (BABEL_ENV === 'karma' || BABEL_ENV === 'coverage') {
   plugins.push('babel-plugin-rewire');
 }
 
+// Jest is running in node environment
+if (BABEL_ENV === 'jest') {
+  plugins.push('transform-es2015-modules-commonjs');
+  plugins.push('dynamic-import-node');
+}
+
 module.exports = { presets, plugins };

.eslintignore

@@ -2,6 +2,7 @@
 /config/
 /builds/
 /coverage/
+/coverage-frontend/
 /coverage-javascript/
 /node_modules/
 /public/

.gitignore

@@ -78,5 +78,5 @@ eslint-report.html
 /plugins/*
 /.gitlab_pages_secret
 package-lock.json
-/junit_rspec.xml
-/junit_karma.xml
+/junit_*.xml
+/coverage-frontend/

.gitlab-ci.yml

@@ -877,6 +877,32 @@ karma:
     reports:
       junit: junit_karma.xml
 
+jest:
+  <<: *dedicated-no-docs-and-no-qa-pull-cache-job
+  <<: *use-pg
+  dependencies:
+  - compile-assets
+  - setup-test-env
+  script:
+    - scripts/gitaly-test-spawn
+    - date
+    - bundle exec rake karma:fixtures
+    - date
+    - yarn jest --ci --coverage
+  artifacts:
+    name: coverage-frontend
+    expire_in: 31d
+    when: always
+    paths:
+    - coverage-frontend/
+    - junit_jest.xml
+    reports:
+      junit: junit_jest.xml
+  cache:
+    key: jest
+    paths:
+      - tmp/jest/jest/
+
 code_quality:
   <<: *dedicated-no-docs-no-db-pull-cache-job
   image: docker:stable

app/assets/javascripts/diffs/store/actions.js

@@ -147,8 +147,14 @@ export const scrollToLineIfNeededParallel = (_, line) => {
   }
 };
 
-export const loadCollapsedDiff = ({ commit }, file) =>
-  axios.get(file.load_collapsed_diff_url).then(res => {
+export const loadCollapsedDiff = ({ commit, getters }, file) =>
+  axios
+    .get(file.load_collapsed_diff_url, {
+      params: {
+        commit_id: getters.commitId,
+      },
+    })
+    .then(res => {
       commit(types.ADD_COLLAPSED_DIFFS, {
         file,
         data: res.data,

app/assets/javascripts/diffs/store/mutations.js

@@ -130,7 +130,7 @@ export default {
 
         if (file.highlighted_diff_lines) {
           file.highlighted_diff_lines = file.highlighted_diff_lines.map(line => {
-            if (lineCheck(line)) {
+            if (!line.discussions.some(({ id }) => discussion.id === id) && lineCheck(line)) {
               return {
                 ...line,
                 discussions: line.discussions.concat(discussion),
@@ -150,11 +150,17 @@ export default {
               return {
                 left: {
                   ...line.left,
-                  discussions: left ? line.left.discussions.concat(discussion) : [],
+                  discussions:
+                    left && !line.left.discussions.some(({ id }) => id === discussion.id)
+                      ? line.left.discussions.concat(discussion)
+                      : (line.left && line.left.discussions) || [],
                 },
                 right: {
                   ...line.right,
-                  discussions: right && !left ? line.right.discussions.concat(discussion) : [],
+                  discussions:
+                    right && !left && !line.right.discussions.some(({ id }) => id === discussion.id)
+                      ? line.right.discussions.concat(discussion)
+                      : (line.right && line.right.discussions) || [],
                 },
               };
             }

app/assets/stylesheets/framework/variables_overrides.scss

@@ -19,3 +19,4 @@ $info: $blue-500;
 $warning: $orange-500;
 $danger: $red-500;
 $zindex-modal-backdrop: 1040;
+$nav-divider-margin-y: ($grid-size / 2);

app/controllers/projects/merge_requests/diffs_controller.rb

@@ -24,12 +24,9 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
 
   def render_diffs
     @environment = @merge_request.environments_for(current_user).last
-    notes_grouped_by_path = renderable_notes.group_by { |note| note.position.file_path }
 
-    @diffs.diff_files.each do |diff_file|
-      notes = notes_grouped_by_path.fetch(diff_file.file_path, [])
-      notes.each { |note| diff_file.unfold_diff_lines(note.position) }
-    end
+    note_positions = renderable_notes.map(&:position).compact
+    @diffs.unfold_diff_files(note_positions)
 
     @diffs.write_cache
 

app/models/ci/runner.rb

@@ -114,7 +114,8 @@ module Ci
 
     cached_attr_reader :version, :revision, :platform, :architecture, :ip_address, :contacted_at
 
-    chronic_duration_attr :maximum_timeout_human_readable, :maximum_timeout
+    chronic_duration_attr :maximum_timeout_human_readable, :maximum_timeout,
+        error_message: 'Maximum job timeout has a value which could not be accepted'
 
     validates :maximum_timeout, allow_nil: true,
                                 numericality: { greater_than_or_equal_to: 600,

app/models/concerns/chronic_duration_attribute.rb

@@ -24,7 +24,7 @@ module ChronicDurationAttribute
         end
       end
 
-      validates virtual_attribute, allow_nil: true, duration: true
+      validates virtual_attribute, allow_nil: true, duration: { message: parameters[:error_message] }
     end
 
     alias_method :chronic_duration_attr, :chronic_duration_attr_writer

app/models/project.rb

@@ -404,7 +404,8 @@ class Project < ActiveRecord::Base
 
   enum auto_cancel_pending_pipelines: { disabled: 0, enabled: 1 }
 
-  chronic_duration_attr :build_timeout_human_readable, :build_timeout, default: 3600
+  chronic_duration_attr :build_timeout_human_readable, :build_timeout,
+    default: 3600, error_message: 'Maximum job timeout has a value which could not be accepted'
 
   validates :build_timeout, allow_nil: true,
                             numericality: { greater_than_or_equal_to: 10.minutes,

app/validators/duration_validator.rb

@@ -14,6 +14,10 @@ class DurationValidator < ActiveModel::EachValidator
   def validate_each(record, attribute, value)
     ChronicDuration.parse(value)
   rescue ChronicDuration::DurationParseError
+    if options[:message]
+      record.errors.add(:base, options[:message])
+    else
       record.errors.add(attribute, "is not a correct duration")
     end
+  end
 end

app/views/projects/imports/show.html.haml

 - page_title import_in_progress_title
+- @no_container = true
+- @content_class = "limit-container-width" unless fluid_layout
 
 .save-project-loader
   .center

app/views/projects/settings/ci_cd/_form.html.haml

@@ -29,7 +29,7 @@
           = f.label :build_timeout_human_readable, _('Timeout'), class: 'label-bold'
           = f.text_field :build_timeout_human_readable, class: 'form-control'
           %p.form-text.text-muted
-            = _("Per job. If a job passes this threshold, it will be marked as failed")
+            = _('If any job surpasses this timeout threshold, it will be marked as failed. Human readable time input language is accepted like "1 hour". Values without specification represent seconds.')
             = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'timeout'), target: '_blank'
 
         %hr

app/views/shared/runners/_form.html.haml

@@ -45,7 +45,7 @@
       = _('Maximum job timeout')
     .col-sm-10
       = f.text_field :maximum_timeout_human_readable, class: 'form-control'
-      .form-text.text-muted= _('This timeout will take precedence when lower than Project-defined timeout')
+      .form-text.text-muted= _('This timeout will take precedence when lower than project-defined timeout and accepts a human readable time input language like "1 hour". Values without specification represent seconds.')
   .form-group.row
     = label_tag :tag_list, class: 'col-form-label col-sm-2' do
       = _('Tags')

changelogs/unreleased/54826-use-read_repository-scope-on-read-only-files-endpoints.yml

+---
+title: Use read_repository scope on read-only files API
+merge_request: 23534
+author:
+type: fixed

changelogs/unreleased/diff-expand-commit-file.yml

+---
+title: Fixed diff files expanding not loading commit content
+merge_request:
+author:
+type: fixed

changelogs/unreleased/fix-gb-improve-timeout-inputs-help-sections.yml

+---
+title: Improve help and validation sections of maximum build timeout inputs
+merge_request: 23586
+author:
+type: fixed

changelogs/unreleased/gt-change-container-width-for-project-import.yml

+---
+title: Change container width for project import
+merge_request: 23318
+author: George Tsiolis
+type: fixed

changelogs/unreleased/multiple-diff-line-discussions-fix.yml

+---
+title: Fixed duplicate discussions getting added to diff lines
+merge_request:
+author:
+type: fixed

changelogs/unreleased/osw-fix-grouping-by-file-path.yml

+---
+title: Avoid 500's when serializing legacy diff notes
+merge_request: 23544
+author:
+type: fixed

changelogs/unreleased/winh-divider-margin.yml

+---
+title: Adjust divider margin to comply with design specs
+merge_request: 23548
+author:
+type: changed

config/jest.config.js

+/* eslint-disable filenames/match-regex */
+
+const reporters = ['default'];
+
+if (process.env.CI) {
+  reporters.push([
+    'jest-junit',
+    {
+      output: './junit_jest.xml',
+    },
+  ]);
+}
+
+// eslint-disable-next-line import/no-commonjs
+module.exports = {
+  testMatch: ['<rootDir>/spec/frontend/**/*_spec.js'],
+  moduleNameMapper: {
+    '^~(.*)$': '<rootDir>/app/assets/javascripts$1',
+  },
+  collectCoverageFrom: ['<rootDir>/app/assets/javascripts/**/*.{js,vue}'],
+  coverageDirectory: '<rootDir>/coverage-frontend/',
+  coverageReporters: ['json', 'lcov', 'text-summary', 'clover'],
+  cacheDirectory: '<rootDir>/tmp/cache/jest',
+  modulePathIgnorePatterns: ['<rootDir>/.yarn-cache/'],
+  reporters,
+  rootDir: '..', // necessary because this file is in the config/ subdirectory
+};

danger/changelog/Dangerfile

@@ -2,7 +2,7 @@
 
 require 'yaml'
 
-NO_CHANGELOG_LABELS = %w[backstage Documentation QA test].freeze
+NO_CHANGELOG_LABELS = %w[backstage ci-build Documentation meta QA test].freeze
 SEE_DOC = "See [the documentation](https://docs.gitlab.com/ce/development/changelog.html).".freeze
 CREATE_CHANGELOG_MESSAGE = <<~MSG.freeze
 You can create one with:

doc/api/repository_files.md

@@ -4,6 +4,16 @@
 
 **Create, read, update and delete repository files using this API**
 
+The different scopes available using [personal access tokens](../user/profile/personal_access_tokens.md) are depicted
+in the following table.
+
+| Scope | Description |
+| ----- | ----------- |
+| `read_repository` | Allows read-access to the repository files. |
+| `api` | Allows read-write access to the repository files. |
+
+> `read_repository` scope was [introduced](https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/23534) in GitLab 11.6.
+
 ## Get file from repository
 
 Allows you to receive information about file in repository like name, size,

doc/development/i18n/proofreader.md

@@ -57,12 +57,14 @@ are very appreciative of the work done by translators and proofreaders!
   - Chang-Ho Cha - [GitLab](https://gitlab.com/changho-cha), [Crowdin](https://crowdin.com/profile/zzazang)
   - Huang Tao - [GitLab](https://gitlab.com/htve), [Crowdin](https://crowdin.com/profile/htve)
   - Ji Hun Oh - [GitLab](https://gitlab.com/Baw-Appie), [Crowdin](https://crowdin.com/profile/BawAppie)
+  - Jeongwhan Choi - [GitLab](https://gitlab.com/jeongwhanchoi), [Crowdin](https://crowdin.com/profile/jeongwhanchoi)
 - Mongolian
   - Proofreaders needed.
 - Norwegian Bokmal
   - Proofreaders needed.
 - Polish
   - Filip Mech - [GitLab](https://gitlab.com/mehenz), [Crowdin](https://crowdin.com/profile/mehenz)
+  - Maksymilian Roman - [GitLab](https://gitlab.com/villaincandle), [Crowdin](https://crowdin.com/profile/villaincandle)
 - Portuguese
   - Proofreaders needed.
 - Portuguese, Brazilian

doc/development/new_fe_guide/development/testing.md

@@ -6,9 +6,15 @@ Tests relevant for frontend development can be found at two places:
   - [frontend unit tests](#frontend-unit-tests)
   - [frontend component tests](#frontend-component-tests)
   - [frontend integration tests](#frontend-integration-tests)
+- `spec/frontend/` which are run by Jest and contain
+  - [frontend unit tests](#frontend-unit-tests)
+  - [frontend component tests](#frontend-component-tests)
+  - [frontend integration tests](#frontend-integration-tests)
 - `spec/features/` which are run by RSpec and contain
   - [feature tests](#feature-tests)
 
+All tests in `spec/javascripts/` will eventually be migrated to `spec/frontend/` (see also [#53757]).
+
 In addition there were feature tests in `features/` run by Spinach in the past.
 These have been removed from our codebase in May 2018 ([#23036](https://gitlab.com/gitlab-org/gitlab-ce/issues/23036)).
 
@@ -17,6 +23,8 @@ See also:
 - [old testing guide](../../testing_guide/frontend_testing.html)
 - [notes on testing Vue components](../../fe_guide/vue.html#testing-vue-components)
 
+[#53757]: https://gitlab.com/gitlab-org/gitlab-ce/issues/53757
+
 ## Frontend unit tests
 
 Unit tests are on the lowest abstraction level and typically test functionality that is not directly perceivable by a user.

doc/install/docker.md

@@ -7,9 +7,10 @@ GitLab provides official Docker images to allowing you to easily take advantage
 ## Omnibus GitLab based images
 
 GitLab maintains a set of [official Docker images](https://hub.docker.com/r/gitlab) based on our [Omnibus GitLab package](https://docs.gitlab.com/omnibus/README.html). These images include:
-* [GitLab Community Edition](https://hub.docker.com/r/gitlab/gitlab-ce/)
-* [GitLab Enterprise Edition](https://hub.docker.com/r/gitlab/gitlab-ee/)
-* [GitLab Runner](https://hub.docker.com/r/gitlab/gitlab-runner/)
+
+- [GitLab Community Edition](https://hub.docker.com/r/gitlab/gitlab-ce/).
+- [GitLab Enterprise Edition](https://hub.docker.com/r/gitlab/gitlab-ee/).
+- [GitLab Runner](https://hub.docker.com/r/gitlab/gitlab-runner/).
 
 A [complete usage guide](https://docs.gitlab.com/omnibus/docker/) to these images is available, as well as the [Dockerfile used for building the images](https://gitlab.com/gitlab-org/omnibus-gitlab/tree/master/docker).
 

doc/security/two_factor_authentication.md

@@ -6,7 +6,7 @@ password to login, they'll be prompted for a code generated by an application on
 their phone.
 
 You can read more about it here:
-[Two-factor Authentication (2FA)](../profile/two_factor_authentication.md)
+[Two-factor Authentication (2FA)](../user/profile/account/two_factor_authentication.md)
 
 ## Enforcing 2FA for all users
 

lib/api/files.rb

@@ -2,6 +2,8 @@
 
 module API
   class Files < Grape::API
+    include APIGuard
+
     FILE_ENDPOINT_REQUIREMENTS = API::NAMESPACE_OR_PROJECT_REQUIREMENTS.merge(file_path: API::NO_SLASH_URL_PART_REGEX)
 
     # Prevents returning plain/text responses for files with .txt extension
@@ -79,6 +81,8 @@ module API
       requires :id, type: String, desc: 'The project ID'
     end
     resource :projects, requirements: FILE_ENDPOINT_REQUIREMENTS do
+      allow_access_with_scope :read_repository, if: -> (request) { request.get? || request.head? }
+
       desc 'Get raw file metadata from repository'
       params do
         requires :file_path, type: String, desc: 'The url encoded path to the file. Ex. lib%2Fclass%2Erb'

lib/gitlab/diff/file_collection/base.rb

@@ -34,6 +34,16 @@ module Gitlab
           @diff_files ||= diffs.decorate! { |diff| decorate_diff!(diff) }
         end
 
+        # This mutates `diff_files` lines.
+        def unfold_diff_files(positions)
+          positions_grouped_by_path = positions.group_by { |position| position.file_path }
+
+          diff_files.each do |diff_file|
+            positions = positions_grouped_by_path.fetch(diff_file.file_path, [])
+            positions.each { |position| diff_file.unfold_diff_lines(position) }
+          end
+        end
+
         def diff_file_with_old_path(old_path)
           diff_files.find { |diff_file| diff_file.old_path == old_path }
         end

lib/gitlab/diff/file_collection/compare.rb

@@ -10,6 +10,10 @@ module Gitlab
             diff_options: diff_options,
             diff_refs:    diff_refs)
         end
+
+        def unfold_diff_lines(positions)
+          # no-op
+        end
       end
     end
   end

lib/gitlab/usage_data.rb

@@ -57,7 +57,11 @@ module Gitlab
             environments: count(::Environment),
             clusters: count(::Clusters::Cluster),
             clusters_enabled: count(::Clusters::Cluster.enabled),
+            project_clusters_enabled: count(::Clusters::Cluster.enabled.project_type),
+            group_clusters_enabled: count(::Clusters::Cluster.enabled.group_type),
             clusters_disabled: count(::Clusters::Cluster.disabled),
+            project_clusters_disabled: count(::Clusters::Cluster.disabled.project_type),
+            group_clusters_disabled: count(::Clusters::Cluster.disabled.group_type),
             clusters_platforms_gke: count(::Clusters::Cluster.gcp_installed.enabled),
             clusters_platforms_user: count(::Clusters::Cluster.user_provided.enabled),
             clusters_applications_helm: count(::Clusters::Applications::Helm.installed),

locale/gitlab.pot

@@ -4500,6 +4500,9 @@ msgstr ""
 msgid "Identity provider single sign on URL"
 msgstr ""
 
+msgid "If any job surpasses this timeout threshold, it will be marked as failed. Human readable time input language is accepted like \"1 hour\". Values without specification represent seconds."
+msgstr ""
+
 msgid "If disabled, a diverged local branch will not be automatically updated with commits from its remote counterpart, to prevent local data loss. If the default branch (%{default_branch}) has diverged and cannot be updated, mirroring will fail. Other diverged branches are silently ignored."
 msgstr ""
 
@@ -6082,9 +6085,6 @@ msgstr ""
 msgid "People without permission will never get a notification and won't be able to comment."
 msgstr ""
 
-msgid "Per job. If a job passes this threshold, it will be marked as failed"
-msgstr ""
-
 msgid "Perform advanced options such as changing path, transferring, or removing the group."
 msgstr ""
 
@@ -8616,7 +8616,7 @@ msgstr ""
 msgid "This source diff could not be displayed because it is too large."
 msgstr ""
 
-msgid "This timeout will take precedence when lower than Project-defined timeout"
+msgid "This timeout will take precedence when lower than project-defined timeout and accepts a human readable time input language like \"1 hour\". Values without specification represent seconds."
 msgstr ""
 
 msgid "This user has no identities"

package.json

@@ -6,6 +6,7 @@
     "eslint": "eslint --max-warnings 0 --report-unused-disable-directives --ext .js,.vue .",
     "eslint-fix": "eslint --max-warnings 0 --report-unused-disable-directives --ext .js,.vue --fix .",
     "eslint-report": "eslint --max-warnings 0 --ext .js,.vue --format html --output-file ./eslint-report.html --no-inline-config .",
+    "jest": "BABEL_ENV=jest jest --config=config/jest.config.js",
     "karma": "BABEL_ENV=${BABEL_ENV:=karma} karma start --single-run true config/karma.config.js",
     "karma-coverage": "BABEL_ENV=coverage karma start --single-run true config/karma.config.js",
     "karma-start": "BABEL_ENV=karma karma start config/karma.config.js",
@@ -120,17 +121,23 @@
     "@gitlab/eslint-config": "^1.2.0",
     "@vue/test-utils": "^1.0.0-beta.25",
     "axios-mock-adapter": "^1.15.0",
+    "babel-core": "^7.0.0-bridge",
+    "babel-jest": "^23.6.0",
+    "babel-plugin-dynamic-import-node": "^2.2.0",
     "babel-plugin-istanbul": "^5.1.0",
     "babel-plugin-rewire": "^1.2.0",
+    "babel-plugin-transform-es2015-modules-commonjs": "^6.26.2",
     "babel-template": "^6.26.0",
     "babel-types": "^6.26.0",
     "chalk": "^2.4.1",
     "commander": "^2.18.0",
     "eslint": "~5.6.0",
+    "eslint-import-resolver-jest": "^2.1.1",
     "eslint-import-resolver-webpack": "^0.10.1",
     "eslint-plugin-html": "4.0.5",
     "eslint-plugin-import": "^2.14.0",
     "eslint-plugin-jasmine": "^2.10.1",
+    "eslint-plugin-jest": "^22.1.0",
     "gettext-extractor": "^3.3.2",
     "gettext-extractor-vue": "^4.0.1",
     "graphql-tag": "^2.10.0",
@@ -138,6 +145,8 @@
     "jasmine-core": "^2.9.0",
     "jasmine-diff": "^0.1.3",
     "jasmine-jquery": "^2.1.1",
+    "jest": "^23.6.0",
+    "jest-junit": "^5.2.0",
     "karma": "^3.0.0",
     "karma-chrome-launcher": "^2.2.0",
     "karma-coverage-istanbul-reporter": "^2.0.4",

spec/controllers/projects/merge_requests/diffs_controller_spec.rb

@@ -36,6 +36,18 @@ describe Projects::MergeRequests::DiffsController do
         end
       end
 
+      context 'when note has no position' do
+        before do
+          create(:legacy_diff_note_on_merge_request, project: project, noteable: merge_request, position: nil)
+        end
+
+        it 'serializes merge request diff collection' do
+          expect_any_instance_of(DiffsSerializer).to receive(:represent).with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), an_instance_of(Hash))
+
+          go
+        end
+      end
+
       context 'with forked projects with submodules' do
         render_views
 

spec/frontend/.eslintrc.yml

+---
+env:
+  jest/globals: true
+plugins:
+- jest
+settings:
+  import/resolver:
+    jest:
+      jestConfigFile: "config/jest.config.js"

spec/frontend/dummy_spec.js

+it('does nothing', () => {});

spec/javascripts/diffs/store/actions_spec.js

@@ -382,24 +382,47 @@ describe('DiffsStoreActions', () => {
       const file = { hash: 123, load_collapsed_diff_url: '/load/collapsed/diff/url' };
       const data = { hash: 123, parallelDiffLines: [{ lineCode: 1 }] };
       const mock = new MockAdapter(axios);
+      const commit = jasmine.createSpy('commit');
       mock.onGet(file.loadCollapsedDiffUrl).reply(200, data);
 
-      testAction(
-        loadCollapsedDiff,
-        file,
-        {},
-        [
-          {
-            type: types.ADD_COLLAPSED_DIFFS,
-            payload: { file, data },
-          },
-        ],
-        [],
-        () => {
+      loadCollapsedDiff({ commit, getters: { commitId: null } }, file)
+        .then(() => {
+          expect(commit).toHaveBeenCalledWith(types.ADD_COLLAPSED_DIFFS, { file, data });
+
           mock.restore();
           done();
-        },
-      );
+        })
+        .catch(done.fail);
+    });
+
+    it('should fetch data without commit ID', () => {
+      const file = { load_collapsed_diff_url: '/load/collapsed/diff/url' };
+      const getters = {
+        commitId: null,
+      };
+
+      spyOn(axios, 'get').and.returnValue(Promise.resolve({ data: {} }));
+
+      loadCollapsedDiff({ commit() {}, getters }, file);
+
+      expect(axios.get).toHaveBeenCalledWith(file.load_collapsed_diff_url, {
+        params: { commit_id: null },
+      });
+    });
+
+    it('should fetch data with commit ID', () => {
+      const file = { load_collapsed_diff_url: '/load/collapsed/diff/url' };
+      const getters = {
+        commitId: '123',
+      };
+
+      spyOn(axios, 'get').and.returnValue(Promise.resolve({ data: {} }));
+
+      loadCollapsedDiff({ commit() {}, getters }, file);
+
+      expect(axios.get).toHaveBeenCalledWith(file.load_collapsed_diff_url, {
+        params: { commit_id: '123' },
+      });
     });
   });
 

spec/javascripts/diffs/store/mutations_spec.js

@@ -199,6 +199,84 @@ describe('DiffsStoreMutations', () => {
       expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1);
     });
 
+    it('should not duplicate discussions on line', () => {
+      const diffPosition = {
+        base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+        head_sha: 'b921914f9a834ac47e6fd9420f78db0f83559130',
+        new_line: null,
+        new_path: '500-lines-4.txt',
+        old_line: 5,
+        old_path: '500-lines-4.txt',
+        start_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',
+      };
+
+      const state = {
+        latestDiff: true,
+        diffFiles: [
+          {
+            file_hash: 'ABC',
+            parallel_diff_lines: [
+              {
+                left: {
+                  line_code: 'ABC_1',
+                  discussions: [],
+                },
+                right: {
+                  line_code: 'ABC_1',
+                  discussions: [],
+                },
+              },
+            ],
+            highlighted_diff_lines: [
+              {
+                line_code: 'ABC_1',
+                discussions: [],
+              },
+            ],
+          },
+        ],
+      };
+      const discussion = {
+        id: 1,
+        line_code: 'ABC_1',
+        diff_discussion: true,
+        resolvable: true,
+        original_position: diffPosition,
+        position: diffPosition,
+        diff_file: {
+          file_hash: state.diffFiles[0].file_hash,
+        },
+      };
+
+      const diffPositionByLineCode = {
+        ABC_1: diffPosition,
+      };
+
+      mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, {
+        discussion,
+        diffPositionByLineCode,
+      });
+
+      expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1);
+      expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1);
+      expect(state.diffFiles[0].parallel_diff_lines[0].right.discussions).toEqual([]);
+
+      expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1);
+      expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1);
+
+      mutations[types.SET_LINE_DISCUSSIONS_FOR_FILE](state, {
+        discussion,
+        diffPositionByLineCode,
+      });
+
+      expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions.length).toEqual(1);
+      expect(state.diffFiles[0].parallel_diff_lines[0].left.discussions[0].id).toEqual(1);
+      expect(state.diffFiles[0].parallel_diff_lines[0].right.discussions).toEqual([]);
+
+      expect(state.diffFiles[0].highlighted_diff_lines[0].discussions.length).toEqual(1);
+      expect(state.diffFiles[0].highlighted_diff_lines[0].discussions[0].id).toEqual(1);
+    });
+
     it('should add legacy discussions to the given line', () => {
       const diffPosition = {
         base_sha: 'ed13df29948c41ba367caa757ab3ec4892509910',

spec/lib/gitlab/diff/file_collection/commit_spec.rb

@@ -12,4 +12,8 @@ describe Gitlab::Diff::FileCollection::Commit do
     let(:diffable) { project.commit }
     let(:stub_path) { 'bar/branch-test.txt' }
   end
+
+  it_behaves_like 'unfoldable diff' do
+    let(:diffable) { project.commit }
+  end
 end

spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb

@@ -2,8 +2,10 @@ require 'spec_helper'
 
 describe Gitlab::Diff::FileCollection::MergeRequestDiff do
   let(:merge_request) { create(:merge_request) }
-  let(:diff_files) { described_class.new(merge_request.merge_request_diff, diff_options: nil).diff_files }
+  let(:subject) { described_class.new(merge_request.merge_request_diff, diff_options: nil) }
+  let(:diff_files) { subject.diff_files }
 
+  describe '#diff_files' do
     it 'does not highlight binary files' do
       allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(false)
 
@@ -19,6 +21,11 @@ describe Gitlab::Diff::FileCollection::MergeRequestDiff do
 
       diff_files
     end
+  end
+
+  it_behaves_like 'unfoldable diff' do
+    let(:diffable) { merge_request.merge_request_diff }
+  end
 
   it 'it uses a different cache key if diff line keys change' do
     mr_diff = described_class.new(merge_request.merge_request_diff, diff_options: nil)

spec/lib/gitlab/usage_data_spec.rb

@@ -18,6 +18,9 @@ describe Gitlab::UsageData do
       gcp_cluster = create(:cluster, :provided_by_gcp)
       create(:cluster, :provided_by_user)
       create(:cluster, :provided_by_user, :disabled)
+      create(:cluster, :group)
+      create(:cluster, :group, :disabled)
+      create(:cluster, :group, :disabled)
       create(:clusters_applications_helm, :installed, cluster: gcp_cluster)
       create(:clusters_applications_ingress, :installed, cluster: gcp_cluster)
       create(:clusters_applications_cert_managers, :installed, cluster: gcp_cluster)
@@ -78,7 +81,11 @@ describe Gitlab::UsageData do
         environments
         clusters
         clusters_enabled
+        project_clusters_enabled
+        group_clusters_enabled
         clusters_disabled
+        project_clusters_disabled
+        group_clusters_disabled
         clusters_platforms_gke
         clusters_platforms_user
         clusters_applications_helm
@@ -128,8 +135,13 @@ describe Gitlab::UsageData do
       expect(count_data[:projects_slack_notifications_active]).to eq(2)
       expect(count_data[:projects_slack_slash_active]).to eq(1)
 
-      expect(count_data[:clusters_enabled]).to eq(6)
-      expect(count_data[:clusters_disabled]).to eq(1)
+      expect(count_data[:clusters_enabled]).to eq(7)
+      expect(count_data[:project_clusters_enabled]).to eq(6)
+      expect(count_data[:group_clusters_enabled]).to eq(1)
+      expect(count_data[:clusters_disabled]).to eq(3)
+      expect(count_data[:project_clusters_disabled]).to eq(1)
+      expect(count_data[:group_clusters_disabled]).to eq(2)
+      expect(count_data[:group_clusters_enabled]).to eq(1)
       expect(count_data[:clusters_platforms_gke]).to eq(1)
       expect(count_data[:clusters_platforms_user]).to eq(1)
       expect(count_data[:clusters_applications_helm]).to eq(1)

spec/models/concerns/chronic_duration_attribute_spec.rb

@@ -54,7 +54,8 @@ shared_examples 'ChronicDurationAttribute writer' do
       subject.send("#{virtual_field}=", '-10m')
 
       expect(subject.valid?).to be_falsey
-      expect(subject.errors&.messages).to include(virtual_field => ['is not a correct duration'])
+      expect(subject.errors&.messages)
+        .to include(base: ['Maximum job timeout has a value which could not be accepted'])
     end
   end
 

spec/models/project_spec.rb

@@ -158,6 +158,29 @@ describe Project do
         end
       end
     end
+
+    describe 'ci_pipelines association' do
+      context 'when feature flag pipeline_ci_sources_only is enabled' do
+        it 'returns only pipelines from ci_sources' do
+          stub_feature_flags(pipeline_ci_sources_only: true)
+
+          expect(Ci::Pipeline).to receive(:ci_sources).and_call_original
+
+          subject.ci_pipelines
+        end
+      end
+
+      context 'when feature flag pipeline_ci_sources_only is disabled' do
+        it 'returns all pipelines' do
+          stub_feature_flags(pipeline_ci_sources_only: false)
+
+          expect(Ci::Pipeline).not_to receive(:ci_sources).and_call_original
+          expect(Ci::Pipeline).to receive(:all).and_call_original.at_least(:once)
+
+          subject.ci_pipelines
+        end
+      end
+    end
   end
 
   describe 'modules' do

spec/requests/api/files_spec.rb

@@ -121,6 +121,13 @@ describe API::Files do
       end
     end
 
+    context 'when PATs are used' do
+      it_behaves_like 'repository files' do
+        let(:token) { create(:personal_access_token, scopes: ['read_repository'], user: user) }
+        let(:current_user) { { personal_access_token: token } }
+      end
+    end
+
     context 'when authenticated', 'as a developer' do
       it_behaves_like 'repository files' do
         let(:current_user) { user }
@@ -217,6 +224,13 @@ describe API::Files do
       end
     end
 
+    context 'when PATs are used' do
+      it_behaves_like 'repository files' do
+        let(:token) { create(:personal_access_token, scopes: ['read_repository'], user: user) }
+        let(:current_user) { { personal_access_token: token } }
+      end
+    end
+
     context 'when unauthenticated', 'and project is private' do
       it_behaves_like '404 response' do
         let(:request) { get api(route(file_path)), params }
@@ -317,6 +331,21 @@ describe API::Files do
         let(:request) { get api(route(file_path), guest), params }
       end
     end
+
+    context 'when PATs are used' do
+      it 'returns file by commit sha' do
+        token = create(:personal_access_token, scopes: ['read_repository'], user: user)
+
+        # This file is deleted on HEAD
+        file_path = "files%2Fjs%2Fcommit%2Ejs%2Ecoffee"
+        params[:ref] = "6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9"
+        expect(Gitlab::Workhorse).to receive(:send_git_blob)
+
+        get api(route(file_path) + "/raw", personal_access_token: token), params
+
+        expect(response).to have_gitlab_http_status(200)
+      end
+    end
   end
 
   describe "POST /projects/:id/repository/files/:file_path" do
@@ -362,6 +391,24 @@ describe API::Files do
       expect(response).to have_gitlab_http_status(400)
     end
 
+    context 'with PATs' do
+      it 'returns 403 with `read_repository` scope' do
+        token = create(:personal_access_token, scopes: ['read_repository'], user: user)
+
+        post api(route(file_path), personal_access_token: token), params
+
+        expect(response).to have_gitlab_http_status(403)
+      end
+
+      it 'returns 201 with `api` scope' do
+        token = create(:personal_access_token, scopes: ['api'], user: user)
+
+        post api(route(file_path), personal_access_token: token), params
+
+        expect(response).to have_gitlab_http_status(201)
+      end
+    end
+
     context "when specifying an author" do
       it "creates a new file with the specified author" do
         params.merge!(author_email: author_email, author_name: author_name)

spec/serializers/issue_serializer_spec.rb

@@ -36,4 +36,12 @@ describe IssueSerializer do
       expect(json_entity).to match_schema('entities/issue_board')
     end
   end
+
+  context 'board issue serialization' do
+    let(:serializer) { 'board' }
+
+    it 'matches board issue json schema' do
+      expect(json_entity).to match_schema('entities/issue_board')
+    end
+  end
 end

spec/support/shared_examples/diff_file_collections.rb

@@ -45,3 +45,19 @@ shared_examples 'diff statistics' do |test_include_stats_flag: true|
     end
   end
 end
+
+shared_examples 'unfoldable diff' do
+  let(:subject) { described_class.new(diffable, diff_options: nil) }
+
+  it 'calls Gitlab::Diff::File#unfold_diff_lines with correct position' do
+    position = instance_double(Gitlab::Diff::Position, file_path: 'README')
+    readme_file = instance_double(Gitlab::Diff::File, file_path: 'README')
+    other_file = instance_double(Gitlab::Diff::File, file_path: 'foo.rb')
+    nil_path_file = instance_double(Gitlab::Diff::File, file_path: nil)
+
+    allow(subject).to receive(:diff_files) { [readme_file, other_file, nil_path_file] }
+    expect(readme_file).to receive(:unfold_diff_lines).with(position)
+
+    subject.unfold_diff_files([position])
+  end
+end

spec/features/admin/admin_active_tab_spec.rb → spec/views/layouts/nav/sidebar/_admin.html.haml_spec.rb

 require 'spec_helper'
 
-RSpec.describe 'admin active tab' do
-  before do
-    sign_in(create(:admin))
-  end
-
+describe 'layouts/nav/sidebar/_admin' do
   shared_examples 'page has active tab' do |title|
     it "activates #{title} tab" do
-      expect(page).to have_selector('.nav-sidebar .sidebar-top-level-items > li.active', count: 1)
-      expect(page.find('.nav-sidebar .sidebar-top-level-items > li.active')).to have_content(title)
+      render
+
+      expect(rendered).to have_selector('.nav-sidebar .sidebar-top-level-items > li.active', count: 1)
+      expect(rendered).to have_css('.nav-sidebar .sidebar-top-level-items > li.active', text: title)
     end
   end
 
   shared_examples 'page has active sub tab' do |title|
     it "activates #{title} sub tab" do
-      expect(page).to have_selector('.sidebar-sub-level-items > li.active', count: 2)
-      expect(page.all('.sidebar-sub-level-items > li.active')[1]).to have_content(title)
+      render
+
+      expect(rendered).to have_css('.sidebar-sub-level-items > li.active', text: title)
     end
   end
 
   context 'on home page' do
     before do
-      visit admin_root_path
+      allow(controller).to receive(:controller_name).and_return('dashboard')
     end
 
     it_behaves_like 'page has active tab', 'Overview'
@@ -29,7 +28,8 @@ RSpec.describe 'admin active tab' do
 
   context 'on projects' do
     before do
-      visit admin_projects_path
+      allow(controller).to receive(:controller_name).and_return('projects')
+      allow(controller).to receive(:controller_path).and_return('admin/projects')
     end
 
     it_behaves_like 'page has active tab', 'Overview'
@@ -38,7 +38,7 @@ RSpec.describe 'admin active tab' do
 
   context 'on groups' do
     before do
-      visit admin_groups_path
+      allow(controller).to receive(:controller_name).and_return('groups')
     end
 
     it_behaves_like 'page has active tab', 'Overview'
@@ -47,7 +47,7 @@ RSpec.describe 'admin active tab' do
 
   context 'on users' do
     before do
-      visit admin_users_path
+      allow(controller).to receive(:controller_name).and_return('users')
     end
 
     it_behaves_like 'page has active tab', 'Overview'
@@ -56,7 +56,7 @@ RSpec.describe 'admin active tab' do
 
   context 'on logs' do
     before do
-      visit admin_logs_path
+      allow(controller).to receive(:controller_name).and_return('logs')
     end
 
     it_behaves_like 'page has active tab', 'Monitoring'
@@ -65,7 +65,7 @@ RSpec.describe 'admin active tab' do
 
   context 'on messages' do
     before do
-      visit admin_broadcast_messages_path
+      allow(controller).to receive(:controller_name).and_return('broadcast_messages')
     end
 
     it_behaves_like 'page has active tab', 'Messages'
@@ -73,7 +73,7 @@ RSpec.describe 'admin active tab' do
 
   context 'on hooks' do
     before do
-      visit admin_hooks_path
+      allow(controller).to receive(:controller_name).and_return('hooks')
     end
 
     it_behaves_like 'page has active tab', 'Hooks'
@@ -81,7 +81,7 @@ RSpec.describe 'admin active tab' do
 
   context 'on background jobs' do
     before do
-      visit admin_background_jobs_path
+      allow(controller).to receive(:controller_name).and_return('background_jobs')
     end
 
     it_behaves_like 'page has active tab', 'Monitoring'