diff options
109 files changed, 2442 insertions, 823 deletions
diff --git a/.gitlab/ci/reports.gitlab-ci.yml b/.gitlab/ci/reports.gitlab-ci.yml index 4d6c02336fe..797ec4f5860 100644 --- a/.gitlab/ci/reports.gitlab-ci.yml +++ b/.gitlab/ci/reports.gitlab-ci.yml @@ -1,5 +1,120 @@ +# Make sure to update all the similar conditions in other CI config files if you modify these conditions +.if-canonical-gitlab-merge-request: &if-canonical-gitlab-merge-request + if: '$CI_SERVER_HOST == "gitlab.com" && $CI_PROJECT_NAMESPACE == "gitlab-org" && $CI_MERGE_REQUEST_IID' + +# Make sure to update all the similar conditions in other CI config files if you modify these conditions +.if-canonical-dot-com-gitlab-org-group-schedule: &if-canonical-dot-com-gitlab-org-group-schedule + if: '$CI_SERVER_HOST == "gitlab.com" && $CI_PROJECT_NAMESPACE == "gitlab-org" && $CI_PIPELINE_SOURCE == "schedule"' + +# Make sure to update all the similar conditions in other CI config files if you modify these conditions +.if-master-refs: &if-master-refs + if: '$CI_COMMIT_REF_NAME == "master"' + +# Make sure to update all the similar conditions in other CI config files if you modify these conditions +.if-default-refs: &if-default-refs + if: '$CI_COMMIT_REF_NAME == "master" || $CI_COMMIT_REF_NAME =~ /^[\d-]+-stable(-ee)?$/ || $CI_COMMIT_REF_NAME =~ /^\d+-\d+-auto-deploy-\d+$/ || $CI_COMMIT_REF_NAME =~ /^security\// || $CI_MERGE_REQUEST_IID || $CI_COMMIT_TAG' + +# Make sure to update all the similar patterns in other CI config files if you modify these patterns +.code-backstage-patterns: &code-backstage-patterns + - ".gitlab/ci/**/*" + - ".{eslintignore,gitattributes,nvmrc,prettierrc,stylelintrc,yamllint}" + - ".{codeclimate,eslintrc,gitlab-ci,haml-lint,haml-lint_todo,rubocop,rubocop_todo,scss-lint}.yml" + - ".csscomb.json" + - "Dockerfile.assets" + - "*_VERSION" + - "Gemfile{,.lock}" + - "Rakefile" + - "{babel.config,jest.config}.js" + - "config.ru" + - "{package.json,yarn.lock}" + - "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*" + - "doc/api/graphql/reference/*" # Files in this folder are auto-generated + # Backstage changes + - "Dangerfile" + - "danger/**/*" + - "{,ee/}fixtures/**/*" + - "{,ee/}rubocop/**/*" + - "{,ee/}spec/**/*" + - "doc/README.md" # Some RSpec test rely on this file + +# Make sure to update all the similar patterns in other CI config files if you modify these patterns +.code-qa-patterns: &code-qa-patterns + - ".gitlab/ci/**/*" + - ".{eslintignore,gitattributes,nvmrc,prettierrc,stylelintrc,yamllint}" + - ".{codeclimate,eslintrc,gitlab-ci,haml-lint,haml-lint_todo,rubocop,rubocop_todo,scss-lint}.yml" + - ".csscomb.json" + - "Dockerfile.assets" + - "*_VERSION" + - "Gemfile{,.lock}" + - "Rakefile" + - "{babel.config,jest.config}.js" + - "config.ru" + - "{package.json,yarn.lock}" + - "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*" + - "doc/api/graphql/reference/*" # Files in this folder are auto-generated + # QA changes + - ".dockerignore" + - "qa/**/*" + +# Make sure to update all the similar patterns in other CI config files if you modify these patterns +.code-backstage-qa-patterns: &code-backstage-qa-patterns + - ".gitlab/ci/**/*" + - ".{eslintignore,gitattributes,nvmrc,prettierrc,stylelintrc,yamllint}" + - ".{codeclimate,eslintrc,gitlab-ci,haml-lint,haml-lint_todo,rubocop,rubocop_todo,scss-lint}.yml" + - ".csscomb.json" + - "Dockerfile.assets" + - "*_VERSION" + - "Gemfile{,.lock}" + - "Rakefile" + - "{babel.config,jest.config}.js" + - "config.ru" + - "{package.json,yarn.lock}" + - "{,ee/}{app,bin,config,db,haml_lint,lib,locale,public,scripts,symbol,vendor}/**/*" + - "doc/api/graphql/reference/*" # Files in this folder are auto-generated + # Backstage changes + - "Dangerfile" + - "danger/**/*" + - "{,ee/}fixtures/**/*" + - "{,ee/}rubocop/**/*" + - "{,ee/}spec/**/*" + - "doc/README.md" # Some RSpec test rely on this file + # QA changes + - ".dockerignore" + - "qa/**/*" + +.reports:rules:code_quality: + rules: + - if: '$CODE_QUALITY_DISABLED' + when: never + # - <<: *if-master-refs # To be done in a later iteration: https://gitlab.com/gitlab-org/gitlab/issues/31160#note_278188255 + - <<: *if-default-refs + changes: *code-backstage-patterns + +.reports:rules:sast: + rules: + - if: '$SAST_DISABLED || $GITLAB_FEATURES !~ /\bsast\b/' + when: never + # - <<: *if-master-refs # To be done in a later iteration: https://gitlab.com/gitlab-org/gitlab/issues/31160#note_278188255 + - <<: *if-default-refs + changes: *code-backstage-qa-patterns + +.reports:rules:dependency_scanning: + rules: + - if: '$DEPENDENCY_SCANNING_DISABLED || $GITLAB_FEATURES !~ /\bdependency_scanning\b/' + when: never + # - <<: *if-master-refs # To be done in a later iteration: https://gitlab.com/gitlab-org/gitlab/issues/31160#note_278188255 + - <<: *if-default-refs + changes: *code-backstage-qa-patterns + +.reports:rules:dast: + rules: + - if: '$DAST_DISABLED || $GITLAB_FEATURES !~ /\bdast\b/' + when: never + - <<: *if-canonical-gitlab-merge-request + changes: *code-qa-patterns + # include: -# - template: Code-Quality.gitlab-ci.yml +# - template: Jobs/Code-Quality.gitlab-ci.yml # - template: Security/SAST.gitlab-ci.yml # - template: Security/Dependency-Scanning.gitlab-ci.yml # - template: Security/DAST.gitlab-ci.yml @@ -10,8 +125,7 @@ code_quality: extends: - .default-retry - - .default-only - - .only:changes-code-backstage + - .reports:rules:code_quality stage: test image: docker:stable allow_failure: true @@ -38,12 +152,9 @@ code_quality: reports: codequality: gl-code-quality-report.json paths: - - gl-code-quality-report.json - expire_in: 1 week + - gl-code-quality-report.json # GitLab-specific + expire_in: 1 week # GitLab-specific dependencies: [] - except: - variables: - - $CODE_QUALITY_DISABLED # We need to duplicate this job's definition because it seems it's impossible to # override an included `only.refs`. @@ -53,16 +164,22 @@ code_quality: sast: extends: - .default-retry - - .default-only - - .only:changes-code-backstage-qa + - .reports:rules:sast stage: test + allow_failure: true + dependencies: [] # GitLab-specific + artifacts: + paths: + - gl-sast-report.json # GitLab-specific + reports: + sast: gl-sast-report.json + expire_in: 1 week # GitLab-specific image: docker:stable variables: DOCKER_DRIVER: overlay2 DOCKER_TLS_CERTDIR: "" - SAST_BRAKEMAN_LEVEL: 2 - SAST_EXCLUDED_PATHS: qa,spec,doc,ee/spec - allow_failure: true + SAST_BRAKEMAN_LEVEL: 2 # GitLab-specific + SAST_EXCLUDED_PATHS: qa,spec,doc,ee/spec # GitLab-specific services: - docker:stable-dind script: @@ -73,61 +190,12 @@ sast: export DOCKER_HOST='tcp://localhost:2375' fi fi - - | # this is required to avoid undesirable reset of Docker image ENV variables being set on build stage - function propagate_env_vars() { - CURRENT_ENV=$(printenv) - - for VAR_NAME; do - echo $CURRENT_ENV | grep "${VAR_NAME}=" > /dev/null && echo "--env $VAR_NAME " - done - } - | - docker run \ - $(propagate_env_vars \ - SAST_BANDIT_EXCLUDED_PATHS \ - SAST_ANALYZER_IMAGES \ - SAST_ANALYZER_IMAGE_PREFIX \ - SAST_ANALYZER_IMAGE_TAG \ - SAST_DEFAULT_ANALYZERS \ - SAST_PULL_ANALYZER_IMAGES \ - SAST_BRAKEMAN_LEVEL \ - SAST_FLAWFINDER_LEVEL \ - SAST_GITLEAKS_ENTROPY_LEVEL \ - SAST_GOSEC_LEVEL \ - SAST_EXCLUDED_PATHS \ - SAST_DOCKER_CLIENT_NEGOTIATION_TIMEOUT \ - SAST_PULL_ANALYZER_IMAGE_TIMEOUT \ - SAST_RUN_ANALYZER_TIMEOUT \ - SAST_JAVA_VERSION \ - ANT_HOME \ - ANT_PATH \ - GRADLE_PATH \ - JAVA_OPTS \ - JAVA_PATH \ - JAVA_8_VERSION \ - JAVA_11_VERSION \ - MAVEN_CLI_OPTS \ - MAVEN_PATH \ - MAVEN_REPO_PATH \ - SBT_PATH \ - FAIL_NEVER \ - ) \ + ENVS=`printenv | grep -vE '^(DOCKER_|CI|GITLAB_|FF_|HOME|PWD|OLDPWD|PATH|SHLVL|HOSTNAME)' | sed -n '/^[^\t]/s/=.*//p' | sed '/^$/d' | sed 's/^/-e /g' | tr '\n' ' '` + docker run "$ENVS" \ --volume "$PWD:/code" \ --volume /var/run/docker.sock:/var/run/docker.sock \ "registry.gitlab.com/gitlab-org/security-products/sast:$SAST_VERSION" /app/bin/run /code - artifacts: - expire_in: 7 days - paths: - - gl-sast-report.json - reports: - sast: gl-sast-report.json - dependencies: [] - only: - variables: - - $GITLAB_FEATURES =~ /\bsast\b/ - except: - variables: - - $SAST_DISABLED # We need to duplicate this job's definition because it seems it's impossible to # override an included `only.refs`. @@ -135,14 +203,13 @@ sast: dependency_scanning: extends: - .default-retry - - .default-only - - .only:changes-code-backstage-qa + - .reports:rules:dependency_scanning stage: test image: docker:stable variables: DOCKER_DRIVER: overlay2 DOCKER_TLS_CERTDIR: "" - DS_EXCLUDED_PATHS: "qa/qa/ee/fixtures/secure_premade_reports,spec,ee/spec" + DS_EXCLUDED_PATHS: "qa/qa/ee/fixtures/secure_premade_reports,spec,ee/spec" # GitLab-specific allow_failure: true services: - docker:stable-dind @@ -174,23 +241,29 @@ dependency_scanning: DS_PULL_ANALYZER_IMAGE_TIMEOUT \ DS_RUN_ANALYZER_TIMEOUT \ DS_PYTHON_VERSION \ + DS_PIP_VERSION \ DS_PIP_DEPENDENCY_PATH \ + GEMNASIUM_DB_LOCAL_PATH \ + GEMNASIUM_DB_REMOTE_URL \ + GEMNASIUM_DB_REF_NAME \ PIP_INDEX_URL \ PIP_EXTRA_INDEX_URL \ + PIP_REQUIREMENTS_FILE \ + MAVEN_CLI_OPTS \ + BUNDLER_AUDIT_UPDATE_DISABLED \ + BUNDLER_AUDIT_ADVISORY_DB_URL \ + BUNDLER_AUDIT_ADVISORY_DB_REF_NAME \ ) \ --volume "$PWD:/code" \ --volume /var/run/docker.sock:/var/run/docker.sock \ "registry.gitlab.com/gitlab-org/security-products/dependency-scanning:$DS_VERSION" /code artifacts: + paths: + - gl-dependency-scanning-report.json # GitLab-specific reports: dependency_scanning: gl-dependency-scanning-report.json + expire_in: 1 week # GitLab-specific dependencies: [] - only: - variables: - - $GITLAB_FEATURES =~ /\bdependency_scanning\b/ - except: - variables: - - $DEPENDENCY_SCANNING_DISABLED # We need to duplicate this job's definition because it seems it's impossible to # override an included `only.refs`. @@ -198,40 +271,38 @@ dependency_scanning: dast: extends: - .default-retry - - .default-only - - .only:changes-code-qa - - .only-review - stage: qa - needs: ["review-deploy"] - dependencies: ["review-deploy"] - before_script: - - export DAST_WEBSITE="$(cat review_app_url.txt)" + - .reports:rules:dast + needs: + - job: review-deploy + artifacts: true + stage: qa # GitLab-specific image: - name: "registry.gitlab.com/gitlab-org/security-products/dast:$CI_SERVER_VERSION_MAJOR-$CI_SERVER_VERSION_MINOR-stable" + name: "registry.gitlab.com/gitlab-org/security-products/dast:$DAST_VERSION" variables: - # URL to scan: - # DAST_WEBSITE: https://example.com/ - # - # Time limit for target availability (scan is attempted even when timeout): - # DAST_TARGET_AVAILABILITY_TIMEOUT: 60 - # - # Set these variables to scan with an authenticated user: - # DAST_AUTH_URL: https://example.com/sign-in - # DAST_USERNAME: john.doe@example.com - # DAST_PASSWORD: john-doe-password - # DAST_USERNAME_FIELD: session[user] # the name of username field at the sign-in HTML form - # DAST_PASSWORD_FIELD: session[password] # the name of password field at the sign-in HTML form - # DAST_AUTH_EXCLUDE_URLS: http://example.com/sign-out,http://example.com/sign-out-2 # optional: URLs to skip during the authenticated scan; comma-separated, no spaces in between - # - # Perform ZAP Full Scan, which includes both passive and active scanning: - # DAST_FULL_SCAN_ENABLED: "true" + # To be done in a later iteration + # DAST_USERNAME: "root" + # DAST_USERNAME_FIELD: "user[login]" + # DAST_PASSWORD_FIELD: "user[passowrd]" allow_failure: true script: - - export DAST_WEBSITE=${DAST_WEBSITE:-$(cat environment_url.txt)} + - 'export DAST_WEBSITE="${DAST_WEBSITE:-$(cat environment_url.txt)}"' + # To be done in a later iteration + # - 'export DAST_AUTH_URL="${DAST_WEBSITE}/users/sign_in"' + # - 'export DAST_PASSWORD="${REVIEW_APPS_ROOT_PASSWORD}"' - /analyze -t $DAST_WEBSITE artifacts: - expire_in: 7 days paths: - - gl-dast-report.json + - gl-dast-report.json # GitLab-specific reports: dast: gl-dast-report.json + expire_in: 1 week # GitLab-specific + +# To be done in a later iteration: https://gitlab.com/gitlab-org/gitlab/issues/31160#note_278188255 +# schedule:dast: +# extends: dast +# rules: +# - if: '$DAST_DISABLED || $GITLAB_FEATURES !~ /\bdast\b/' +# when: never +# - <<: *if-canonical-dot-com-gitlab-org-group-schedule +# variables: +# DAST_FULL_SCAN_ENABLED: "true" diff --git a/.gitlab/ci/review.gitlab-ci.yml b/.gitlab/ci/review.gitlab-ci.yml index 5d2c33d06b4..682ed12e199 100644 --- a/.gitlab/ci/review.gitlab-ci.yml +++ b/.gitlab/ci/review.gitlab-ci.yml @@ -148,7 +148,7 @@ review-deploy: - export GITLAB_SHELL_VERSION=$(<GITLAB_SHELL_VERSION) - export GITALY_VERSION=$(<GITALY_SERVER_VERSION) - export GITLAB_WORKHORSE_VERSION=$(<GITLAB_WORKHORSE_VERSION) - - echo "${CI_ENVIRONMENT_URL}" > review_app_url.txt + - echo "${CI_ENVIRONMENT_URL}" > environment_url.txt - source ./scripts/utils.sh - install_api_client_dependencies_with_apk - source scripts/review_apps/review-apps.sh @@ -161,7 +161,7 @@ review-deploy: - date - deploy || (display_deployment_debug && exit 1) artifacts: - paths: [review_app_url.txt] + paths: [environment_url.txt] expire_in: 2 days when: always @@ -216,7 +216,7 @@ review-stop: before_script: - '[[ ! -d "ee/" ]] || export GITLAB_EDITION="ee"' - export QA_IMAGE="${CI_REGISTRY}/${CI_PROJECT_PATH}/gitlab/gitlab-${GITLAB_EDITION}-qa:${CI_COMMIT_REF_SLUG}" - - export CI_ENVIRONMENT_URL="$(cat review_app_url.txt)" + - export CI_ENVIRONMENT_URL="$(cat environment_url.txt)" - echo "${CI_ENVIRONMENT_URL}" - echo "${QA_IMAGE}" - source scripts/utils.sh @@ -255,7 +255,7 @@ review-performance: artifacts: true allow_failure: true before_script: - - export CI_ENVIRONMENT_URL="$(cat review_app_url.txt)" + - export CI_ENVIRONMENT_URL="$(cat environment_url.txt)" - echo "${CI_ENVIRONMENT_URL}" - mkdir -p gitlab-exporter - wget -O ./gitlab-exporter/index.js https://gitlab.com/gitlab-org/gl-performance/raw/master/index.js diff --git a/app/assets/javascripts/clusters/stores/clusters_store.js b/app/assets/javascripts/clusters/stores/clusters_store.js index 26456fb28db..939c396e1b9 100644 --- a/app/assets/javascripts/clusters/stores/clusters_store.js +++ b/app/assets/javascripts/clusters/stores/clusters_store.js @@ -257,6 +257,7 @@ export default class ClusterStore { name: environment.name, project: environment.project, environmentPath: environment.environment_path, + logsPath: environment.logs_path, lastDeployment: environment.last_deployment, rolloutStatus: { status: environment.rollout_status ? environment.rollout_status.status : null, diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue deleted file mode 100644 index 9eaceb8893c..00000000000 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ /dev/null @@ -1,150 +0,0 @@ -<script> -import { mapState, mapGetters, mapActions } from 'vuex'; -import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils'; -import Icon from '~/vue_shared/components/icon.vue'; -import DiffGutterAvatars from './diff_gutter_avatars.vue'; -import { LINE_POSITION_RIGHT } from '../constants'; - -export default { - components: { - DiffGutterAvatars, - Icon, - }, - props: { - line: { - type: Object, - required: true, - }, - fileHash: { - type: String, - required: true, - }, - contextLinesPath: { - type: String, - required: true, - }, - lineNumber: { - type: Number, - required: false, - default: 0, - }, - linePosition: { - type: String, - required: false, - default: '', - }, - showCommentButton: { - type: Boolean, - required: false, - default: false, - }, - isBottom: { - type: Boolean, - required: false, - default: false, - }, - isMatchLine: { - type: Boolean, - required: false, - default: false, - }, - isMetaLine: { - type: Boolean, - required: false, - default: false, - }, - isContextLine: { - type: Boolean, - required: false, - default: false, - }, - isHover: { - type: Boolean, - required: false, - default: false, - }, - }, - computed: { - ...mapState({ - diffViewType: state => state.diffs.diffViewType, - diffFiles: state => state.diffs.diffFiles, - }), - ...mapGetters(['isLoggedIn']), - lineCode() { - return ( - this.line.line_code || - (this.line.left && this.line.left.line_code) || - (this.line.right && this.line.right.line_code) - ); - }, - lineHref() { - return `#${this.line.line_code || ''}`; - }, - shouldShowCommentButton() { - return ( - this.isHover && - !this.isMatchLine && - !this.isContextLine && - !this.isMetaLine && - !this.hasDiscussions - ); - }, - hasDiscussions() { - return this.line.discussions && this.line.discussions.length > 0; - }, - shouldShowAvatarsOnGutter() { - if (!this.line.type && this.linePosition === LINE_POSITION_RIGHT) { - return false; - } - return this.showCommentButton && this.hasDiscussions; - }, - shouldRenderCommentButton() { - const isDiffHead = parseBoolean(getParameterByName('diff_head')); - return !isDiffHead && this.isLoggedIn && this.showCommentButton; - }, - }, - methods: { - ...mapActions('diffs', [ - 'loadMoreLines', - 'showCommentForm', - 'setHighlightedRow', - 'toggleLineDiscussions', - 'toggleLineDiscussionWrappers', - ]), - handleCommentButton() { - this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.fileHash }); - }, - }, -}; -</script> - -<template> - <div> - <button - v-if="shouldRenderCommentButton" - v-show="shouldShowCommentButton" - type="button" - class="add-diff-note js-add-diff-note-button qa-diff-comment" - title="Add a comment to this line" - @click="handleCommentButton" - > - <icon :size="12" name="comment" /> - </button> - <a - v-if="lineNumber" - ref="lineNumberRef" - :data-linenumber="lineNumber" - :href="lineHref" - @click="setHighlightedRow(lineCode)" - > - </a> - <diff-gutter-avatars - v-if="shouldShowAvatarsOnGutter" - :discussions="line.discussions" - :discussions-expanded="line.discussionsExpanded" - @toggleLineDiscussions=" - toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded }) - " - /> - </div> -</template> diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index 0f3e9208d21..9544fbe9fc5 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -1,21 +1,24 @@ <script> import { mapGetters, mapActions } from 'vuex'; -import DiffLineGutterContent from './diff_line_gutter_content.vue'; +import { GlIcon } from '@gitlab/ui'; +import { getParameterByName, parseBoolean } from '~/lib/utils/common_utils'; +import DiffGutterAvatars from './diff_gutter_avatars.vue'; import { MATCH_LINE_TYPE, CONTEXT_LINE_TYPE, + LINE_POSITION_RIGHT, EMPTY_CELL_TYPE, OLD_LINE_TYPE, OLD_NO_NEW_LINE_TYPE, NEW_NO_NEW_LINE_TYPE, LINE_HOVER_CLASS_NAME, LINE_UNFOLD_CLASS_NAME, - INLINE_DIFF_VIEW_TYPE, } from '../constants'; export default { components: { - DiffLineGutterContent, + DiffGutterAvatars, + GlIcon, }, props: { line: { @@ -33,12 +36,6 @@ export default { isHighlighted: { type: Boolean, required: true, - default: false, - }, - diffViewType: { - type: String, - required: false, - default: INLINE_DIFF_VIEW_TYPE, }, showCommentButton: { type: Boolean, @@ -73,6 +70,38 @@ export default { }, computed: { ...mapGetters(['isLoggedIn']), + lineCode() { + return ( + this.line.line_code || + (this.line.left && this.line.left.line_code) || + (this.line.right && this.line.right.line_code) + ); + }, + lineHref() { + return `#${this.line.line_code || ''}`; + }, + shouldShowCommentButton() { + return ( + this.isHover && + !this.isMatchLine && + !this.isContextLine && + !this.isMetaLine && + !this.hasDiscussions + ); + }, + hasDiscussions() { + return this.line.discussions && this.line.discussions.length > 0; + }, + shouldShowAvatarsOnGutter() { + if (!this.line.type && this.linePosition === LINE_POSITION_RIGHT) { + return false; + } + return this.showCommentButton && this.hasDiscussions; + }, + shouldRenderCommentButton() { + const isDiffHead = parseBoolean(getParameterByName('diff_head')); + return !isDiffHead && this.isLoggedIn && this.showCommentButton; + }, isMatchLine() { return this.line.type === MATCH_LINE_TYPE; }, @@ -107,24 +136,45 @@ export default { return this.lineType === OLD_LINE_TYPE ? this.line.old_line : this.line.new_line; }, }, - methods: mapActions('diffs', ['setHighlightedRow']), + methods: { + ...mapActions('diffs', ['showCommentForm', 'setHighlightedRow', 'toggleLineDiscussions']), + handleCommentButton() { + this.showCommentForm({ lineCode: this.line.line_code, fileHash: this.fileHash }); + }, + }, }; </script> <template> - <td :class="classNameMap"> - <diff-line-gutter-content - :line="line" - :file-hash="fileHash" - :context-lines-path="contextLinesPath" - :line-position="linePosition" - :line-number="lineNumber" - :show-comment-button="showCommentButton" - :is-hover="isHover" - :is-bottom="isBottom" - :is-match-line="isMatchLine" - :is-context-line="isContentLine" - :is-meta-line="isMetaLine" - /> + <td ref="td" :class="classNameMap"> + <div> + <button + v-if="shouldRenderCommentButton" + v-show="shouldShowCommentButton" + ref="addDiffNoteButton" + type="button" + class="add-diff-note js-add-diff-note-button qa-diff-comment" + title="Add a comment to this line" + @click="handleCommentButton" + > + <gl-icon :size="12" name="comment" /> + </button> + <a + v-if="lineNumber" + ref="lineNumberRef" + :data-linenumber="lineNumber" + :href="lineHref" + @click="setHighlightedRow(lineCode)" + > + </a> + <diff-gutter-avatars + v-if="shouldShowAvatarsOnGutter" + :discussions="line.discussions" + :discussions-expanded="line.discussionsExpanded" + @toggleLineDiscussions=" + toggleLineDiscussions({ lineCode, fileHash, expanded: !line.discussionsExpanded }) + " + /> + </div> </td> </template> diff --git a/app/assets/javascripts/environments/components/environments_table.vue b/app/assets/javascripts/environments/components/environments_table.vue index 8abc927c500..3f316643784 100644 --- a/app/assets/javascripts/environments/components/environments_table.vue +++ b/app/assets/javascripts/environments/components/environments_table.vue @@ -162,8 +162,7 @@ export default { :is-loading="model.isLoadingDeployBoard" :is-empty="model.isEmptyDeployBoard" :has-legacy-app-label="model.hasLegacyAppLabel" - :project-path="model.project_path" - :environment-name="model.name" + :logs-path="model.logs_path" /> </div> </div> diff --git a/app/assets/javascripts/error_tracking/details.js b/app/assets/javascripts/error_tracking/details.js index 1a92681374b..55ab362f805 100644 --- a/app/assets/javascripts/error_tracking/details.js +++ b/app/assets/javascripts/error_tracking/details.js @@ -8,28 +8,30 @@ import csrf from '~/lib/utils/csrf'; Vue.use(VueApollo); export default () => { + const selector = '#js-error_details'; + + const domEl = document.querySelector(selector); + const { + issueId, + projectPath, + issueUpdatePath, + issueStackTracePath, + projectIssuesPath, + } = domEl.dataset; + const apolloProvider = new VueApollo({ defaultClient: createDefaultClient(), }); // eslint-disable-next-line no-new new Vue({ - el: '#js-error_details', + el: selector, apolloProvider, components: { ErrorDetails, }, store, render(createElement) { - const domEl = document.querySelector(this.$options.el); - const { - issueId, - projectPath, - issueUpdatePath, - issueStackTracePath, - projectIssuesPath, - } = domEl.dataset; - return createElement('error-details', { props: { issueId, diff --git a/app/assets/javascripts/error_tracking/list.js b/app/assets/javascripts/error_tracking/list.js index 8f3700249da..cb656a9ef13 100644 --- a/app/assets/javascripts/error_tracking/list.js +++ b/app/assets/javascripts/error_tracking/list.js @@ -4,27 +4,29 @@ import store from './store'; import ErrorTrackingList from './components/error_tracking_list.vue'; export default () => { + const selector = '#js-error_tracking'; + + const domEl = document.querySelector(selector); + const { + indexPath, + enableErrorTrackingLink, + illustrationPath, + projectPath, + listPath, + } = domEl.dataset; + let { errorTrackingEnabled, userCanEnableErrorTracking } = domEl.dataset; + + errorTrackingEnabled = parseBoolean(errorTrackingEnabled); + userCanEnableErrorTracking = parseBoolean(userCanEnableErrorTracking); + // eslint-disable-next-line no-new new Vue({ - el: '#js-error_tracking', + el: selector, components: { ErrorTrackingList, }, store, render(createElement) { - const domEl = document.querySelector(this.$options.el); - const { - indexPath, - enableErrorTrackingLink, - illustrationPath, - projectPath, - listPath, - } = domEl.dataset; - let { errorTrackingEnabled, userCanEnableErrorTracking } = domEl.dataset; - - errorTrackingEnabled = parseBoolean(errorTrackingEnabled); - userCanEnableErrorTracking = parseBoolean(userCanEnableErrorTracking); - return createElement('error-tracking-list', { props: { indexPath, diff --git a/app/assets/javascripts/monitoring/components/charts/time_series.vue b/app/assets/javascripts/monitoring/components/charts/time_series.vue index 8b2c5e44bb5..eaf0780d9e1 100644 --- a/app/assets/javascripts/monitoring/components/charts/time_series.vue +++ b/app/assets/javascripts/monitoring/components/charts/time_series.vue @@ -18,6 +18,17 @@ import { import { makeDataSeries } from '~/helpers/monitor_helper'; import { graphDataValidatorForValues } from '../../utils'; +/** + * A "virtual" coordinates system for the deployment icons. + * Deployment icons are displayed along the [min, max] + * range at height `pos`. + */ +const deploymentYAxisCoords = { + min: 0, + pos: 3, // 3% height of chart's grid + max: 100, +}; + const THROTTLED_DATAZOOM_WAIT = 1000; // miliseconds const timestampToISODate = timestamp => new Date(timestamp).toISOString(); @@ -145,10 +156,33 @@ export default { }, []); }, chartOptionSeries() { - return (this.option.series || []).concat(this.scatterSeries ? [this.scatterSeries] : []); + return (this.option.series || []).concat( + this.deploymentSeries ? [this.deploymentSeries] : [], + ); }, chartOptions() { const option = omit(this.option, 'series'); + + const dataYAxis = { + name: this.yAxisLabel, + nameGap: 50, // same as gitlab-ui's default + nameLocation: 'center', // same as gitlab-ui's default + boundaryGap: [0.1, 0.1], + scale: true, + axisLabel: { + formatter: num => roundOffFloat(num, 3).toString(), + }, + }; + const deploymentsYAxis = { + show: false, + min: deploymentYAxisCoords.min, + max: deploymentYAxisCoords.max, + axisLabel: { + // formatter fn required to trigger tooltip re-positioning + formatter: () => {}, + }, + }; + return { series: this.chartOptionSeries, xAxis: { @@ -161,12 +195,7 @@ export default { snap: true, }, }, - yAxis: { - name: this.yAxisLabel, - axisLabel: { - formatter: num => roundOffFloat(num, 3).toString(), - }, - }, + yAxis: [dataYAxis, deploymentsYAxis], dataZoom: [this.dataZoomConfig], ...option, }; @@ -228,10 +257,16 @@ export default { return acc; }, []); }, - scatterSeries() { + deploymentSeries() { return { type: graphTypes.deploymentData, - data: this.recentDeployments.map(deployment => [deployment.createdAt, 0]), + + yAxisIndex: 1, // deploymentsYAxis index + data: this.recentDeployments.map(deployment => [ + deployment.createdAt, + deploymentYAxisCoords.pos, + ]), + symbol: this.svgs.rocket, symbolSize: symbolSizes.default, itemStyle: { @@ -265,6 +300,7 @@ export default { formatTooltipText(params) { this.tooltip.title = dateFormat(params.value, dateFormats.default); this.tooltip.content = []; + params.seriesData.forEach(dataPoint => { if (dataPoint.value) { const [xVal, yVal] = dataPoint.value; diff --git a/app/assets/javascripts/registry/shared/components/expiration_policy_fields.vue b/app/assets/javascripts/registry/shared/components/expiration_policy_fields.vue index 84d1c5ccc6a..a15b854cb9b 100644 --- a/app/assets/javascripts/registry/shared/components/expiration_policy_fields.vue +++ b/app/assets/javascripts/registry/shared/components/expiration_policy_fields.vue @@ -82,7 +82,7 @@ export default { regexHelpText() { return sprintf( s__( - 'ContainerRegistry|Wildcards such as %{codeStart}*-stable%{codeEnd} or %{codeStart}production/*%{codeEnd} are supported. To select all tags, use %{codeStart}.*%{codeEnd}', + 'ContainerRegistry|Wildcards such as %{codeStart}.*-stable%{codeEnd} or %{codeStart}production/.*%{codeEnd} are supported. To select all tags, use %{codeStart}.*%{codeEnd}', ), { codeStart: '<code>', diff --git a/app/assets/javascripts/snippet/collapsible_input.js b/app/assets/javascripts/snippet/collapsible_input.js new file mode 100644 index 00000000000..e7225162f86 --- /dev/null +++ b/app/assets/javascripts/snippet/collapsible_input.js @@ -0,0 +1,45 @@ +const hide = el => el.classList.add('d-none'); +const show = el => el.classList.remove('d-none'); + +const setupCollapsibleInput = el => { + const collapsedEl = el.querySelector('.js-collapsed'); + const expandedEl = el.querySelector('.js-expanded'); + const collapsedInputEl = collapsedEl.querySelector('textarea,input,select'); + const expandedInputEl = expandedEl.querySelector('textarea,input,select'); + const formEl = el.closest('form'); + + const collapse = () => { + hide(expandedEl); + show(collapsedEl); + }; + + const expand = () => { + hide(collapsedEl); + show(expandedEl); + }; + + // NOTE: + // We add focus listener to all form inputs so that we can collapse + // when something is focused that's not the expanded input. + formEl.addEventListener('focusin', e => { + if (e.target === collapsedInputEl) { + expand(); + expandedInputEl.focus(); + } else if (!el.contains(e.target) && !expandedInputEl.value) { + collapse(); + } + }); +}; + +/** + * Usage in HAML + * + * .js-collapsible-input + * .js-collapsed{ class: ('d-none' if is_expanded) } + * = input + * .js-expanded{ class: ('d-none' if !is_expanded) } + * = big_input + */ +export default () => { + Array.from(document.querySelectorAll('.js-collapsible-input')).forEach(setupCollapsibleInput); +}; diff --git a/app/assets/javascripts/snippet/snippet_bundle.js b/app/assets/javascripts/snippet/snippet_bundle.js index dcee17453b8..652531a1289 100644 --- a/app/assets/javascripts/snippet/snippet_bundle.js +++ b/app/assets/javascripts/snippet/snippet_bundle.js @@ -1,6 +1,7 @@ /* global ace */ import $ from 'jquery'; +import setupCollapsibleInputs from './collapsible_input'; export default () => { const editor = ace.edit('editor'); @@ -8,4 +9,6 @@ export default () => { $('.snippet-form-holder form').on('submit', () => { $('.snippet-file-content').val(editor.getValue()); }); + + setupCollapsibleInputs(); }; diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb index 9fbfc59f630..8414095d454 100644 --- a/app/controllers/admin/users_controller.rb +++ b/app/controllers/admin/users_controller.rb @@ -75,7 +75,9 @@ class Admin::UsersController < Admin::ApplicationController end def block - if update_user { |user| user.block } + result = Users::BlockService.new(current_user).execute(user) + + if result[:status] = :success redirect_back_or_admin_user(notice: _("Successfully blocked")) else redirect_back_or_admin_user(alert: _("Error occurred. User was not blocked")) diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index d3b0304f2c7..1ce76fd57b1 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -717,6 +717,6 @@ module ProjectsHelper def settings_container_registry_expiration_policy_available?(project) Feature.enabled?(:registry_retention_policies_settings, project) && Gitlab.config.registry.enabled && - can?(current_user, :read_container_image, project) + can?(current_user, :destroy_container_image, project) end end diff --git a/app/models/commit.rb b/app/models/commit.rb index 46222bbc4cd..d8a3bbfeeb2 100644 --- a/app/models/commit.rb +++ b/app/models/commit.rb @@ -244,6 +244,8 @@ class Commit # Discover issues should be closed when this commit is pushed to a project's # default branch. def closes_issues(current_user = self.committer) + return unless repository.repo_type.project? + Gitlab::ClosingIssueExtractor.new(project, current_user).closed_by_message(safe_message) end @@ -297,7 +299,11 @@ class Commit end def merge_requests - @merge_requests ||= project&.merge_requests&.by_commit_sha(sha) + strong_memoize(:merge_requests) do + next MergeRequest.none unless repository.repo_type.project? && project + + project.merge_requests.by_commit_sha(sha) + end end def method_missing(method, *args, &block) @@ -507,7 +513,7 @@ class Commit end def commit_reference(from, referable_commit_id, full: false) - base = project&.to_reference_base(from, full: full) + base = container.to_reference_base(from, full: full) if base.present? "#{base}#{self.class.reference_prefix}#{referable_commit_id}" diff --git a/app/models/concerns/has_repository.rb b/app/models/concerns/has_repository.rb index 66c2f57bedd..d04a6408a21 100644 --- a/app/models/concerns/has_repository.rb +++ b/app/models/concerns/has_repository.rb @@ -1,9 +1,17 @@ # frozen_string_literal: true +# This concern is created to handle repository actions. +# It should be include inside any object capable +# of directly having a repository, like project or snippet. +# +# It also includes `Referable`, therefore the method +# `to_reference` should be overriden in case the object +# needs any special behavior. module HasRepository extend ActiveSupport::Concern include Gitlab::ShellAdapter include AfterCommitQueue + include Referable include Gitlab::Utils::StrongMemoize delegate :base_dir, :disk_path, to: :storage diff --git a/app/models/personal_snippet.rb b/app/models/personal_snippet.rb index 1b5be8698b1..5940265b17a 100644 --- a/app/models/personal_snippet.rb +++ b/app/models/personal_snippet.rb @@ -2,4 +2,8 @@ class PersonalSnippet < Snippet include WithUploads + + def web_url(only_path: nil) + Gitlab::Routing.url_helpers.snippet_url(self, only_path: only_path) + end end diff --git a/app/models/project.rb b/app/models/project.rb index 0ed2510dbf4..bc652a19986 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -9,7 +9,6 @@ class Project < ApplicationRecord include AccessRequestable include Avatarable include CacheMarkdownField - include Referable include Sortable include AfterCommitQueue include CaseSensitivity @@ -2336,6 +2335,10 @@ class Project < ApplicationRecord false end + def self_monitoring? + Gitlab::CurrentSettings.self_monitoring_project_id == id + end + private def closest_namespace_setting(name) diff --git a/app/models/project_snippet.rb b/app/models/project_snippet.rb index ffb08e10f1f..6045ec71c6e 100644 --- a/app/models/project_snippet.rb +++ b/app/models/project_snippet.rb @@ -5,4 +5,8 @@ class ProjectSnippet < Snippet validates :project, presence: true validates :secret, inclusion: { in: [false] } + + def web_url(only_path: nil) + Gitlab::Routing.url_helpers.project_snippet_url(project, self, only_path: only_path) + end end diff --git a/app/models/repository.rb b/app/models/repository.rb index 37a20404ae7..c439d0700f1 100644 --- a/app/models/repository.rb +++ b/app/models/repository.rb @@ -1131,7 +1131,11 @@ class Repository end def project - container + if repo_type.snippet? + container.project + else + container + end end private @@ -1145,7 +1149,7 @@ class Repository Gitlab::Git::Commit.find(raw_repository, oid_or_ref) end - ::Commit.new(commit, project) if commit + ::Commit.new(commit, container) if commit end def cache diff --git a/app/models/snippet.rb b/app/models/snippet.rb index 77ec683f584..e2b72dfde7a 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -6,7 +6,6 @@ class Snippet < ApplicationRecord include CacheMarkdownField include Noteable include Participable - include Referable include Sortable include Awardable include Mentionable @@ -15,10 +14,11 @@ class Snippet < ApplicationRecord include Gitlab::SQL::Pattern include FromUnion include IgnorableColumns - + include HasRepository extend ::Gitlab::Utils::Override ignore_column :storage_version, remove_with: '12.9', remove_after: '2020-03-22' + ignore_column :repository_storage, remove_with: '12.10', remove_after: '2020-04-22' cache_markdown_field :title, pipeline: :single_line cache_markdown_field :description @@ -42,6 +42,7 @@ class Snippet < ApplicationRecord has_many :notes, as: :noteable, dependent: :destroy # rubocop:disable Cop/ActiveRecordDependent has_many :user_mentions, class_name: "SnippetUserMention" + has_one :snippet_repository, inverse_of: :snippet delegate :name, :email, to: :author, prefix: true, allow_nil: true @@ -254,6 +255,47 @@ class Snippet < ApplicationRecord super end + def repository + @repository ||= Repository.new(full_path, self, disk_path: disk_path, repo_type: Gitlab::GlRepository::SNIPPET) + end + + def storage + @storage ||= Storage::Hashed.new(self, prefix: Storage::Hashed::SNIPPET_REPOSITORY_PATH_PREFIX) + end + + # This is the full_path used to identify the + # the snippet repository. It will be used mostly + # for logging purposes. + def full_path + return unless persisted? + + @full_path ||= begin + components = [] + components << project.full_path if project_id? + components << '@snippets' + components << self.id + components.join('/') + end + end + + def repository_storage + snippet_repository&.shard_name || + Gitlab::CurrentSettings.pick_repository_storage + end + + def create_repository + return if repository_exists? + + repository.create_if_not_exists + + track_snippet_repository if repository_exists? + end + + def track_snippet_repository + repository = snippet_repository || build_snippet_repository + repository.update!(shard_name: repository_storage, disk_path: disk_path) + end + class << self # Searches for snippets with a matching title or file name. # diff --git a/app/models/snippet_repository.rb b/app/models/snippet_repository.rb new file mode 100644 index 00000000000..ba2a061a5f4 --- /dev/null +++ b/app/models/snippet_repository.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class SnippetRepository < ApplicationRecord + include Shardable + + belongs_to :snippet, inverse_of: :snippet_repository + + class << self + def find_snippet(disk_path) + find_by(disk_path: disk_path)&.snippet + end + end +end diff --git a/app/models/storage/hashed.rb b/app/models/storage/hashed.rb index 898e75194db..3dea50ab98b 100644 --- a/app/models/storage/hashed.rb +++ b/app/models/storage/hashed.rb @@ -2,14 +2,15 @@ module Storage class Hashed - attr_accessor :project - delegate :gitlab_shell, :repository_storage, to: :project + attr_accessor :container + delegate :gitlab_shell, :repository_storage, to: :container REPOSITORY_PATH_PREFIX = '@hashed' + SNIPPET_REPOSITORY_PATH_PREFIX = '@snippets' POOL_PATH_PREFIX = '@pools' - def initialize(project, prefix: REPOSITORY_PATH_PREFIX) - @project = project + def initialize(container, prefix: REPOSITORY_PATH_PREFIX) + @container = container @prefix = prefix end @@ -20,9 +21,10 @@ module Storage "#{@prefix}/#{disk_hash[0..1]}/#{disk_hash[2..3]}" if disk_hash end - # Disk path is used to build repository and project's wiki path on disk + # Disk path is used to build repository path on disk # - # @return [String] combination of base_dir and the repository own name without `.git` or `.wiki.git` extensions + # @return [String] combination of base_dir and the repository own name + # without `.git`, `.wiki.git`, or any other extension def disk_path "#{base_dir}/#{disk_hash}" if disk_hash end @@ -33,10 +35,10 @@ module Storage private - # Generates the hash for the project path and name on disk + # Generates the hash for the repository path and name on disk # If you need to refer to the repository on disk, use the `#disk_path` def disk_hash - @disk_hash ||= Digest::SHA2.hexdigest(project.id.to_s) if project.id + @disk_hash ||= Digest::SHA2.hexdigest(container.id.to_s) if container.id end end end diff --git a/app/services/boards/list_service.rb b/app/services/boards/list_service.rb index c6dfd62804f..729bca6580e 100644 --- a/app/services/boards/list_service.rb +++ b/app/services/boards/list_service.rb @@ -5,13 +5,7 @@ module Boards def execute(create_default_board: true) create_board! if create_default_board && parent.boards.empty? - if parent.multiple_issue_boards_available? - boards - else - # When multiple issue boards are not available - # a user is only allowed to view the default shown board - first_board - end + find_boards end private @@ -27,5 +21,18 @@ module Boards def create_board! Boards::CreateService.new(parent, current_user).execute end + + def find_boards + found = + if parent.multiple_issue_boards_available? + boards + else + # When multiple issue boards are not available + # a user is only allowed to view the default shown board + first_board + end + + params[:board_id].present? ? [found.find(params[:board_id])] : found + end end end diff --git a/app/services/container_expiration_policy_service.rb b/app/services/container_expiration_policy_service.rb index 5d141d4d64d..82274fd8668 100644 --- a/app/services/container_expiration_policy_service.rb +++ b/app/services/container_expiration_policy_service.rb @@ -6,9 +6,11 @@ class ContainerExpirationPolicyService < BaseService container_expiration_policy.container_repositories.find_each do |container_repository| CleanupContainerRepositoryWorker.perform_async( - current_user.id, + nil, container_repository.id, - container_expiration_policy.attributes.except("created_at", "updated_at") + container_expiration_policy.attributes + .except('created_at', 'updated_at') + .merge(container_expiration_policy: true) ) end end diff --git a/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb b/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb new file mode 100644 index 00000000000..d705c3f3ce5 --- /dev/null +++ b/app/services/metrics/dashboard/self_monitoring_dashboard_service.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +# Fetches the self monitoring metrics dashboard and formats the output. +# Use Gitlab::Metrics::Dashboard::Finder to retrieve dashboards. +module Metrics + module Dashboard + class SelfMonitoringDashboardService < ::Metrics::Dashboard::PredefinedDashboardService + DASHBOARD_PATH = 'config/prometheus/self_monitoring_default.yml' + DASHBOARD_NAME = 'Default' + + SEQUENCE = [ + STAGES::ProjectMetricsInserter, + STAGES::EndpointInserter, + STAGES::Sorter + ].freeze + + class << self + def valid_params?(params) + matching_dashboard?(params[:dashboard_path]) || self_monitoring_project?(params) + end + + def all_dashboard_paths(_project) + [{ + path: DASHBOARD_PATH, + display_name: DASHBOARD_NAME, + default: true, + system_dashboard: false + }] + end + + def self_monitoring_project?(params) + params[:dashboard_path].nil? && params[:environment]&.project&.self_monitoring? + end + end + end + end +end diff --git a/app/services/projects/container_repository/cleanup_tags_service.rb b/app/services/projects/container_repository/cleanup_tags_service.rb index b995df12e56..046745d725e 100644 --- a/app/services/projects/container_repository/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_service.rb @@ -5,7 +5,7 @@ module Projects class CleanupTagsService < BaseService def execute(container_repository) return error('feature disabled') unless can_use? - return error('access denied') unless can_admin? + return error('access denied') unless can_destroy? tags = container_repository.tags tags_by_digest = group_by_digest(tags) @@ -82,8 +82,10 @@ module Projects end end - def can_admin? - can?(current_user, :admin_container_image, project) + def can_destroy? + return true if params['container_expiration_policy'] + + can?(current_user, :destroy_container_image, project) end def can_use? diff --git a/app/services/projects/container_repository/delete_tags_service.rb b/app/services/projects/container_repository/delete_tags_service.rb index d19f275e928..21081bd077f 100644 --- a/app/services/projects/container_repository/delete_tags_service.rb +++ b/app/services/projects/container_repository/delete_tags_service.rb @@ -42,7 +42,7 @@ module Projects # Deletes the dummy image # All created tag digests are the same since they all have the same dummy image. # a single delete is sufficient to remove all tags with it - if deleted_tags.any? && container_repository.delete_tag_by_digest(deleted_tags.values.first) + if deleted_tags.any? && container_repository.delete_tag_by_digest(deleted_tags.each_value.first) success(deleted: deleted_tags.keys) else error('could not delete tags') diff --git a/app/services/users/block_service.rb b/app/services/users/block_service.rb new file mode 100644 index 00000000000..9c393832d8f --- /dev/null +++ b/app/services/users/block_service.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +module Users + class BlockService < BaseService + def initialize(current_user) + @current_user = current_user + end + + def execute(user) + if user.block + after_block_hook(user) + success + else + messages = user.errors.full_messages + error(messages.uniq.join('. ')) + end + end + + private + + def after_block_hook(user) + # overriden by EE module + end + end +end + +Users::BlockService.prepend_if_ee('EE::Users::BlockService') diff --git a/app/views/dashboard/_activities.html.haml b/app/views/dashboard/_activities.html.haml index 4359a2c3c2b..2db3e35250f 100644 --- a/app/views/dashboard/_activities.html.haml +++ b/app/views/dashboard/_activities.html.haml @@ -5,4 +5,5 @@ %i.fa.fa-rss .content_list -= spinner +.loading + .spinner.spinner-md diff --git a/app/views/projects/snippets/edit.html.haml b/app/views/projects/snippets/edit.html.haml index 6dbd67df886..9f5af1cfe1e 100644 --- a/app/views/projects/snippets/edit.html.haml +++ b/app/views/projects/snippets/edit.html.haml @@ -1,6 +1,7 @@ - add_to_breadcrumbs _("Snippets"), project_snippets_path(@project) - breadcrumb_title @snippet.to_reference - page_title _("Edit"), "#{@snippet.title} (#{@snippet.to_reference})", _("Snippets") +- @content_class = "limit-container-width" unless fluid_layout %h3.page-title = _("Edit Snippet") diff --git a/app/views/projects/snippets/new.html.haml b/app/views/projects/snippets/new.html.haml index d64e3a49a81..d55a1160d48 100644 --- a/app/views/projects/snippets/new.html.haml +++ b/app/views/projects/snippets/new.html.haml @@ -1,6 +1,7 @@ - add_to_breadcrumbs _("Snippets"), project_snippets_path(@project) - breadcrumb_title _("New") - page_title _("New Snippet") +- @content_class = "limit-container-width" unless fluid_layout %h3.page-title = _("New Snippet") diff --git a/app/views/shared/snippets/_form.html.haml b/app/views/shared/snippets/_form.html.haml index 73401029da4..f867fb2b6f7 100644 --- a/app/views/shared/snippets/_form.html.haml +++ b/app/views/shared/snippets/_form.html.haml @@ -6,27 +6,37 @@ html: { class: "snippet-form js-requires-input js-quick-submit common-note-form" } do |f| = form_errors(@snippet) - .form-group.row - .col-sm-2.col-form-label - = f.label :title - .col-sm-10 - = f.text_field :title, class: 'form-control qa-snippet-title', required: true, autofocus: true - - = render 'shared/form_elements/description', model: @snippet, project: @project, form: f - - = render 'shared/old_visibility_level', f: f, visibility_level: @snippet.visibility_level, can_change_visibility_level: true, form_model: @snippet, with_label: false - - .file-editor - .form-group.row - .col-sm-2.col-form-label - = f.label :file_name, "File" - .col-sm-10 - .file-holder.snippet - .js-file-title.file-title-flex-parent - = f.text_field :file_name, placeholder: "Optionally name this file to add code highlighting, e.g. example.rb for Ruby.", class: 'form-control snippet-file-name qa-snippet-file-name' - .file-content.code - %pre#editor= @snippet.content - = f.hidden_field :content, class: 'snippet-file-content' + .form-group + = f.label :title, class: 'label-bold' + = f.text_field :title, class: 'form-control qa-snippet-title', required: true, autofocus: true + + .form-group.js-description-input + - description_placeholder = s_('Snippets|Optionally add a description about what your snippet does or how to use it...') + - is_expanded = @snippet.description && !@snippet.description.empty? + = f.label :description, s_("Snippets|Description (optional)"), class: 'label-bold' + .js-collapsible-input + .js-collapsed{ class: ('d-none' if is_expanded) } + = text_field_tag nil, nil, class: 'form-control', placeholder: description_placeholder + .js-expanded{ class: ('d-none' if !is_expanded) } + = render layout: 'projects/md_preview', locals: { url: preview_markdown_path(@project), referenced_users: true } do + = render 'projects/zen', f: f, attr: :description, classes: 'note-textarea', placeholder: description_placeholder + = render 'shared/notes/hints' + + .form-group.file-editor + = f.label :file_name, s_('Snippets|File') + .file-holder.snippet + .js-file-title.file-title-flex-parent + = f.text_field :file_name, placeholder: s_("Snippets|Give your file a name to add code highlighting, e.g. example.rb for Ruby"), class: 'form-control snippet-file-name qa-snippet-file-name' + .file-content.code + %pre#editor= @snippet.content + = f.hidden_field :content, class: 'snippet-file-content' + + .form-group + .font-weight-bold + = _('Visibility level') + = link_to icon('question-circle'), help_page_path("public_access/public_access"), target: '_blank' + = render 'shared/visibility_level', f: f, visibility_level: @snippet.visibility_level, can_change_visibility_level: true, form_model: @snippet, with_label: false + - if params[:files] - params[:files].each_with_index do |file, index| = hidden_field_tag "files[]", file, id: "files_#{index}" diff --git a/app/views/snippets/edit.html.haml b/app/views/snippets/edit.html.haml index f5ffb037152..66f5e8148e1 100644 --- a/app/views/snippets/edit.html.haml +++ b/app/views/snippets/edit.html.haml @@ -1,4 +1,5 @@ - page_title _("Edit"), "#{@snippet.title} (#{@snippet.to_reference})", _("Snippets") +- @content_class = "limit-container-width" unless fluid_layout %h3.page-title = _("Edit Snippet") diff --git a/app/views/snippets/new.html.haml b/app/views/snippets/new.html.haml index 9d462865471..acc0ce0fff3 100644 --- a/app/views/snippets/new.html.haml +++ b/app/views/snippets/new.html.haml @@ -1,6 +1,7 @@ - @hide_top_links = true - @hide_breadcrumbs = true - page_title _("New Snippet") +- @content_class = "limit-container-width" unless fluid_layout .page-title-holder.d-flex.align-items-center %h1.page-title= _('New Snippet') diff --git a/app/workers/cleanup_container_repository_worker.rb b/app/workers/cleanup_container_repository_worker.rb index 83fb3e58d29..83397a1dda2 100644 --- a/app/workers/cleanup_container_repository_worker.rb +++ b/app/workers/cleanup_container_repository_worker.rb @@ -11,6 +11,7 @@ class CleanupContainerRepositoryWorker def perform(current_user_id, container_repository_id, params) @current_user = User.find_by_id(current_user_id) @container_repository = ContainerRepository.find_by_id(container_repository_id) + @params = params return unless valid? @@ -22,9 +23,15 @@ class CleanupContainerRepositoryWorker private def valid? + return true if run_by_container_expiration_policy? + current_user && container_repository && project end + def run_by_container_expiration_policy? + @params['container_expiration_policy'] && container_repository && project + end + def project container_repository&.project end diff --git a/changelogs/unreleased/121719-self-monitoring-default.yml b/changelogs/unreleased/121719-self-monitoring-default.yml new file mode 100644 index 00000000000..8c9a9889902 --- /dev/null +++ b/changelogs/unreleased/121719-self-monitoring-default.yml @@ -0,0 +1,5 @@ +--- +title: Set default dashboard for self monitoring project +merge_request: 24814 +author: +type: added diff --git a/changelogs/unreleased/202696-only-display-y-axis-of-data-value-ranges-for-charts.yml b/changelogs/unreleased/202696-only-display-y-axis-of-data-value-ranges-for-charts.yml new file mode 100644 index 00000000000..644ab2d5b25 --- /dev/null +++ b/changelogs/unreleased/202696-only-display-y-axis-of-data-value-ranges-for-charts.yml @@ -0,0 +1,5 @@ +--- +title: Display the y-axis on the range of data value in the chart +merge_request: 24953 +author: +type: added diff --git a/changelogs/unreleased/32714-snippets-ux-improvement.yml b/changelogs/unreleased/32714-snippets-ux-improvement.yml new file mode 100644 index 00000000000..3858bec98e1 --- /dev/null +++ b/changelogs/unreleased/32714-snippets-ux-improvement.yml @@ -0,0 +1,5 @@ +--- +title: Improve UX of optional fields in Snippets form +merge_request: 24762 +author: +type: other diff --git a/changelogs/unreleased/fj-create-snippet-repository-model.yml b/changelogs/unreleased/fj-create-snippet-repository-model.yml new file mode 100644 index 00000000000..5a2b8abbf02 --- /dev/null +++ b/changelogs/unreleased/fj-create-snippet-repository-model.yml @@ -0,0 +1,5 @@ +--- +title: Create snippet repository model +merge_request: 23796 +author: +type: added diff --git a/changelogs/unreleased/leipert-fix-user-label-bug.yml b/changelogs/unreleased/leipert-fix-user-label-bug.yml new file mode 100644 index 00000000000..67e1a4011af --- /dev/null +++ b/changelogs/unreleased/leipert-fix-user-label-bug.yml @@ -0,0 +1,5 @@ +--- +title: Fix autocomplete limitation bug +merge_request: 25127 +author: +type: fixed diff --git a/changelogs/unreleased/refactoring-entities-file-18.yml b/changelogs/unreleased/refactoring-entities-file-18.yml new file mode 100644 index 00000000000..b05d4ba7afe --- /dev/null +++ b/changelogs/unreleased/refactoring-entities-file-18.yml @@ -0,0 +1,5 @@ +--- +title: Separate board, list and other entities into own class files +merge_request: 24939 +author: Rajendra Kadam +type: added diff --git a/changelogs/unreleased/refactoring-entities-file-20.yml b/changelogs/unreleased/refactoring-entities-file-20.yml new file mode 100644 index 00000000000..c2715f08a99 --- /dev/null +++ b/changelogs/unreleased/refactoring-entities-file-20.yml @@ -0,0 +1,5 @@ +--- +title: Separate tag and release entities into own class files +merge_request: 24943 +author: Rajendra Kadam +type: added diff --git a/db/migrate/20200206112850_create_snippet_repository_table.rb b/db/migrate/20200206112850_create_snippet_repository_table.rb new file mode 100644 index 00000000000..0c14b37855d --- /dev/null +++ b/db/migrate/20200206112850_create_snippet_repository_table.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class CreateSnippetRepositoryTable < ActiveRecord::Migration[6.0] + DOWNTIME = false + + def change + create_table :snippet_repositories, id: false, primary_key: :snippet_id do |t| + t.references :shard, null: false, index: true, foreign_key: { on_delete: :restrict } + t.references :snippet, primary_key: true, default: nil, index: false, foreign_key: { on_delete: :cascade } + t.string :disk_path, limit: 80, null: false, index: { unique: true } + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 90f2bb5cf0f..2ab9d456531 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -3892,6 +3892,13 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do t.index ["user_id"], name: "index_smartcard_identities_on_user_id" end + create_table "snippet_repositories", primary_key: "snippet_id", id: :bigint, default: nil, force: :cascade do |t| + t.bigint "shard_id", null: false + t.string "disk_path", limit: 80, null: false + t.index ["disk_path"], name: "index_snippet_repositories_on_disk_path", unique: true + t.index ["shard_id"], name: "index_snippet_repositories_on_shard_id" + end + create_table "snippet_user_mentions", force: :cascade do |t| t.integer "snippet_id", null: false t.integer "note_id" @@ -4963,6 +4970,8 @@ ActiveRecord::Schema.define(version: 2020_02_12_052620) do add_foreign_key "services", "projects", name: "fk_71cce407f9", on_delete: :cascade add_foreign_key "slack_integrations", "services", on_delete: :cascade add_foreign_key "smartcard_identities", "users", on_delete: :cascade + add_foreign_key "snippet_repositories", "shards", on_delete: :restrict + add_foreign_key "snippet_repositories", "snippets", on_delete: :cascade add_foreign_key "snippet_user_mentions", "notes", on_delete: :cascade add_foreign_key "snippet_user_mentions", "snippets", on_delete: :cascade add_foreign_key "snippets", "projects", name: "fk_be41fd4bb7", on_delete: :cascade diff --git a/doc/administration/audit_events.md b/doc/administration/audit_events.md index a086a26a370..e46b74163f3 100644 --- a/doc/administration/audit_events.md +++ b/doc/administration/audit_events.md @@ -107,6 +107,7 @@ recorded: - Started/stopped user impersonation - Changed username ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/7797) in GitLab 12.8) - User was deleted ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/251) in GitLab 12.8) +- User was blocked via Admin Area ([introduced](https://gitlab.com/gitlab-org/gitlab/issues/251) in GitLab 12.8) It is possible to filter particular actions by choosing an audit data type from the filter dropdown box. You can further filter by specific group, project or user diff --git a/doc/install/aws/index.md b/doc/install/aws/index.md index a8ed5e78b1b..096d724717e 100644 --- a/doc/install/aws/index.md +++ b/doc/install/aws/index.md @@ -357,15 +357,7 @@ In this step we'll configure some details: ### Add storage -The root volume is 8GB by default and should be enough given that we won't store -any data there. Let's create a new EBS volume that will host the Git data. Its -size depends on your needs and you can always migrate to a bigger volume later. -You will be able to [set up that volume](#setting-up-the-ebs-volume) -after the instance is created. - -CAUTION: **Caution:** -We **do not** recommend using the AWS Elastic File System (EFS), as it can result -in [significantly degraded performance](../../administration/high_availability/nfs.md#avoid-using-awss-elastic-file-system-efs). +The root volume is 8GB by default and should be enough given that we won't store any data there. ### Configure security group @@ -490,33 +482,6 @@ sudo gitlab-ctl status If everything looks good, you should be able to reach GitLab in your browser. -### Setting up the EBS volume - -The EBS volume will host the Git repositories data: - -1. First, format the `/dev/xvdb` volume and then mount it under the directory - where the data will be stored. For example, `/mnt/gitlab-data/`. -1. Tell GitLab to store its data in the new directory by editing - `/etc/gitlab/gitlab.rb` with your editor: - - ```ruby - git_data_dirs({ - "default" => { "path" => "/mnt/gitlab-data" } - }) - ``` - - where `/mnt/gitlab-data` the location where you will store the Git data. - -1. Save the file and reconfigure GitLab: - - ```shell - sudo gitlab-ctl reconfigure - ``` - -TIP: **Tip:** -If you wish to add more than one data volumes to store the Git repositories, -read the [repository storage paths docs](../../administration/repository_storage_paths.md). - ### Setting up Gitaly CAUTION: **Caution:** In this architecture, having a single Gitaly server creates a single point of failure. This limitation will be removed once [Gitaly HA](https://gitlab.com/groups/gitlab-org/-/epics/842) is released. diff --git a/lib/api/entities.rb b/lib/api/entities.rb index 5d5a61b76f1..5128ffa6a1f 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -163,160 +163,6 @@ module API end end - class List < Grape::Entity - expose :id - expose :label, using: Entities::LabelBasic - expose :position - end - - class Board < Grape::Entity - expose :id - expose :project, using: Entities::BasicProjectDetails - - expose :lists, using: Entities::List do |board| - board.destroyable_lists - end - end - - class Compare < Grape::Entity - expose :commit, using: Entities::Commit do |compare, _| - compare.commits.last - end - - expose :commits, using: Entities::Commit do |compare, _| - compare.commits - end - - expose :diffs, using: Entities::Diff do |compare, _| - compare.diffs.diffs.to_a - end - - expose :compare_timeout do |compare, _| - compare.diffs.diffs.overflow? - end - - expose :same, as: :compare_same_ref - end - - class Contributor < Grape::Entity - expose :name, :email, :commits, :additions, :deletions - end - - class BroadcastMessage < Grape::Entity - expose :message, :starts_at, :ends_at, :color, :font, :target_path, :broadcast_type - end - - class Release < Grape::Entity - include ::API::Helpers::Presentable - - expose :name do |release, _| - can_download_code? ? release.name : "Release-#{release.id}" - end - expose :tag, as: :tag_name, if: ->(_, _) { can_download_code? } - expose :description - expose :description_html do |entity| - MarkupHelper.markdown_field(entity, :description) - end - expose :created_at - expose :released_at - expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? } - expose :commit, using: Entities::Commit, if: ->(_, _) { can_download_code? } - expose :upcoming_release?, as: :upcoming_release - expose :milestones, using: Entities::Milestone, if: -> (release, _) { release.milestones.present? && can_read_milestone? } - expose :commit_path, expose_nil: false - expose :tag_path, expose_nil: false - expose :evidence_sha, expose_nil: false, if: ->(_, _) { can_download_code? } - expose :assets do - expose :assets_count, as: :count do |release, _| - assets_to_exclude = can_download_code? ? [] : [:sources] - release.assets_count(except: assets_to_exclude) - end - expose :sources, using: Entities::Releases::Source, if: ->(_, _) { can_download_code? } - expose :links, using: Entities::Releases::Link do |release, options| - release.links.sorted - end - expose :evidence_file_path, expose_nil: false, if: ->(_, _) { can_download_code? } - end - expose :_links do - expose :self_url, as: :self, expose_nil: false - expose :merge_requests_url, expose_nil: false - expose :issues_url, expose_nil: false - expose :edit_url, expose_nil: false - end - - private - - def can_download_code? - Ability.allowed?(options[:current_user], :download_code, object.project) - end - - def can_read_milestone? - Ability.allowed?(options[:current_user], :read_milestone, object.project) - end - end - - class Tag < Grape::Entity - expose :name, :message, :target - - expose :commit, using: Entities::Commit do |repo_tag, options| - options[:project].repository.commit(repo_tag.dereferenced_target) - end - - # rubocop: disable CodeReuse/ActiveRecord - expose :release, using: Entities::TagRelease do |repo_tag, options| - options[:project].releases.find_by(tag: repo_tag.name) - end - # rubocop: enable CodeReuse/ActiveRecord - - expose :protected do |repo_tag, options| - ::ProtectedTag.protected?(options[:project], repo_tag.name) - end - end - - class Runner < Grape::Entity - expose :id - expose :description - expose :ip_address - expose :active - expose :instance_type?, as: :is_shared - expose :name - expose :online?, as: :online - expose :status - end - - class RunnerDetails < Runner - expose :tag_list - expose :run_untagged - expose :locked - expose :maximum_timeout - expose :access_level - expose :version, :revision, :platform, :architecture - expose :contacted_at - expose :token, if: lambda { |runner, options| options[:current_user].admin? || !runner.instance_type? } - # rubocop: disable CodeReuse/ActiveRecord - expose :projects, with: Entities::BasicProjectDetails do |runner, options| - if options[:current_user].admin? - runner.projects - else - options[:current_user].authorized_projects.where(id: runner.projects) - end - end - # rubocop: enable CodeReuse/ActiveRecord - # rubocop: disable CodeReuse/ActiveRecord - expose :groups, with: Entities::BasicGroupDetails do |runner, options| - if options[:current_user].admin? - runner.groups - else - options[:current_user].authorized_groups.where(id: runner.groups) - end - end - # rubocop: enable CodeReuse/ActiveRecord - end - - class RunnerRegistrationDetails < Grape::Entity - expose :id, :token - end - class Trigger < Grape::Entity include ::API::Helpers::Presentable diff --git a/lib/api/entities/board.rb b/lib/api/entities/board.rb new file mode 100644 index 00000000000..afbf5b4b65b --- /dev/null +++ b/lib/api/entities/board.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module API + module Entities + class Board < Grape::Entity + expose :id + expose :project, using: Entities::BasicProjectDetails + + expose :lists, using: Entities::List do |board| + board.destroyable_lists + end + end + end +end diff --git a/lib/api/entities/broadcast_message.rb b/lib/api/entities/broadcast_message.rb index 31d4cc50526..403677aa300 100644 --- a/lib/api/entities/broadcast_message.rb +++ b/lib/api/entities/broadcast_message.rb @@ -3,7 +3,7 @@ module API module Entities class BroadcastMessage < Grape::Entity - expose :id, :message, :starts_at, :ends_at, :color, :font + expose :id, :message, :starts_at, :ends_at, :color, :font, :target_path, :broadcast_type expose :active?, as: :active end end diff --git a/lib/api/entities/compare.rb b/lib/api/entities/compare.rb new file mode 100644 index 00000000000..fe2f03db2af --- /dev/null +++ b/lib/api/entities/compare.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module API + module Entities + class Compare < Grape::Entity + expose :commit, using: Entities::Commit do |compare, _| + compare.commits.last + end + + expose :commits, using: Entities::Commit do |compare, _| + compare.commits + end + + expose :diffs, using: Entities::Diff do |compare, _| + compare.diffs.diffs.to_a + end + + expose :compare_timeout do |compare, _| + compare.diffs.diffs.overflow? + end + + expose :same, as: :compare_same_ref + end + end +end diff --git a/lib/api/entities/contributor.rb b/lib/api/entities/contributor.rb new file mode 100644 index 00000000000..8763822b674 --- /dev/null +++ b/lib/api/entities/contributor.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class Contributor < Grape::Entity + expose :name, :email, :commits, :additions, :deletions + end + end +end diff --git a/lib/api/entities/list.rb b/lib/api/entities/list.rb new file mode 100644 index 00000000000..e856359efc1 --- /dev/null +++ b/lib/api/entities/list.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +module API + module Entities + class List < Grape::Entity + expose :id + expose :label, using: Entities::LabelBasic + expose :position + end + end +end diff --git a/lib/api/entities/release.rb b/lib/api/entities/release.rb new file mode 100644 index 00000000000..dc4b91e594e --- /dev/null +++ b/lib/api/entities/release.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +module API + module Entities + class Release < Grape::Entity + include ::API::Helpers::Presentable + + expose :name do |release, _| + can_download_code? ? release.name : "Release-#{release.id}" + end + expose :tag, as: :tag_name, if: ->(_, _) { can_download_code? } + expose :description + expose :description_html do |entity| + MarkupHelper.markdown_field(entity, :description) + end + expose :created_at + expose :released_at + expose :author, using: Entities::UserBasic, if: -> (release, _) { release.author.present? } + expose :commit, using: Entities::Commit, if: ->(_, _) { can_download_code? } + expose :upcoming_release?, as: :upcoming_release + expose :milestones, using: Entities::Milestone, if: -> (release, _) { release.milestones.present? && can_read_milestone? } + expose :commit_path, expose_nil: false + expose :tag_path, expose_nil: false + expose :evidence_sha, expose_nil: false, if: ->(_, _) { can_download_code? } + expose :assets do + expose :assets_count, as: :count do |release, _| + assets_to_exclude = can_download_code? ? [] : [:sources] + release.assets_count(except: assets_to_exclude) + end + expose :sources, using: Entities::Releases::Source, if: ->(_, _) { can_download_code? } + expose :links, using: Entities::Releases::Link do |release, options| + release.links.sorted + end + expose :evidence_file_path, expose_nil: false, if: ->(_, _) { can_download_code? } + end + expose :_links do + expose :self_url, as: :self, expose_nil: false + expose :merge_requests_url, expose_nil: false + expose :issues_url, expose_nil: false + expose :edit_url, expose_nil: false + end + + private + + def can_download_code? + Ability.allowed?(options[:current_user], :download_code, object.project) + end + + def can_read_milestone? + Ability.allowed?(options[:current_user], :read_milestone, object.project) + end + end + end +end diff --git a/lib/api/entities/runner.rb b/lib/api/entities/runner.rb new file mode 100644 index 00000000000..6165b54cddb --- /dev/null +++ b/lib/api/entities/runner.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +module API + module Entities + class Runner < Grape::Entity + expose :id + expose :description + expose :ip_address + expose :active + expose :instance_type?, as: :is_shared + expose :name + expose :online?, as: :online + expose :status + end + end +end diff --git a/lib/api/entities/runner_details.rb b/lib/api/entities/runner_details.rb new file mode 100644 index 00000000000..17202821e6e --- /dev/null +++ b/lib/api/entities/runner_details.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module API + module Entities + class RunnerDetails < Runner + expose :tag_list + expose :run_untagged + expose :locked + expose :maximum_timeout + expose :access_level + expose :version, :revision, :platform, :architecture + expose :contacted_at + expose :token, if: lambda { |runner, options| options[:current_user].admin? || !runner.instance_type? } + # rubocop: disable CodeReuse/ActiveRecord + expose :projects, with: Entities::BasicProjectDetails do |runner, options| + if options[:current_user].admin? + runner.projects + else + options[:current_user].authorized_projects.where(id: runner.projects) + end + end + # rubocop: enable CodeReuse/ActiveRecord + # rubocop: disable CodeReuse/ActiveRecord + expose :groups, with: Entities::BasicGroupDetails do |runner, options| + if options[:current_user].admin? + runner.groups + else + options[:current_user].authorized_groups.where(id: runner.groups) + end + end + # rubocop: enable CodeReuse/ActiveRecord + end + end +end diff --git a/lib/api/entities/runner_registration_details.rb b/lib/api/entities/runner_registration_details.rb new file mode 100644 index 00000000000..c8ed88ba10a --- /dev/null +++ b/lib/api/entities/runner_registration_details.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +module API + module Entities + class RunnerRegistrationDetails < Grape::Entity + expose :id, :token + end + end +end diff --git a/lib/api/entities/tag.rb b/lib/api/entities/tag.rb new file mode 100644 index 00000000000..2d3569bb9bb --- /dev/null +++ b/lib/api/entities/tag.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module API + module Entities + class Tag < Grape::Entity + expose :name, :message, :target + + expose :commit, using: Entities::Commit do |repo_tag, options| + options[:project].repository.commit(repo_tag.dereferenced_target) + end + + # rubocop: disable CodeReuse/ActiveRecord + expose :release, using: Entities::TagRelease do |repo_tag, options| + options[:project].releases.find_by(tag: repo_tag.name) + end + # rubocop: enable CodeReuse/ActiveRecord + + expose :protected do |repo_tag, options| + ::ProtectedTag.protected?(options[:project], repo_tag.name) + end + end + end +end diff --git a/lib/api/project_container_repositories.rb b/lib/api/project_container_repositories.rb index 2b33069e324..e2963c0de22 100644 --- a/lib/api/project_container_repositories.rb +++ b/lib/api/project_container_repositories.rb @@ -80,7 +80,7 @@ module API render_api_error!(message, 400) unless obtain_new_cleanup_container_lease CleanupContainerRepositoryWorker.perform_async(current_user.id, repository.id, - declared_params.except(:repository_id)) + declared_params.except(:repository_id).merge(container_expiration_policy: false)) track_event('delete_tag_bulk') diff --git a/lib/gitlab/git_access_snippet.rb b/lib/gitlab/git_access_snippet.rb new file mode 100644 index 00000000000..d99b9c3fe89 --- /dev/null +++ b/lib/gitlab/git_access_snippet.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +module Gitlab + class GitAccessSnippet < GitAccess + ERROR_MESSAGES = { + snippet_not_found: 'The snippet you were looking for could not be found.', + repository_not_found: 'The snippet repository you were looking for could not be found.' + }.freeze + + attr_reader :snippet + + def initialize(actor, snippet, protocol, **kwargs) + @snippet = snippet + + super(actor, project, protocol, **kwargs) + end + + def check(cmd, _changes) + unless Feature.enabled?(:version_snippets, user) + raise NotFoundError, ERROR_MESSAGES[:snippet_not_found] + end + + check_snippet_accessibility! + + success_result(cmd) + end + + def project + snippet&.project + end + + private + + def repository + snippet&.repository + end + + def check_snippet_accessibility! + if snippet.blank? + raise NotFoundError, ERROR_MESSAGES[:snippet_not_found] + end + + unless repository&.exists? + raise NotFoundError, ERROR_MESSAGES[:repository_not_found] + end + end + end +end diff --git a/lib/gitlab/gl_repository.rb b/lib/gitlab/gl_repository.rb index 347084e779c..fcebcb463cd 100644 --- a/lib/gitlab/gl_repository.rb +++ b/lib/gitlab/gl_repository.rb @@ -15,10 +15,17 @@ module Gitlab repository_resolver: -> (project) { project.wiki.repository }, suffix: :wiki ).freeze + SNIPPET = RepoType.new( + name: :snippet, + access_checker_class: Gitlab::GitAccessSnippet, + repository_resolver: -> (snippet) { snippet.repository }, + container_resolver: -> (id) { Snippet.find_by_id(id) } + ).freeze TYPES = { PROJECT.name.to_s => PROJECT, - WIKI.name.to_s => WIKI + WIKI.name.to_s => WIKI, + SNIPPET.name.to_s => SNIPPET }.freeze def self.types diff --git a/lib/gitlab/gl_repository/repo_type.rb b/lib/gitlab/gl_repository/repo_type.rb index 20a0dd48468..9663fd7de8f 100644 --- a/lib/gitlab/gl_repository/repo_type.rb +++ b/lib/gitlab/gl_repository/repo_type.rb @@ -47,6 +47,10 @@ module Gitlab self == PROJECT end + def snippet? + self == SNIPPET + end + def path_suffix suffix ? ".#{suffix}" : '' end diff --git a/lib/gitlab/metrics/dashboard/finder.rb b/lib/gitlab/metrics/dashboard/finder.rb index 0374484d357..3dd86c8685d 100644 --- a/lib/gitlab/metrics/dashboard/finder.rb +++ b/lib/gitlab/metrics/dashboard/finder.rb @@ -65,7 +65,7 @@ module Gitlab def find_all_paths_from_source(project) Gitlab::Metrics::Dashboard::Cache.delete_all! - system_service.all_dashboard_paths(project) + default_dashboard_path(project) .+ project_service.all_dashboard_paths(project) end @@ -79,6 +79,18 @@ module Gitlab ::Metrics::Dashboard::ProjectDashboardService end + def self_monitoring_service + ::Metrics::Dashboard::SelfMonitoringDashboardService + end + + def default_dashboard_path(project) + if project.self_monitoring? + self_monitoring_service.all_dashboard_paths(project) + else + system_service.all_dashboard_paths(project) + end + end + def service_for(options) Gitlab::Metrics::Dashboard::ServiceSelector.call(options) end diff --git a/lib/gitlab/metrics/dashboard/service_selector.rb b/lib/gitlab/metrics/dashboard/service_selector.rb index 5a3007e4814..24ea85a5a95 100644 --- a/lib/gitlab/metrics/dashboard/service_selector.rb +++ b/lib/gitlab/metrics/dashboard/service_selector.rb @@ -18,6 +18,7 @@ module Gitlab ::Metrics::Dashboard::DefaultEmbedService, ::Metrics::Dashboard::SystemDashboardService, ::Metrics::Dashboard::PodDashboardService, + ::Metrics::Dashboard::SelfMonitoringDashboardService, ::Metrics::Dashboard::ProjectDashboardService ].freeze diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 657d6c75800..8b695e1b127 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5184,7 +5184,7 @@ msgstr "" msgid "ContainerRegistry|We are having trouble connecting to Docker, which could be due to an issue with your project name or path. %{docLinkStart}More Information%{docLinkEnd}" msgstr "" -msgid "ContainerRegistry|Wildcards such as %{codeStart}*-stable%{codeEnd} or %{codeStart}production/*%{codeEnd} are supported. To select all tags, use %{codeStart}.*%{codeEnd}" +msgid "ContainerRegistry|Wildcards such as %{codeStart}.*-stable%{codeEnd} or %{codeStart}production/.*%{codeEnd} are supported. To select all tags, use %{codeStart}.*%{codeEnd}" msgstr "" msgid "ContainerRegistry|With the Container Registry, every project can have its own space to store its Docker images. %{docLinkStart}More Information%{docLinkEnd}" @@ -17681,6 +17681,18 @@ msgstr "" msgid "SnippetsEmptyState|There are no snippets to show." msgstr "" +msgid "Snippets|Description (optional)" +msgstr "" + +msgid "Snippets|File" +msgstr "" + +msgid "Snippets|Give your file a name to add code highlighting, e.g. example.rb for Ruby" +msgstr "" + +msgid "Snippets|Optionally add a description about what your snippet does or how to use it..." +msgstr "" + msgid "Snowplow" msgstr "" diff --git a/package.json b/package.json index 3a5f4edb4ef..b6c2d56ca0a 100644 --- a/package.json +++ b/package.json @@ -201,6 +201,7 @@ "yarn-deduplicate": "^1.1.1" }, "resolutions": { + "at.js": "https://gitlab.com/gitlab-org/frontend/At.js.git#121ce9a557b51c33f5693ac8df52d2bda1e53cbe", "vue-jest/ts-jest": "24.0.0", "monaco-editor": "0.18.1" }, diff --git a/qa/qa/page/merge_request/show.rb b/qa/qa/page/merge_request/show.rb index 940b7f332c7..b69990bae9c 100644 --- a/qa/qa/page/merge_request/show.rb +++ b/qa/qa/page/merge_request/show.rb @@ -55,7 +55,7 @@ module QA element :diffs_tab end - view 'app/assets/javascripts/diffs/components/diff_line_gutter_content.vue' do + view 'app/assets/javascripts/diffs/components/diff_table_cell.vue' do element :diff_comment end diff --git a/rubocop/cop/gitlab/keys-first-and-values-first.rb b/rubocop/cop/gitlab/keys-first-and-values-first.rb new file mode 100644 index 00000000000..9b68957cba2 --- /dev/null +++ b/rubocop/cop/gitlab/keys-first-and-values-first.rb @@ -0,0 +1,55 @@ +module RuboCop + module Cop + module Gitlab + class KeysFirstAndValuesFirst < RuboCop::Cop::Cop + FIRST_PATTERN = /\Afirst\z/.freeze + + def message(used_method) + <<~MSG + Don't use `.keys.first` and `.values.first`. + Instead use `.each_key.first` and `.each_value.first` (or `.first.first` and `first.second`) + + This will reduce memory usage and execution time. + MSG + end + + def on_send(node) + if find_on_keys_or_values?(node) + add_offense(node, location: :selector, message: message(node.method_name)) + end + end + + def autocorrect(node) + lambda do |corrector| + replace_with = if node.descendants.first.method_name == :values + '.each_value' + elsif node.descendants.first.method_name == :keys + '.each_key' + else + throw("Expect '.values.first' or '.keys.first', but get #{node.descendants.first.method_name}.first") + end + + upto_including_keys_or_values = node.descendants.first.source_range + before_keys_or_values = node.descendants[1].source_range + range_to_replace = node.source_range + .with(begin_pos: before_keys_or_values.end_pos, + end_pos: upto_including_keys_or_values.end_pos) + corrector.replace(range_to_replace, replace_with) + end + end + + def find_on_keys_or_values?(node) + chained_on_node = node.descendants.first + node.method_name.to_s =~ FIRST_PATTERN && + chained_on_node.is_a?(RuboCop::AST::SendNode) && + [:keys, :values].include?(chained_on_node.method_name) && + node.descendants[1] + end + + def method_name_for_node(node) + children[1].to_s + end + end + end + end +end diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb index 5284cef5346..9305153dc7d 100644 --- a/rubocop/rubocop.rb +++ b/rubocop/rubocop.rb @@ -5,6 +5,7 @@ require_relative 'cop/gitlab/httparty' require_relative 'cop/gitlab/finder_with_find_by' require_relative 'cop/gitlab/union' require_relative 'cop/gitlab/rails_logger' +require_relative 'cop/gitlab/keys-first-and-values-first' require_relative 'cop/include_sidekiq_worker' require_relative 'cop/safe_params' require_relative 'cop/active_record_association_reload' diff --git a/spec/controllers/user_callouts_controller_spec.rb b/spec/controllers/user_callouts_controller_spec.rb index babc93a83e5..04f73749e1d 100644 --- a/spec/controllers/user_callouts_controller_spec.rb +++ b/spec/controllers/user_callouts_controller_spec.rb @@ -13,7 +13,7 @@ describe UserCalloutsController do subject { post :create, params: { feature_name: feature_name }, format: :json } context 'with valid feature name' do - let(:feature_name) { UserCallout.feature_names.keys.first } + let(:feature_name) { UserCallout.feature_names.each_key.first } context 'when callout entry does not exist' do it 'creates a callout entry with dismissed state' do @@ -28,7 +28,7 @@ describe UserCalloutsController do end context 'when callout entry already exists' do - let!(:callout) { create(:user_callout, feature_name: UserCallout.feature_names.keys.first, user: user) } + let!(:callout) { create(:user_callout, feature_name: UserCallout.feature_names.each_key.first, user: user) } it 'returns success' do subject diff --git a/spec/factories/snippet_repositories.rb b/spec/factories/snippet_repositories.rb new file mode 100644 index 00000000000..1f9e68514bb --- /dev/null +++ b/spec/factories/snippet_repositories.rb @@ -0,0 +1,12 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :snippet_repository do + snippet + + after(:build) do |snippet_repository, _| + snippet_repository.shard_name = snippet_repository.snippet.repository_storage + snippet_repository.disk_path = snippet_repository.snippet.disk_path + end + end +end diff --git a/spec/factories/snippets.rb b/spec/factories/snippets.rb index 5990ed7ffb0..6fcb0319748 100644 --- a/spec/factories/snippets.rb +++ b/spec/factories/snippets.rb @@ -20,6 +20,21 @@ FactoryBot.define do trait :private do visibility_level { Snippet::PRIVATE } end + + # Test repository - https://gitlab.com/gitlab-org/gitlab-test + trait :repository do + after :create do |snippet| + TestEnv.copy_repo(snippet, + bare_repo: TestEnv.factory_repo_path_bare, + refs: TestEnv::BRANCH_SHA) + end + end + + trait :empty_repo do + after(:create) do |snippet| + raise "Failed to create repository!" unless snippet.repository.create_if_not_exists + end + end end factory :project_snippet, parent: :snippet, class: :ProjectSnippet do diff --git a/spec/features/projects/snippets/create_snippet_spec.rb b/spec/features/projects/snippets/create_snippet_spec.rb index 94af023e804..5a425fb5d27 100644 --- a/spec/features/projects/snippets/create_snippet_spec.rb +++ b/spec/features/projects/snippets/create_snippet_spec.rb @@ -8,9 +8,17 @@ describe 'Projects > Snippets > Create Snippet', :js do let(:user) { create(:user) } let(:project) { create(:project, :public) } + def description_field + find('.js-description-input input,textarea') + end + def fill_form fill_in 'project_snippet_title', with: 'My Snippet Title' + + # Click placeholder first to expand full description field + description_field.click fill_in 'project_snippet_description', with: 'My Snippet **Description**' + page.within('.file-editor') do find('.ace_text-input', visible: false).send_keys('Hello World!') end @@ -27,6 +35,18 @@ describe 'Projects > Snippets > Create Snippet', :js do click_on('New snippet') end + it 'shows collapsible description input' do + collapsed = description_field + + expect(page).not_to have_field('project_snippet_description') + expect(collapsed).to be_visible + + collapsed.click + + expect(page).to have_field('project_snippet_description') + expect(collapsed).not_to be_visible + end + it 'creates a new snippet' do fill_form click_button('Create snippet') diff --git a/spec/features/snippets/spam_snippets_spec.rb b/spec/features/snippets/spam_snippets_spec.rb index 0c3ca6f17c8..dac36ba2b28 100644 --- a/spec/features/snippets/spam_snippets_spec.rb +++ b/spec/features/snippets/spam_snippets_spec.rb @@ -5,6 +5,10 @@ require 'spec_helper' describe 'User creates snippet', :js do let(:user) { create(:user) } + def description_field + find('.js-description-input input,textarea') + end + before do stub_feature_flags(allow_possible_spam: false) stub_feature_flags(snippets_vue: false) @@ -22,7 +26,11 @@ describe 'User creates snippet', :js do visit new_snippet_path fill_in 'personal_snippet_title', with: 'My Snippet Title' + + # Click placeholder first to expand full description field + description_field.click fill_in 'personal_snippet_description', with: 'My Snippet **Description**' + find('#personal_snippet_visibility_level_20').set(true) page.within('.file-editor') do find('.ace_text-input', visible: false).send_keys 'Hello World!' diff --git a/spec/features/snippets/user_creates_snippet_spec.rb b/spec/features/snippets/user_creates_snippet_spec.rb index b373264bbe4..eb55613b954 100644 --- a/spec/features/snippets/user_creates_snippet_spec.rb +++ b/spec/features/snippets/user_creates_snippet_spec.rb @@ -13,9 +13,17 @@ describe 'User creates snippet', :js do visit new_snippet_path end + def description_field + find('.js-description-input input,textarea') + end + def fill_form fill_in 'personal_snippet_title', with: 'My Snippet Title' + + # Click placeholder first to expand full description field + description_field.click fill_in 'personal_snippet_description', with: 'My Snippet **Description**' + page.within('.file-editor') do find('.ace_text-input', visible: false).send_keys 'Hello World!' end @@ -36,6 +44,8 @@ describe 'User creates snippet', :js do end it 'previews a snippet with file' do + # Click placeholder first to expand full description field + description_field.click fill_in 'personal_snippet_description', with: 'My Snippet' dropzone_file Rails.root.join('spec', 'fixtures', 'banana_sample.gif') find('.js-md-preview-button').click diff --git a/spec/frontend/diffs/components/diff_line_gutter_content_spec.js b/spec/frontend/diffs/components/diff_table_cell_spec.js index 0553498bfa0..1af0746f3bd 100644 --- a/spec/frontend/diffs/components/diff_line_gutter_content_spec.js +++ b/spec/frontend/diffs/components/diff_table_cell_spec.js @@ -1,6 +1,6 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils'; import Vuex from 'vuex'; -import { shallowMount, createLocalVue } from '@vue/test-utils'; -import DiffLineGutterContent from '~/diffs/components/diff_line_gutter_content.vue'; +import DiffTableCell from '~/diffs/components/diff_table_cell.vue'; import DiffGutterAvatars from '~/diffs/components/diff_gutter_avatars.vue'; import { LINE_POSITION_RIGHT } from '~/diffs/constants'; import { createStore } from '~/mr_notes/stores'; @@ -17,7 +17,7 @@ const TEST_LINE_NUMBER = 1; const TEST_LINE_CODE = 'LC_42'; const TEST_FILE_HASH = diffFileMockData.file_hash; -describe('DiffLineGutterContent', () => { +describe('DiffTableCell', () => { let wrapper; let line; let store; @@ -49,22 +49,40 @@ describe('DiffLineGutterContent', () => { value, }); }; + const createComponent = (props = {}) => { - wrapper = shallowMount(DiffLineGutterContent, { + wrapper = shallowMount(DiffTableCell, { localVue, store, propsData: { line, fileHash: TEST_FILE_HASH, contextLinesPath: '/context/lines/path', + isHighlighted: false, ...props, }, }); }; - const findNoteButton = () => wrapper.find('.js-add-diff-note-button'); + + const findTd = () => wrapper.find({ ref: 'td' }); + const findNoteButton = () => wrapper.find({ ref: 'addDiffNoteButton' }); const findLineNumber = () => wrapper.find({ ref: 'lineNumberRef' }); const findAvatars = () => wrapper.find(DiffGutterAvatars); + describe('td', () => { + it('highlights when isHighlighted true', () => { + createComponent({ isHighlighted: true }); + + expect(findTd().classes()).toContain('hll'); + }); + + it('does not highlight when isHighlighted false', () => { + createComponent({ isHighlighted: false }); + + expect(findTd().classes()).not.toContain('hll'); + }); + }); + describe('comment button', () => { it.each` showCommentButton | userData | query | expectation @@ -84,13 +102,13 @@ describe('DiffLineGutterContent', () => { ); it.each` - isHover | otherProps | discussions | expectation - ${true} | ${{}} | ${[]} | ${true} - ${false} | ${{}} | ${[]} | ${false} - ${true} | ${{ isMatchLine: true }} | ${[]} | ${false} - ${true} | ${{ isContextLine: true }} | ${[]} | ${false} - ${true} | ${{ isMetaLine: true }} | ${[]} | ${false} - ${true} | ${{}} | ${[{}]} | ${false} + isHover | otherProps | discussions | expectation + ${true} | ${{}} | ${[]} | ${true} + ${false} | ${{}} | ${[]} | ${false} + ${true} | ${{ line: { ...line, type: 'match' } }} | ${[]} | ${false} + ${true} | ${{ line: { ...line, type: 'context' } }} | ${[]} | ${false} + ${true} | ${{ line: { ...line, type: 'old-nonewline' } }} | ${[]} | ${false} + ${true} | ${{}} | ${[{}]} | ${false} `( 'visible is $expectation - with isHover ($isHover), discussions ($discussions), otherProps ($otherProps)', ({ isHover, otherProps, discussions, expectation }) => { @@ -109,7 +127,7 @@ describe('DiffLineGutterContent', () => { describe('line number', () => { describe('without lineNumber prop', () => { it('does not render', () => { - createComponent(); + createComponent({ lineType: 'old' }); expect(findLineNumber().exists()).toBe(false); }); diff --git a/spec/frontend/monitoring/components/charts/time_series_spec.js b/spec/frontend/monitoring/components/charts/time_series_spec.js index 0a7e3dca183..4871619c85a 100644 --- a/spec/frontend/monitoring/components/charts/time_series_spec.js +++ b/spec/frontend/monitoring/components/charts/time_series_spec.js @@ -74,6 +74,8 @@ describe('Time series component', () => { describe('general functions', () => { let timeSeriesChart; + const findChart = () => timeSeriesChart.find({ ref: 'chart' }); + beforeEach(done => { timeSeriesChart = makeTimeSeriesChart(mockGraphData, 'area-chart'); timeSeriesChart.vm.$nextTick(done); @@ -109,8 +111,6 @@ describe('Time series component', () => { let startValue; let endValue; - const findChart = () => timeSeriesChart.find({ ref: 'chart' }); - beforeEach(done => { eChartMock = { handlers: {}, @@ -285,6 +285,8 @@ describe('Time series component', () => { }); describe('computed', () => { + const getChartOptions = () => findChart().props('option'); + describe('chartData', () => { let chartData; const seriesData = () => chartData[0]; @@ -329,7 +331,7 @@ describe('Time series component', () => { }); return timeSeriesChart.vm.$nextTick().then(() => { - expect(timeSeriesChart.vm.chartOptions).toEqual(expect.objectContaining(mockOption)); + expect(getChartOptions()).toEqual(expect.objectContaining(mockOption)); }); }); @@ -345,7 +347,7 @@ describe('Time series component', () => { }); return timeSeriesChart.vm.$nextTick().then(() => { - const optionSeries = timeSeriesChart.vm.chartOptions.series; + const optionSeries = getChartOptions().series; expect(optionSeries.length).toEqual(2); expect(optionSeries[0].name).toEqual(mockSeriesName); @@ -354,33 +356,58 @@ describe('Time series component', () => { }); describe('yAxis formatter', () => { - let format; + let dataFormatter; + let deploymentFormatter; beforeEach(() => { - format = timeSeriesChart.vm.chartOptions.yAxis.axisLabel.formatter; + dataFormatter = getChartOptions().yAxis[0].axisLabel.formatter; + deploymentFormatter = getChartOptions().yAxis[1].axisLabel.formatter; }); it('rounds to 3 decimal places', () => { - expect(format(0.88888)).toBe('0.889'); + expect(dataFormatter(0.88888)).toBe('0.889'); + }); + + it('deployment formatter is set as is required to display a tooltip', () => { + expect(deploymentFormatter).toEqual(expect.any(Function)); }); }); }); - describe('scatterSeries', () => { + describe('deploymentSeries', () => { it('utilizes deployment data', () => { - expect(timeSeriesChart.vm.scatterSeries.data).toEqual([ - ['2019-07-16T10:14:25.589Z', 0], - ['2019-07-16T11:14:25.589Z', 0], - ['2019-07-16T12:14:25.589Z', 0], + expect(timeSeriesChart.vm.deploymentSeries.yAxisIndex).toBe(1); // same as deployment y axis + expect(timeSeriesChart.vm.deploymentSeries.data).toEqual([ + ['2019-07-16T10:14:25.589Z', expect.any(Number)], + ['2019-07-16T11:14:25.589Z', expect.any(Number)], + ['2019-07-16T12:14:25.589Z', expect.any(Number)], ]); - expect(timeSeriesChart.vm.scatterSeries.symbolSize).toBe(14); + expect(timeSeriesChart.vm.deploymentSeries.symbolSize).toBe(14); }); }); describe('yAxisLabel', () => { + it('y axis is configured correctly', () => { + const { yAxis } = getChartOptions(); + + expect(yAxis).toHaveLength(2); + + const [dataAxis, deploymentAxis] = yAxis; + + expect(dataAxis.boundaryGap).toHaveLength(2); + expect(dataAxis.scale).toBe(true); + + expect(deploymentAxis.show).toBe(false); + expect(deploymentAxis.min).toEqual(expect.any(Number)); + expect(deploymentAxis.max).toEqual(expect.any(Number)); + expect(deploymentAxis.min).toBeLessThan(deploymentAxis.max); + }); + it('constructs a label for the chart y-axis', () => { - expect(timeSeriesChart.vm.yAxisLabel).toBe('Memory Used per Pod'); + const { yAxis } = getChartOptions(); + + expect(yAxis[0].name).toBe('Memory Used per Pod'); }); }); }); @@ -405,7 +432,7 @@ describe('Time series component', () => { glChartComponents.forEach(dynamicComponent => { describe(`GitLab UI: ${dynamicComponent.chartType}`, () => { let timeSeriesAreaChart; - const findChart = () => timeSeriesAreaChart.find(dynamicComponent.component); + const findChartComponent = () => timeSeriesAreaChart.find(dynamicComponent.component); beforeEach(done => { timeSeriesAreaChart = makeTimeSeriesChart(mockGraphData, dynamicComponent.chartType); @@ -417,12 +444,12 @@ describe('Time series component', () => { }); it('is a Vue instance', () => { - expect(findChart().exists()).toBe(true); - expect(findChart().isVueInstance()).toBe(true); + expect(findChartComponent().exists()).toBe(true); + expect(findChartComponent().isVueInstance()).toBe(true); }); it('receives data properties needed for proper chart render', () => { - const props = findChart().props(); + const props = findChartComponent().props(); expect(props.data).toBe(timeSeriesAreaChart.vm.chartData); expect(props.option).toBe(timeSeriesAreaChart.vm.chartOptions); @@ -435,9 +462,9 @@ describe('Time series component', () => { timeSeriesAreaChart.vm.tooltip.title = mockTitle; timeSeriesAreaChart.vm.$nextTick(() => { - expect(shallowWrapperContainsSlotText(findChart(), 'tooltipTitle', mockTitle)).toBe( - true, - ); + expect( + shallowWrapperContainsSlotText(findChartComponent(), 'tooltipTitle', mockTitle), + ).toBe(true); done(); }); }); @@ -452,9 +479,9 @@ describe('Time series component', () => { }); it('uses deployment title', () => { - expect(shallowWrapperContainsSlotText(findChart(), 'tooltipTitle', 'Deployed')).toBe( - true, - ); + expect( + shallowWrapperContainsSlotText(findChartComponent(), 'tooltipTitle', 'Deployed'), + ).toBe(true); }); it('renders clickable commit sha in tooltip content', done => { diff --git a/spec/frontend/snippet/collapsible_input_spec.js b/spec/frontend/snippet/collapsible_input_spec.js new file mode 100644 index 00000000000..acd15164c95 --- /dev/null +++ b/spec/frontend/snippet/collapsible_input_spec.js @@ -0,0 +1,104 @@ +import setupCollapsibleInputs from '~/snippet/collapsible_input'; +import { setHTMLFixture } from 'helpers/fixtures'; + +describe('~/snippet/collapsible_input', () => { + let formEl; + let descriptionEl; + let titleEl; + let fooEl; + + beforeEach(() => { + setHTMLFixture(` + <form> + <div class="js-collapsible-input js-title"> + <div class="js-collapsed d-none"> + <input type="text" /> + </div> + <div class="js-expanded"> + <textarea>Hello World!</textarea> + </div> + </div> + <div class="js-collapsible-input js-description"> + <div class="js-collapsed"> + <input type="text" /> + </div> + <div class="js-expanded d-none"> + <textarea></textarea> + </div> + </div> + <input type="text" class="js-foo" /> + </form> + `); + + formEl = document.querySelector('form'); + titleEl = formEl.querySelector('.js-title'); + descriptionEl = formEl.querySelector('.js-description'); + fooEl = formEl.querySelector('.js-foo'); + + setupCollapsibleInputs(); + }); + + const findInput = el => el.querySelector('textarea,input'); + const findCollapsed = el => el.querySelector('.js-collapsed'); + const findExpanded = el => el.querySelector('.js-expanded'); + const findCollapsedInput = el => findInput(findCollapsed(el)); + const findExpandedInput = el => findInput(findExpanded(el)); + const focusIn = target => target.dispatchEvent(new Event('focusin', { bubbles: true })); + const expectIsCollapsed = (el, isCollapsed) => { + expect(findCollapsed(el).classList.contains('d-none')).toEqual(!isCollapsed); + expect(findExpanded(el).classList.contains('d-none')).toEqual(isCollapsed); + }; + + describe('when collapsed', () => { + it('is collapsed', () => { + expectIsCollapsed(descriptionEl, true); + }); + + describe('when focused', () => { + beforeEach(() => { + focusIn(findCollapsedInput(descriptionEl)); + }); + + it('is expanded', () => { + expectIsCollapsed(descriptionEl, false); + }); + + describe.each` + desc | value | isCollapsed + ${'is collapsed'} | ${''} | ${true} + ${'stays open if given value'} | ${'Hello world!'} | ${false} + `('when loses focus', ({ desc, value, isCollapsed }) => { + it(desc, () => { + findExpandedInput(descriptionEl).value = value; + focusIn(fooEl); + + expectIsCollapsed(descriptionEl, isCollapsed); + }); + }); + }); + }); + + describe('when expanded and has value', () => { + it('does not collapse, when focusing out', () => { + expectIsCollapsed(titleEl, false); + + focusIn(fooEl); + + expectIsCollapsed(titleEl, false); + }); + + describe('and loses value', () => { + beforeEach(() => { + findExpandedInput(titleEl).value = ''; + }); + + it('collapses, when focusing out', () => { + expectIsCollapsed(titleEl, false); + + focusIn(fooEl); + + expectIsCollapsed(titleEl, true); + }); + }); + }); +}); diff --git a/spec/javascripts/diffs/components/diff_table_cell_spec.js b/spec/javascripts/diffs/components/diff_table_cell_spec.js deleted file mode 100644 index f91e3b56805..00000000000 --- a/spec/javascripts/diffs/components/diff_table_cell_spec.js +++ /dev/null @@ -1,37 +0,0 @@ -import Vue from 'vue'; -import { createComponentWithStore } from 'spec/helpers/vue_mount_component_helper'; -import { createStore } from '~/mr_notes/stores'; -import DiffTableCell from '~/diffs/components/diff_table_cell.vue'; -import diffFileMockData from '../mock_data/diff_file'; - -describe('DiffTableCell', () => { - const createComponent = options => - createComponentWithStore(Vue.extend(DiffTableCell), createStore(), { - line: diffFileMockData.highlighted_diff_lines[0], - fileHash: diffFileMockData.file_hash, - contextLinesPath: 'contextLinesPath', - ...options, - }).$mount(); - - it('does not highlight row when isHighlighted prop is false', done => { - const vm = createComponent({ isHighlighted: false }); - - vm.$nextTick() - .then(() => { - expect(vm.$el.classList).not.toContain('hll'); - }) - .then(done) - .catch(done.fail); - }); - - it('highlights row when isHighlighted prop is true', done => { - const vm = createComponent({ isHighlighted: true }); - - vm.$nextTick() - .then(() => { - expect(vm.$el.classList).toContain('hll'); - }) - .then(done) - .catch(done.fail); - }); -}); diff --git a/spec/lib/gitlab/git_access_snippet_spec.rb b/spec/lib/gitlab/git_access_snippet_spec.rb new file mode 100644 index 00000000000..ffb3d86408a --- /dev/null +++ b/spec/lib/gitlab/git_access_snippet_spec.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Gitlab::GitAccessSnippet do + include GitHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:personal_snippet) { create(:personal_snippet, :private, :repository) } + + let(:protocol) { 'ssh' } + let(:changes) { Gitlab::GitAccess::ANY } + let(:push_access_check) { access.check('git-receive-pack', changes) } + let(:pull_access_check) { access.check('git-upload-pack', changes) } + let(:snippet) { personal_snippet } + let(:actor) { personal_snippet.author } + + describe 'when feature flag :version_snippets is enabled' do + it 'allows push and pull access' do + aggregate_failures do + expect { pull_access_check }.not_to raise_error + expect { push_access_check }.not_to raise_error + end + end + end + + describe 'when feature flag :version_snippets is disabled' do + before do + stub_feature_flags(version_snippets: false) + end + + it 'does not allow push and pull access' do + aggregate_failures do + expect { push_access_check }.to raise_snippet_not_found + expect { pull_access_check }.to raise_snippet_not_found + end + end + end + + describe '#check_snippet_accessibility!' do + context 'when the snippet exists' do + it 'allows push and pull access' do + aggregate_failures do + expect { pull_access_check }.not_to raise_error + expect { push_access_check }.not_to raise_error + end + end + end + + context 'when the snippet is nil' do + let(:snippet) { nil } + + it 'blocks push and pull with "not found"' do + aggregate_failures do + expect { pull_access_check }.to raise_snippet_not_found + expect { push_access_check }.to raise_snippet_not_found + end + end + end + + context 'when the snippet does not have a repository' do + let(:snippet) { build_stubbed(:personal_snippet) } + + it 'blocks push and pull with "not found"' do + aggregate_failures do + expect { pull_access_check }.to raise_snippet_not_found + expect { push_access_check }.to raise_snippet_not_found + end + end + end + end + + private + + def access + described_class.new(actor, snippet, protocol, + authentication_abilities: [], + namespace_path: nil, project_path: nil, + redirected_path: nil, auth_result_type: nil) + end + + def raise_snippet_not_found + raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:snippet_not_found]) + end +end diff --git a/spec/lib/gitlab/gl_repository/repo_type_spec.rb b/spec/lib/gitlab/gl_repository/repo_type_spec.rb index 8bbcc644f6d..7cf0442fbe1 100644 --- a/spec/lib/gitlab/gl_repository/repo_type_spec.rb +++ b/spec/lib/gitlab/gl_repository/repo_type_spec.rb @@ -3,6 +3,8 @@ require 'spec_helper' describe Gitlab::GlRepository::RepoType do let_it_be(:project) { create(:project) } + let_it_be(:personal_snippet) { create(:personal_snippet, author: project.owner) } + let_it_be(:project_snippet) { create(:project_snippet, project: project, author: project.owner) } describe Gitlab::GlRepository::PROJECT do it_behaves_like 'a repo type' do @@ -16,6 +18,7 @@ describe Gitlab::GlRepository::RepoType do it 'knows its type' do expect(described_class).not_to be_wiki expect(described_class).to be_project + expect(described_class).not_to be_snippet end it 'checks if repository path is valid' do @@ -36,6 +39,7 @@ describe Gitlab::GlRepository::RepoType do it 'knows its type' do expect(described_class).to be_wiki expect(described_class).not_to be_project + expect(described_class).not_to be_snippet end it 'checks if repository path is valid' do @@ -43,4 +47,38 @@ describe Gitlab::GlRepository::RepoType do expect(described_class.valid?(project.wiki.repository.full_path)).to be_truthy end end + + describe Gitlab::GlRepository::SNIPPET do + context 'when PersonalSnippet' do + it_behaves_like 'a repo type' do + let(:expected_id) { personal_snippet.id.to_s } + let(:expected_identifier) { "snippet-#{expected_id}" } + let(:expected_suffix) { '' } + let(:expected_repository) { personal_snippet.repository } + let(:expected_container) { personal_snippet } + end + + it 'knows its type' do + expect(described_class).to be_snippet + expect(described_class).not_to be_wiki + expect(described_class).not_to be_project + end + end + + context 'when ProjectSnippet' do + it_behaves_like 'a repo type' do + let(:expected_id) { project_snippet.id.to_s } + let(:expected_identifier) { "snippet-#{expected_id}" } + let(:expected_suffix) { '' } + let(:expected_repository) { project_snippet.repository } + let(:expected_container) { project_snippet } + end + + it 'knows its type' do + expect(described_class).to be_snippet + expect(described_class).not_to be_wiki + expect(described_class).not_to be_project + end + end + end end diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 4c521ae7f07..e6a60f39bd4 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -91,6 +91,7 @@ snippets: - award_emoji - user_agent_detail - user_mentions +- snippet_repository releases: - author - project diff --git a/spec/lib/gitlab/import_export/hash_util_spec.rb b/spec/lib/gitlab/import_export/hash_util_spec.rb index ddd874ddecf..b97c6665d0e 100644 --- a/spec/lib/gitlab/import_export/hash_util_spec.rb +++ b/spec/lib/gitlab/import_export/hash_util_spec.rb @@ -9,7 +9,7 @@ describe Gitlab::ImportExport::HashUtil do describe '.deep_symbolize_array!' do it 'symbolizes keys' do expect { described_class.deep_symbolize_array!(stringified_array) }.to change { - stringified_array.first.keys.first + stringified_array.first.each_key.first }.from('test').to(:test) end end @@ -17,13 +17,13 @@ describe Gitlab::ImportExport::HashUtil do describe '.deep_symbolize_array_with_date!' do it 'symbolizes keys' do expect { described_class.deep_symbolize_array_with_date!(stringified_array_with_date) }.to change { - stringified_array_with_date.first.keys.first + stringified_array_with_date.first.each_key.first }.from('test_date').to(:test_date) end it 'transforms date strings into Time objects' do expect { described_class.deep_symbolize_array_with_date!(stringified_array_with_date) }.to change { - stringified_array_with_date.first.values.first.class + stringified_array_with_date.first.each_value.first.class }.from(String).to(ActiveSupport::TimeWithZone) end end diff --git a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb index 697bedf7362..2d3b61e61ce 100644 --- a/spec/lib/gitlab/metrics/dashboard/finder_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/finder_spec.rb @@ -44,6 +44,12 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi it_behaves_like 'valid dashboard service response' end + context 'when the self monitoring dashboard is specified' do + let(:dashboard_path) { self_monitoring_dashboard_path } + + it_behaves_like 'valid dashboard service response' + end + context 'when no dashboard is specified' do let(:service_call) { described_class.find(project, user, environment: environment) } @@ -152,5 +158,33 @@ describe Gitlab::Metrics::Dashboard::Finder, :use_clean_rails_memory_store_cachi expect(all_dashboard_paths).to contain_exactly(system_dashboard, project_dashboard) end end + + context 'when the project is self monitoring' do + let(:self_monitoring_dashboard) do + { + path: self_monitoring_dashboard_path, + display_name: 'Default', + default: true, + system_dashboard: false + } + end + let(:dashboard_path) { '.gitlab/dashboards/test.yml' } + let(:project) { project_with_dashboard(dashboard_path) } + + before do + stub_application_setting(self_monitoring_project_id: project.id) + end + + it 'includes self monitoring and project dashboards' do + project_dashboard = { + path: dashboard_path, + display_name: 'test.yml', + default: false, + system_dashboard: false + } + + expect(all_dashboard_paths).to contain_exactly(self_monitoring_dashboard, project_dashboard) + end + end end end diff --git a/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb b/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb index e0c8133994b..c0d71bfe5d0 100644 --- a/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb +++ b/spec/lib/gitlab/metrics/dashboard/service_selector_spec.rb @@ -30,6 +30,12 @@ describe Gitlab::Metrics::Dashboard::ServiceSelector do end end + context 'when the path is for the self monitoring dashboard' do + let(:arguments) { { dashboard_path: self_monitoring_dashboard_path } } + + it { is_expected.to be Metrics::Dashboard::SelfMonitoringDashboardService } + end + context 'when the embedded flag is provided' do let(:arguments) { { embedded: true } } diff --git a/spec/lib/gitlab/shell_spec.rb b/spec/lib/gitlab/shell_spec.rb index eefc548a4d9..7b8d1b6cd9b 100644 --- a/spec/lib/gitlab/shell_spec.rb +++ b/spec/lib/gitlab/shell_spec.rb @@ -397,7 +397,7 @@ describe Gitlab::Shell do describe 'namespace actions' do subject { described_class.new } - let(:storage) { Gitlab.config.repositories.storages.keys.first } + let(:storage) { Gitlab.config.repositories.storages.each_key.first } describe '#add_namespace' do it 'creates a namespace' do diff --git a/spec/models/blob_spec.rb b/spec/models/blob_spec.rb index 7099d000d4c..a0193b29bb3 100644 --- a/spec/models/blob_spec.rb +++ b/spec/models/blob_spec.rb @@ -6,6 +6,8 @@ describe Blob do include FakeBlobHelpers let(:project) { build(:project, lfs_enabled: true) } + let(:personal_snippet) { build(:personal_snippet) } + let(:project_snippet) { build(:project_snippet, project: project) } before do allow(Gitlab.config.lfs).to receive(:enabled).and_return(true) @@ -18,77 +20,146 @@ describe Blob do end describe '.lazy' do - let(:project) { create(:project, :repository) } - let(:same_project) { Project.find(project.id) } - let(:other_project) { create(:project, :repository) } let(:commit_id) { 'e63f41fe459e62e1228fcef60d7189127aeba95a' } let(:blob_size_limit) { 10 * 1024 * 1024 } - it 'does not fetch blobs when none are accessed' do - expect(project.repository).not_to receive(:blobs_at) + shared_examples '.lazy checks' do + it 'does not fetch blobs when none are accessed' do + expect(container.repository).not_to receive(:blobs_at) - described_class.lazy(project, commit_id, 'CHANGELOG') - end + described_class.lazy(container, commit_id, 'CHANGELOG') + end + + it 'fetches all blobs for the same repository when one is accessed' do + expect(container.repository).to receive(:blobs_at) + .with([[commit_id, 'CHANGELOG'], [commit_id, 'CONTRIBUTING.md']], blob_size_limit: blob_size_limit) + .once.and_call_original + expect(other_container.repository).not_to receive(:blobs_at) + + changelog = described_class.lazy(container, commit_id, 'CHANGELOG') + contributing = described_class.lazy(same_container, commit_id, 'CONTRIBUTING.md') + + described_class.lazy(other_container, commit_id, 'CHANGELOG') + + # Access property so the values are loaded + changelog.id + contributing.id + end + + it 'does not include blobs from previous requests in later requests' do + changelog = described_class.lazy(container, commit_id, 'CHANGELOG') + contributing = described_class.lazy(same_container, commit_id, 'CONTRIBUTING.md') - it 'fetches all blobs for the same repository when one is accessed' do - expect(project.repository).to receive(:blobs_at) - .with([[commit_id, 'CHANGELOG'], [commit_id, 'CONTRIBUTING.md']], blob_size_limit: blob_size_limit) - .once.and_call_original - expect(other_project.repository).not_to receive(:blobs_at) + # Access property so the values are loaded + changelog.id + contributing.id - changelog = described_class.lazy(project, commit_id, 'CHANGELOG') - contributing = described_class.lazy(same_project, commit_id, 'CONTRIBUTING.md') + readme = described_class.lazy(container, commit_id, 'README.md') - described_class.lazy(other_project, commit_id, 'CHANGELOG') + expect(container.repository).to receive(:blobs_at) + .with([[commit_id, 'README.md']], blob_size_limit: blob_size_limit).once.and_call_original - # Access property so the values are loaded - changelog.id - contributing.id + readme.id + end end - it 'does not include blobs from previous requests in later requests' do - changelog = described_class.lazy(project, commit_id, 'CHANGELOG') - contributing = described_class.lazy(same_project, commit_id, 'CONTRIBUTING.md') + context 'with project' do + let(:container) { create(:project, :repository) } + let(:same_container) { Project.find(container.id) } + let(:other_container) { create(:project, :repository) } - # Access property so the values are loaded - changelog.id - contributing.id + it_behaves_like '.lazy checks' + end + + context 'with personal snippet' do + let(:container) { create(:personal_snippet, :repository) } + let(:same_container) { PersonalSnippet.find(container.id) } + let(:other_container) { create(:personal_snippet, :repository) } - readme = described_class.lazy(project, commit_id, 'README.md') + it_behaves_like '.lazy checks' + end - expect(project.repository).to receive(:blobs_at) - .with([[commit_id, 'README.md']], blob_size_limit: blob_size_limit).once.and_call_original + context 'with project snippet' do + let(:container) { create(:project_snippet, :repository) } + let(:same_container) { ProjectSnippet.find(container.id) } + let(:other_container) { create(:project_snippet, :repository) } - readme.id + it_behaves_like '.lazy checks' end end describe '#data' do - context 'using a binary blob' do - it 'returns the data as-is' do - data = "\n\xFF\xB9\xC3" - blob = fake_blob(binary: true, data: data) + shared_examples '#data checks' do + context 'using a binary blob' do + it 'returns the data as-is' do + data = "\n\xFF\xB9\xC3" + blob = fake_blob(binary: true, data: data, container: container) - expect(blob.data).to eq(data) + expect(blob.data).to eq(data) + end end - end - context 'using a text blob' do - it 'converts the data to UTF-8' do - blob = fake_blob(binary: false, data: "\n\xFF\xB9\xC3") + context 'using a text blob' do + it 'converts the data to UTF-8' do + blob = fake_blob(binary: false, data: "\n\xFF\xB9\xC3", container: container) - expect(blob.data).to eq("\n���") + expect(blob.data).to eq("\n���") + end end end + + context 'with project' do + let(:container) { project } + + it_behaves_like '#data checks' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like '#data checks' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like '#data checks' + end end describe '#external_storage_error?' do + shared_examples 'no error' do + it do + expect(blob.external_storage_error?).to be_falsey + end + end + + shared_examples 'returns error' do + it do + expect(blob.external_storage_error?).to be_truthy + end + end + context 'if the blob is stored in LFS' do - let(:blob) { fake_blob(path: 'file.pdf', lfs: true) } + let(:blob) { fake_blob(path: 'file.pdf', lfs: true, container: container) } context 'when the project has LFS enabled' do - it 'returns false' do - expect(blob.external_storage_error?).to be_falsey + context 'with project' do + let(:container) { project } + + it_behaves_like 'no error' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns error' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'no error' end end @@ -97,17 +168,39 @@ describe Blob do project.lfs_enabled = false end - it 'returns true' do - expect(blob.external_storage_error?).to be_truthy + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns error' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns error' end end end context 'if the blob is not stored in LFS' do - let(:blob) { fake_blob(path: 'file.md') } + let(:blob) { fake_blob(path: 'file.md', container: container) } - it 'returns false' do - expect(blob.external_storage_error?).to be_falsey + context 'with project' do + let(:container) { project } + + it_behaves_like 'no error' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'no error' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'no error' end end end @@ -116,19 +209,59 @@ describe Blob do context 'if the blob is stored in LFS' do let(:blob) { fake_blob(path: 'file.pdf', lfs: true) } - context 'when the project has LFS enabled' do - it 'returns true' do + shared_examples 'returns true' do + it do expect(blob.stored_externally?).to be_truthy end end + shared_examples 'returns false' do + it do + expect(blob.stored_externally?).to be_falsey + end + end + + context 'when the project has LFS enabled' do + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns true' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns true' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns true' + end + end + context 'when the project does not have LFS enabled' do before do project.lfs_enabled = false end - it 'returns false' do - expect(blob.stored_externally?).to be_falsey + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns false' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns false' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns false' end end end @@ -143,21 +276,63 @@ describe Blob do end describe '#binary?' do + shared_examples 'returns true' do + it do + expect(blob.binary?).to be_truthy + end + end + + shared_examples 'returns false' do + it do + expect(blob.binary?).to be_falsey + end + end + context 'if the blob is stored externally' do + let(:blob) { fake_blob(path: file, lfs: true) } + context 'if the extension has a rich viewer' do context 'if the viewer is binary' do - it 'returns true' do - blob = fake_blob(path: 'file.pdf', lfs: true) + let(:file) { 'file.pdf' } - expect(blob.binary?).to be_truthy + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns true' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns true' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns true' end end context 'if the viewer is text-based' do - it 'return false' do - blob = fake_blob(path: 'file.md', lfs: true) + let(:file) { 'file.md' } + + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns false' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } - expect(blob.binary?).to be_falsey + it_behaves_like 'returns false' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns false' end end end @@ -165,54 +340,138 @@ describe Blob do context "if the extension doesn't have a rich viewer" do context 'if the extension has a text mime type' do context 'if the extension is for a programming language' do - it 'returns false' do - blob = fake_blob(path: 'file.txt', lfs: true) + let(:file) { 'file.txt' } + + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns false' + end - expect(blob.binary?).to be_falsey + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns false' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns false' end end context 'if the extension is not for a programming language' do - it 'returns false' do - blob = fake_blob(path: 'file.ics', lfs: true) + let(:file) { 'file.ics' } + + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns false' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns false' + end + + context 'with project snippet' do + let(:container) { project_snippet } - expect(blob.binary?).to be_falsey + it_behaves_like 'returns false' end end end context 'if the extension has a binary mime type' do context 'if the extension is for a programming language' do - it 'returns false' do - blob = fake_blob(path: 'file.rb', lfs: true) + let(:file) { 'file.rb' } + + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns false' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns false' + end + + context 'with project snippet' do + let(:container) { project_snippet } - expect(blob.binary?).to be_falsey + it_behaves_like 'returns false' end end context 'if the extension is not for a programming language' do - it 'returns true' do - blob = fake_blob(path: 'file.exe', lfs: true) + let(:file) { 'file.exe' } - expect(blob.binary?).to be_truthy + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns true' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns true' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns true' end end end context 'if the extension has an unknown mime type' do context 'if the extension is for a programming language' do - it 'returns false' do - blob = fake_blob(path: 'file.ini', lfs: true) + let(:file) { 'file.ini' } + + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns false' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns false' + end + + context 'with project snippet' do + let(:container) { project_snippet } - expect(blob.binary?).to be_falsey + it_behaves_like 'returns false' end end context 'if the extension is not for a programming language' do - it 'returns true' do - blob = fake_blob(path: 'file.wtf', lfs: true) + let(:file) { 'file.wtf' } + + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns true' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } - expect(blob.binary?).to be_truthy + it_behaves_like 'returns true' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns true' end end end @@ -221,18 +480,46 @@ describe Blob do context 'if the blob is not stored externally' do context 'if the blob is binary' do - it 'returns true' do - blob = fake_blob(path: 'file.pdf', binary: true) + let(:blob) { fake_blob(path: 'file.pdf', binary: true, container: container) } - expect(blob.binary?).to be_truthy + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns true' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns true' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns true' end end context 'if the blob is text-based' do - it 'return false' do - blob = fake_blob(path: 'file.md') + let(:blob) { fake_blob(path: 'file.md', container: container) } + + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns false' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns false' + end + + context 'with project snippet' do + let(:container) { project_snippet } - expect(blob.binary?).to be_falsey + it_behaves_like 'returns false' end end end @@ -389,38 +676,110 @@ describe Blob do end describe '#rendered_as_text?' do + shared_examples 'returns true' do + it do + expect(blob.rendered_as_text?(ignore_errors: ignore_errors)).to be_truthy + end + end + + shared_examples 'returns false' do + it do + expect(blob.rendered_as_text?(ignore_errors: ignore_errors)).to be_falsey + end + end + context 'when ignoring errors' do + let(:ignore_errors) { true } + context 'when the simple viewer is text-based' do - it 'returns true' do - blob = fake_blob(path: 'file.md', size: 100.megabytes) + let(:blob) { fake_blob(path: 'file.md', size: 100.megabytes, container: container) } - expect(blob.rendered_as_text?).to be_truthy + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns true' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns true' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns true' end end context 'when the simple viewer is binary' do - it 'returns false' do - blob = fake_blob(path: 'file.pdf', binary: true, size: 100.megabytes) + let(:blob) { fake_blob(path: 'file.pdf', binary: true, size: 100.megabytes, container: container) } + + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns false' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns false' + end + + context 'with project snippet' do + let(:container) { project_snippet } - expect(blob.rendered_as_text?).to be_falsey + it_behaves_like 'returns false' end end end context 'when not ignoring errors' do + let(:ignore_errors) { false } + context 'when the viewer has render errors' do - it 'returns false' do - blob = fake_blob(path: 'file.md', size: 100.megabytes) + let(:blob) { fake_blob(path: 'file.md', size: 100.megabytes, container: container) } - expect(blob.rendered_as_text?(ignore_errors: false)).to be_falsey + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns false' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns false' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns false' end end context "when the viewer doesn't have render errors" do - it 'returns true' do - blob = fake_blob(path: 'file.md') + let(:blob) { fake_blob(path: 'file.md', container: container) } + + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns true' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns true' + end + + context 'with project snippet' do + let(:container) { project_snippet } - expect(blob.rendered_as_text?(ignore_errors: false)).to be_truthy + it_behaves_like 'returns true' end end end diff --git a/spec/models/commit_spec.rb b/spec/models/commit_spec.rb index ada25005064..26cc68eb58c 100644 --- a/spec/models/commit_spec.rb +++ b/spec/models/commit_spec.rb @@ -3,8 +3,10 @@ require 'spec_helper' describe Commit do - let(:project) { create(:project, :public, :repository) } - let(:commit) { project.commit } + let_it_be(:project) { create(:project, :public, :repository) } + let_it_be(:personal_snippet) { create(:personal_snippet, :repository) } + let_it_be(:project_snippet) { create(:project_snippet, :repository) } + let(:commit) { project.commit } describe 'modules' do subject { described_class } @@ -17,49 +19,67 @@ describe Commit do end describe '.lazy' do - let_it_be(:project) { create(:project, :repository) } + shared_examples '.lazy checks' do + context 'when the commits are found' do + let(:oids) do + %w( + 498214de67004b1da3d820901307bed2a68a8ef6 + c642fe9b8b9f28f9225d7ea953fe14e74748d53b + 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 + 048721d90c449b244b7b4c53a9186b04330174ec + 281d3a76f31c812dbf48abce82ccf6860adedd81 + ) + end - context 'when the commits are found' do - let(:oids) do - %w( - 498214de67004b1da3d820901307bed2a68a8ef6 - c642fe9b8b9f28f9225d7ea953fe14e74748d53b - 6f6d7e7ed97bb5f0054f2b1df789b39ca89b6ff9 - 048721d90c449b244b7b4c53a9186b04330174ec - 281d3a76f31c812dbf48abce82ccf6860adedd81 - ) - end + subject { oids.map { |oid| described_class.lazy(container, oid) } } - subject { oids.map { |oid| described_class.lazy(project, oid) } } + it 'batches requests for commits' do + expect(container.repository).to receive(:commits_by).once.and_call_original - it 'batches requests for commits' do - expect(project.repository).to receive(:commits_by).once.and_call_original + subject.first.title + subject.last.title + end - subject.first.title - subject.last.title - end + it 'maintains ordering' do + subject.each_with_index do |commit, i| + expect(commit.id).to eq(oids[i]) + end + end - it 'maintains ordering' do - subject.each_with_index do |commit, i| - expect(commit.id).to eq(oids[i]) + it 'does not attempt to replace methods via BatchLoader' do + subject.each do |commit| + expect(commit).to receive(:method_missing).and_call_original + + commit.id + end end end - it 'does not attempt to replace methods via BatchLoader' do - subject.each do |commit| - expect(commit).to receive(:method_missing).and_call_original + context 'when not found' do + it 'returns nil as commit' do + commit = described_class.lazy(container, 'deadbeef').__sync - commit.id + expect(commit).to be_nil end end end - context 'when not found' do - it 'returns nil as commit' do - commit = described_class.lazy(project, 'deadbeef').__sync + context 'with project' do + let(:container) { project } - expect(commit).to be_nil - end + it_behaves_like '.lazy checks' + end + + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like '.lazy checks' + end + + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like '.lazy checks' end end @@ -231,15 +251,43 @@ describe Commit do end describe '#to_reference' do - let(:project) { create(:project, :repository, path: 'sample-project') } + context 'with project' do + let(:project) { create(:project, :repository, path: 'sample-project') } + + it 'returns a String reference to the object' do + expect(commit.to_reference).to eq commit.id + end - it 'returns a String reference to the object' do - expect(commit.to_reference).to eq commit.id + it 'supports a cross-project reference' do + another_project = build(:project, :repository, name: 'another-project', namespace: project.namespace) + expect(commit.to_reference(another_project)).to eq "sample-project@#{commit.id}" + end end - it 'supports a cross-project reference' do - another_project = build(:project, :repository, name: 'another-project', namespace: project.namespace) - expect(commit.to_reference(another_project)).to eq "sample-project@#{commit.id}" + context 'with personal snippet' do + let(:commit) { personal_snippet.commit } + + it 'returns a String reference to the object' do + expect(commit.to_reference).to eq "$#{personal_snippet.id}@#{commit.id}" + end + + it 'supports a cross-snippet reference' do + another_snippet = build(:personal_snippet) + expect(commit.to_reference(another_snippet)).to eq "$#{personal_snippet.id}@#{commit.id}" + end + end + + context 'with project snippet' do + let(:commit) { project_snippet.commit } + + it 'returns a String reference to the object' do + expect(commit.to_reference).to eq "$#{project_snippet.id}@#{commit.id}" + end + + it 'supports a cross-snippet project reference' do + another_snippet = build(:personal_snippet) + expect(commit.to_reference(another_snippet)).to eq "#{project_snippet.project.path}$#{project_snippet.id}@#{commit.id}" + end end end @@ -264,13 +312,41 @@ describe Commit do describe '#reference_link_text' do let(:project) { create(:project, :repository, path: 'sample-project') } - it 'returns a String reference to the object' do - expect(commit.reference_link_text).to eq commit.short_id + context 'with project' do + it 'returns a String reference to the object' do + expect(commit.reference_link_text).to eq commit.short_id + end + + it 'supports a cross-project reference' do + another_project = build(:project, :repository, name: 'another-project', namespace: project.namespace) + expect(commit.reference_link_text(another_project)).to eq "sample-project@#{commit.short_id}" + end + end + + context 'with personal snippet' do + let(:commit) { personal_snippet.commit } + + it 'returns a String reference to the object' do + expect(commit.reference_link_text).to eq "$#{personal_snippet.id}@#{commit.short_id}" + end + + it 'supports a cross-snippet reference' do + another_snippet = build(:personal_snippet, :repository) + expect(commit.reference_link_text(another_snippet)).to eq "$#{personal_snippet.id}@#{commit.short_id}" + end end - it 'supports a cross-project reference' do - another_project = build(:project, :repository, name: 'another-project', namespace: project.namespace) - expect(commit.reference_link_text(another_project)).to eq "sample-project@#{commit.short_id}" + context 'with project snippet' do + let(:commit) { project_snippet.commit } + + it 'returns a String reference to the object' do + expect(commit.reference_link_text).to eq "$#{project_snippet.id}@#{commit.short_id}" + end + + it 'supports a cross-snippet project reference' do + another_snippet = build(:project_snippet, :repository) + expect(commit.reference_link_text(another_snippet)).to eq "#{project_snippet.project.path}$#{project_snippet.id}@#{commit.short_id}" + end end end @@ -401,6 +477,26 @@ eos expect(commit.closes_issues).to be_empty end + + context 'with personal snippet' do + let(:commit) { personal_snippet.commit } + + it 'does not call Gitlab::ClosingIssueExtractor' do + expect(Gitlab::ClosingIssueExtractor).not_to receive(:new) + + commit.closes_issues + end + end + + context 'with project snippet' do + let(:commit) { project_snippet.commit } + + it 'does not call Gitlab::ClosingIssueExtractor' do + expect(Gitlab::ClosingIssueExtractor).not_to receive(:new) + + commit.closes_issues + end + end end it_behaves_like 'a mentionable' do @@ -597,19 +693,39 @@ eos end describe '.from_hash' do - let(:new_commit) { described_class.from_hash(commit.to_hash, project) } + subject { described_class.from_hash(commit.to_hash, container) } - it 'returns a Commit' do - expect(new_commit).to be_an_instance_of(described_class) + shared_examples 'returns Commit' do + it 'returns a Commit' do + expect(subject).to be_an_instance_of(described_class) + end + + it 'wraps a Gitlab::Git::Commit' do + expect(subject.raw).to be_an_instance_of(Gitlab::Git::Commit) + end + + it 'stores the correct commit fields' do + expect(subject.id).to eq(commit.id) + expect(subject.message).to eq(commit.message) + end + end + + context 'with project' do + let(:container) { project } + + it_behaves_like 'returns Commit' end - it 'wraps a Gitlab::Git::Commit' do - expect(new_commit.raw).to be_an_instance_of(Gitlab::Git::Commit) + context 'with personal snippet' do + let(:container) { personal_snippet } + + it_behaves_like 'returns Commit' end - it 'stores the correct commit fields' do - expect(new_commit.id).to eq(commit.id) - expect(new_commit.message).to eq(commit.message) + context 'with project snippet' do + let(:container) { project_snippet } + + it_behaves_like 'returns Commit' end end @@ -670,6 +786,19 @@ eos expect(commit1.merge_requests).to contain_exactly(merge_request1, merge_request2) expect(commit2.merge_requests).to contain_exactly(merge_request1) end + + context 'with personal snippet' do + it 'returns empty relation' do + expect(personal_snippet.repository.commit.merge_requests).to eq MergeRequest.none + end + end + + context 'with project snippet' do + it 'returns empty relation' do + expect(project_snippet.project).not_to receive(:merge_requests) + expect(project_snippet.repository.commit.merge_requests).to eq MergeRequest.none + end + end end describe 'signed commits' do diff --git a/spec/models/concerns/redis_cacheable_spec.rb b/spec/models/concerns/redis_cacheable_spec.rb index a9dca27f258..f88d64e2013 100644 --- a/spec/models/concerns/redis_cacheable_spec.rb +++ b/spec/models/concerns/redis_cacheable_spec.rb @@ -28,7 +28,7 @@ describe RedisCacheable do end describe '#cached_attribute' do - subject { instance.cached_attribute(payload.keys.first) } + subject { instance.cached_attribute(payload.each_key.first) } it 'gets the cache attribute' do Gitlab::Redis::SharedState.with do |redis| @@ -36,7 +36,7 @@ describe RedisCacheable do .and_return(payload.to_json) end - expect(subject).to eq(payload.values.first) + expect(subject).to eq(payload.each_value.first) end end diff --git a/spec/models/personal_snippet_spec.rb b/spec/models/personal_snippet_spec.rb index 276c8e22731..4a949a75cbd 100644 --- a/spec/models/personal_snippet_spec.rb +++ b/spec/models/personal_snippet_spec.rb @@ -16,4 +16,13 @@ describe PersonalSnippet do end end end + + it_behaves_like 'model with repository' do + let_it_be(:container) { create(:personal_snippet, :repository) } + let(:stubbed_container) { build_stubbed(:personal_snippet) } + let(:expected_full_path) { "@snippets/#{container.id}" } + let(:expected_repository_klass) { Repository } + let(:expected_storage_klass) { Storage::Hashed } + let(:expected_web_url_path) { "snippets/#{container.id}" } + end end diff --git a/spec/models/project_snippet_spec.rb b/spec/models/project_snippet_spec.rb index 903671afb13..09b4ec3677c 100644 --- a/spec/models/project_snippet_spec.rb +++ b/spec/models/project_snippet_spec.rb @@ -32,4 +32,13 @@ describe ProjectSnippet do end end end + + it_behaves_like 'model with repository' do + let_it_be(:container) { create(:project_snippet, :repository) } + let(:stubbed_container) { build_stubbed(:project_snippet) } + let(:expected_full_path) { "#{container.project.full_path}/@snippets/#{container.id}" } + let(:expected_repository_klass) { Repository } + let(:expected_storage_klass) { Storage::Hashed } + let(:expected_web_url_path) { "#{container.project.full_path}/snippets/#{container.id}" } + end end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 9dc362594dd..5c56d1aa757 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -113,6 +113,7 @@ describe Project do let(:expected_full_path) { "#{container.namespace.full_path}/somewhere" } let(:expected_repository_klass) { Repository } let(:expected_storage_klass) { Storage::Hashed } + let(:expected_web_url_path) { "#{container.namespace.full_path}/somewhere" } end it 'has an inverse relationship with merge requests' do @@ -5592,6 +5593,24 @@ describe Project do it { is_expected.to be_falsey } end + describe '#self_monitoring?' do + let_it_be(:project) { create(:project) } + + subject { project.self_monitoring? } + + context 'when the project is instance self monitoring' do + before do + stub_application_setting(self_monitoring_project_id: project.id) + end + + it { is_expected.to be true } + end + + context 'when the project is not self monitoring' do + it { is_expected.to be false } + end + end + def rugged_config rugged_repo(project.repository).config end diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb index 77114696fd2..0adf3fc8e85 100644 --- a/spec/models/repository_spec.rb +++ b/spec/models/repository_spec.rb @@ -372,7 +372,7 @@ describe Repository do context 'when some commits are not found ' do let(:oids) do - ['deadbeef'] + TestEnv::BRANCH_SHA.values.first(10) + ['deadbeef'] + TestEnv::BRANCH_SHA.each_value.first(10) end it 'returns only found commits' do diff --git a/spec/models/snippet_repository_spec.rb b/spec/models/snippet_repository_spec.rb new file mode 100644 index 00000000000..9befbb02b17 --- /dev/null +++ b/spec/models/snippet_repository_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe SnippetRepository do + describe 'associations' do + it { is_expected.to belong_to(:shard) } + it { is_expected.to belong_to(:snippet) } + end + + describe '.find_snippet' do + it 'finds snippet by disk path' do + snippet = create(:snippet) + snippet.track_snippet_repository + + expect(described_class.find_snippet(snippet.disk_path)).to eq(snippet) + end + + it 'returns nil when it does not find the snippet' do + expect(described_class.find_snippet('@@unexisting/path/to/snippet')).to be_nil + end + end +end diff --git a/spec/models/snippet_spec.rb b/spec/models/snippet_spec.rb index ae43c0d585a..93bc42c144d 100644 --- a/spec/models/snippet_spec.rb +++ b/spec/models/snippet_spec.rb @@ -19,6 +19,7 @@ describe Snippet do it { is_expected.to have_many(:notes).dependent(:destroy) } it { is_expected.to have_many(:award_emoji).dependent(:destroy) } it { is_expected.to have_many(:user_mentions).class_name("SnippetUserMention") } + it { is_expected.to have_one(:snippet_repository) } end describe 'validation' do @@ -525,4 +526,109 @@ describe Snippet do snippet.to_json(params) end end + + describe '#storage' do + let(:snippet) { create(:snippet) } + + it "stores snippet in #{Storage::Hashed::SNIPPET_REPOSITORY_PATH_PREFIX} dir" do + expect(snippet.storage.disk_path).to start_with Storage::Hashed::SNIPPET_REPOSITORY_PATH_PREFIX + end + end + + describe '#track_snippet_repository' do + let(:snippet) { create(:snippet, :repository) } + + context 'when a snippet repository entry does not exist' do + it 'creates a new entry' do + expect { snippet.track_snippet_repository }.to change(snippet, :snippet_repository) + end + + it 'tracks the snippet storage location' do + snippet.track_snippet_repository + + expect(snippet.snippet_repository).to have_attributes( + disk_path: snippet.disk_path, + shard_name: snippet.repository_storage + ) + end + end + + context 'when a tracking entry exists' do + let!(:snippet_repository) { create(:snippet_repository, snippet: snippet) } + let!(:shard) { create(:shard, name: 'foo') } + + it 'does not create a new entry in the database' do + expect { snippet.track_snippet_repository }.not_to change(snippet, :snippet_repository) + end + + it 'updates the snippet storage location' do + allow(snippet).to receive(:disk_path).and_return('fancy/new/path') + allow(snippet).to receive(:repository_storage).and_return('foo') + + snippet.track_snippet_repository + + expect(snippet.snippet_repository).to have_attributes( + disk_path: 'fancy/new/path', + shard_name: 'foo' + ) + end + end + end + + describe '#create_repository' do + let(:snippet) { create(:snippet) } + + it 'creates the repository' do + expect(snippet.repository).to receive(:after_create).and_call_original + + expect(snippet.create_repository).to be_truthy + expect(snippet.repository.exists?).to be_truthy + end + + it 'tracks snippet repository' do + expect do + snippet.create_repository + end.to change(SnippetRepository, :count).by(1) + end + + context 'when repository exists' do + let(:snippet) { create(:snippet, :repository) } + + it 'does not try to create repository' do + expect(snippet.repository).not_to receive(:after_create) + + expect(snippet.create_repository).to be_nil + end + + it 'does not track snippet repository' do + expect do + snippet.create_repository + end.not_to change(SnippetRepository, :count) + end + end + end + + describe '#repository_storage' do + let(:snippet) { create(:snippet) } + + it 'returns default repository storage' do + expect(Gitlab::CurrentSettings).to receive(:pick_repository_storage) + + snippet.repository_storage + end + + context 'when snippet_project is already created' do + let!(:snippet_repository) { create(:snippet_repository, snippet: snippet) } + + before do + allow(snippet_repository).to receive(:shard_name).and_return('foo') + end + + it 'returns repository_storage from snippet_project' do + expect(Gitlab::CurrentSettings).not_to receive(:pick_repository_storage) + + expect(snippet.repository_storage).to eq 'foo' + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index ac001ec3118..90795e0a8e4 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -4160,7 +4160,7 @@ describe User, :do_not_mock_admin_mode do describe '#dismissed_callout?' do subject(:user) { create(:user) } - let(:feature_name) { UserCallout.feature_names.keys.first } + let(:feature_name) { UserCallout.feature_names.each_key.first } context 'when no callout dismissal record exists' do it 'returns false when no ignore_dismissal_earlier_than provided' do diff --git a/spec/requests/api/project_container_repositories_spec.rb b/spec/requests/api/project_container_repositories_spec.rb index ece2033f9f8..7cec0867b32 100644 --- a/spec/requests/api/project_container_repositories_spec.rb +++ b/spec/requests/api/project_container_repositories_spec.rb @@ -142,7 +142,8 @@ describe API::ProjectContainerRepositories do let(:worker_params) do { name_regex: 'v10.*', keep_n: 100, - older_than: '1 day' } + older_than: '1 day', + container_expiration_policy: false } end let(:lease_key) { "container_repository:cleanup_tags:#{root_repository.id}" } diff --git a/spec/serializers/test_suite_entity_spec.rb b/spec/serializers/test_suite_entity_spec.rb index 54dca3214b7..6a9653954f3 100644 --- a/spec/serializers/test_suite_entity_spec.rb +++ b/spec/serializers/test_suite_entity_spec.rb @@ -4,7 +4,7 @@ require 'spec_helper' describe TestSuiteEntity do let(:pipeline) { create(:ci_pipeline, :with_test_reports) } - let(:entity) { described_class.new(pipeline.test_reports.test_suites.values.first) } + let(:entity) { described_class.new(pipeline.test_reports.test_suites.each_value.first) } describe '#as_json' do subject(:as_json) { entity.as_json } diff --git a/spec/services/container_expiration_policy_service_spec.rb b/spec/services/container_expiration_policy_service_spec.rb index 1e4899c627f..b2f2b2e1236 100644 --- a/spec/services/container_expiration_policy_service_spec.rb +++ b/spec/services/container_expiration_policy_service_spec.rb @@ -17,7 +17,7 @@ describe ContainerExpirationPolicyService do it 'kicks off a cleanup worker for the container repository' do expect(CleanupContainerRepositoryWorker).to receive(:perform_async) - .with(user.id, container_repository.id, anything) + .with(nil, container_repository.id, hash_including(container_expiration_policy: true)) subject end diff --git a/spec/services/metrics/dashboard/self_monitoring_dashboard_service_spec.rb b/spec/services/metrics/dashboard/self_monitoring_dashboard_service_spec.rb new file mode 100644 index 00000000000..9ee5b06b410 --- /dev/null +++ b/spec/services/metrics/dashboard/self_monitoring_dashboard_service_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Metrics::Dashboard::SelfMonitoringDashboardService, :use_clean_rails_memory_store_caching do + include MetricsDashboardHelpers + + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project) } + let_it_be(:environment) { create(:environment, project: project) } + + before do + project.add_maintainer(user) + stub_application_setting(self_monitoring_project_id: project.id) + end + + describe '#get_dashboard' do + let(:service_params) { [project, user, { environment: environment }] } + let(:service_call) { described_class.new(*service_params).get_dashboard } + + it_behaves_like 'valid dashboard service response' + it_behaves_like 'raises error for users with insufficient permissions' + it_behaves_like 'caches the unprocessed dashboard for subsequent calls' + end + + describe '.all_dashboard_paths' do + it 'returns the dashboard attributes' do + all_dashboards = described_class.all_dashboard_paths(project) + + expect(all_dashboards).to eq( + [{ + path: described_class::DASHBOARD_PATH, + display_name: described_class::DASHBOARD_NAME, + default: true, + system_dashboard: false + }] + ) + end + end + + describe '.valid_params?' do + subject { described_class.valid_params?(params) } + + context 'with environment' do + let(:params) { { environment: environment } } + + it { is_expected.to be_truthy } + end + + context 'with dashboard_path' do + let(:params) { { dashboard_path: self_monitoring_dashboard_path } } + + it { is_expected.to be_truthy } + end + + context 'with a different dashboard selected' do + let(:dashboard_path) { '.gitlab/dashboards/test.yml' } + let(:params) { { dashboard_path: dashboard_path, environment: environment } } + + it { is_expected.to be_falsey } + end + + context 'missing environment and dashboard_path' do + let(:params) { {} } + + it { is_expected.to be_falsey } + end + end +end diff --git a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb index cd4d1e3fe67..ef7e9cda9e0 100644 --- a/spec/services/projects/container_repository/cleanup_tags_service_spec.rb +++ b/spec/services/projects/container_repository/cleanup_tags_service_spec.rb @@ -130,6 +130,38 @@ describe Projects::ContainerRepository::CleanupTagsService do is_expected.to include(status: :success, deleted: %w(Bb Ba C)) end end + + context 'when running a container_expiration_policy' do + let(:user) { nil } + + context 'with valid container_expiration_policy param' do + let(:params) do + { 'name_regex' => '.*', + 'keep_n' => 1, + 'older_than' => '1 day', + 'container_expiration_policy' => true } + end + + it 'succeeds without a user' do + expect_delete('sha256:configB').twice + expect_delete('sha256:configC') + + is_expected.to include(status: :success, deleted: %w(Bb Ba C)) + end + end + + context 'without container_expiration_policy param' do + let(:params) do + { 'name_regex' => '.*', + 'keep_n' => 1, + 'older_than' => '1 day' } + end + + it 'fails' do + is_expected.to include(status: :error, message: 'access denied') + end + end + end end private diff --git a/spec/services/users/block_service_spec.rb b/spec/services/users/block_service_spec.rb new file mode 100644 index 00000000000..c3a65a08c0d --- /dev/null +++ b/spec/services/users/block_service_spec.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Users::BlockService do + let(:current_user) { create(:admin) } + + subject(:service) { described_class.new(current_user) } + + describe '#execute' do + subject(:operation) { service.execute(user) } + + context 'when successful' do + let(:user) { create(:user) } + + it { is_expected.to eq(status: :success) } + + it "change the user's state" do + expect { operation }.to change { user.state }.to('blocked') + end + end + + context 'when failed' do + let(:user) { create(:user, :blocked) } + + it 'returns error result' do + aggregate_failures 'error result' do + expect(operation[:status]).to eq(:error) + expect(operation[:message]).to match(/State cannot transition/) + end + end + + it "does not change the user's state" do + expect { operation }.not_to change { user.state } + end + end + end +end diff --git a/spec/support/helpers/fake_blob_helpers.rb b/spec/support/helpers/fake_blob_helpers.rb index ef4740638ff..a7eafb0fd23 100644 --- a/spec/support/helpers/fake_blob_helpers.rb +++ b/spec/support/helpers/fake_blob_helpers.rb @@ -37,6 +37,8 @@ module FakeBlobHelpers end def fake_blob(**kwargs) - Blob.decorate(FakeBlob.new(**kwargs), project) + container = kwargs.delete(:container) || project + + Blob.decorate(FakeBlob.new(**kwargs), container) end end diff --git a/spec/support/helpers/metrics_dashboard_helpers.rb b/spec/support/helpers/metrics_dashboard_helpers.rb index 908a3e1fb09..b8a641d5911 100644 --- a/spec/support/helpers/metrics_dashboard_helpers.rb +++ b/spec/support/helpers/metrics_dashboard_helpers.rb @@ -29,4 +29,8 @@ module MetricsDashboardHelpers def business_metric_title PrometheusMetricEnums.group_details[:business][:group_title] end + + def self_monitoring_dashboard_path + Metrics::Dashboard::SelfMonitoringDashboardService::DASHBOARD_PATH + end end diff --git a/spec/support/shared_examples/lib/gitlab/repo_type_shared_examples.rb b/spec/support/shared_examples/lib/gitlab/repo_type_shared_examples.rb index 3d88b317417..69ae9339f10 100644 --- a/spec/support/shared_examples/lib/gitlab/repo_type_shared_examples.rb +++ b/spec/support/shared_examples/lib/gitlab/repo_type_shared_examples.rb @@ -2,7 +2,7 @@ RSpec.shared_examples 'a repo type' do describe '#identifier_for_container' do - subject { described_class.identifier_for_container(project) } + subject { described_class.identifier_for_container(expected_container) } it { is_expected.to eq(expected_identifier) } end @@ -35,7 +35,7 @@ RSpec.shared_examples 'a repo type' do describe '#repository_for' do it 'finds the repository for the repo type' do - expect(described_class.repository_for(project)).to eq(expected_repository) + expect(described_class.repository_for(expected_container)).to eq(expected_repository) end end end diff --git a/spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb b/spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb index fdea312dfa9..d5606e65981 100644 --- a/spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb @@ -18,7 +18,7 @@ RSpec.shared_examples 'model with repository' do let(:only_path) { false } it 'returns the full web URL for this repo' do - expect(subject).to eq("#{Gitlab.config.gitlab.url}/#{expected_full_path}") + expect(subject).to eq("#{Gitlab.config.gitlab.url}/#{expected_web_url_path}") end end @@ -26,7 +26,7 @@ RSpec.shared_examples 'model with repository' do let(:only_path) { true } it 'returns the relative web URL for this repo' do - expect(subject).to eq("/#{expected_full_path}") + expect(subject).to eq("/#{expected_web_url_path}") end end @@ -34,14 +34,14 @@ RSpec.shared_examples 'model with repository' do let(:only_path) { nil } it 'returns the full web URL for this repo' do - expect(subject).to eq("#{Gitlab.config.gitlab.url}/#{expected_full_path}") + expect(subject).to eq("#{Gitlab.config.gitlab.url}/#{expected_web_url_path}") end end end context 'when not given the only_path option' do it 'returns the full web URL for this repo' do - expect(container.web_url).to eq("#{Gitlab.config.gitlab.url}/#{expected_full_path}") + expect(container.web_url).to eq("#{Gitlab.config.gitlab.url}/#{expected_web_url_path}") end end end @@ -72,7 +72,7 @@ RSpec.shared_examples 'model with repository' do let(:custom_http_clone_url_root) { 'https://git.example.com:51234/mygitlab/' } it 'returns the url to the repo, with the root replaced with the custom one' do - expect(subject).to eq("#{custom_http_clone_url_root}#{expected_full_path}.git") + expect(subject).to eq("#{custom_http_clone_url_root}#{expected_web_url_path}.git") end end @@ -80,7 +80,7 @@ RSpec.shared_examples 'model with repository' do let(:custom_http_clone_url_root) { 'https://git.example.com:51234/mygitlab' } it 'returns the url to the repo, with the root replaced with the custom one' do - expect(subject).to eq("#{custom_http_clone_url_root}/#{expected_full_path}.git") + expect(subject).to eq("#{custom_http_clone_url_root}/#{expected_web_url_path}.git") end end end @@ -90,7 +90,7 @@ RSpec.shared_examples 'model with repository' do let(:custom_http_clone_url_root) { 'https://git.example.com:51234/' } it 'returns the url to the repo, with the root replaced with the custom one' do - expect(subject).to eq("#{custom_http_clone_url_root}#{expected_full_path}.git") + expect(subject).to eq("#{custom_http_clone_url_root}#{expected_web_url_path}.git") end end @@ -98,7 +98,7 @@ RSpec.shared_examples 'model with repository' do let(:custom_http_clone_url_root) { 'https://git.example.com:51234' } it 'returns the url to the repo, with the root replaced with the custom one' do - expect(subject).to eq("#{custom_http_clone_url_root}/#{expected_full_path}.git") + expect(subject).to eq("#{custom_http_clone_url_root}/#{expected_web_url_path}.git") end end end diff --git a/spec/support/shared_examples/services/boards/boards_list_service_shared_examples.rb b/spec/support/shared_examples/services/boards/boards_list_service_shared_examples.rb index 1488027201c..8f7c08ed625 100644 --- a/spec/support/shared_examples/services/boards/boards_list_service_shared_examples.rb +++ b/spec/support/shared_examples/services/boards/boards_list_service_shared_examples.rb @@ -50,5 +50,20 @@ RSpec.shared_examples 'multiple boards list service' do it 'returns boards ordered by name' do expect(service.execute).to eq [board_a, board_B, board_c] end + + context 'when wanting a specific board' do + it 'returns board specified by id' do + service = described_class.new(parent, double, board_id: board_c.id) + + expect(service.execute).to eq [board_c] + end + + it 'raises exception when board is not found' do + outside_board = create(:board, resource_parent: create(:project), name: 'outside board') + service = described_class.new(parent, double, board_id: outside_board.id) + + expect { service.execute }.to raise_exception(ActiveRecord::RecordNotFound) + end + end end end diff --git a/spec/workers/cleanup_container_repository_worker_spec.rb b/spec/workers/cleanup_container_repository_worker_spec.rb index 9be8f882785..1228c2c2d9c 100644 --- a/spec/workers/cleanup_container_repository_worker_spec.rb +++ b/spec/workers/cleanup_container_repository_worker_spec.rb @@ -6,34 +6,49 @@ describe CleanupContainerRepositoryWorker, :clean_gitlab_redis_shared_state do let(:repository) { create(:container_repository) } let(:project) { repository.project } let(:user) { project.owner } - let(:params) { { key: 'value' } } subject { described_class.new } describe '#perform' do let(:service) { instance_double(Projects::ContainerRepository::CleanupTagsService) } - before do - allow(Projects::ContainerRepository::CleanupTagsService).to receive(:new) - .with(project, user, params).and_return(service) + context 'bulk delete api' do + let(:params) { { key: 'value', 'container_expiration_policy' => false } } + + it 'executes the destroy service' do + expect(Projects::ContainerRepository::CleanupTagsService).to receive(:new) + .with(project, user, params.merge('container_expiration_policy' => false)) + .and_return(service) + expect(service).to receive(:execute) + + subject.perform(user.id, repository.id, params) + end + + it 'does not raise error when user could not be found' do + expect do + subject.perform(-1, repository.id, params) + end.not_to raise_error + end + + it 'does not raise error when repository could not be found' do + expect do + subject.perform(user.id, -1, params) + end.not_to raise_error + end end - it 'executes the destroy service' do - expect(service).to receive(:execute) + context 'container expiration policy' do + let(:params) { { key: 'value', 'container_expiration_policy' => true } } - subject.perform(user.id, repository.id, params) - end + it 'executes the destroy service' do + expect(Projects::ContainerRepository::CleanupTagsService).to receive(:new) + .with(project, nil, params.merge('container_expiration_policy' => true)) + .and_return(service) - it 'does not raise error when user could not be found' do - expect do - subject.perform(-1, repository.id, params) - end.not_to raise_error - end + expect(service).to receive(:execute) - it 'does not raise error when repository could not be found' do - expect do - subject.perform(user.id, -1, params) - end.not_to raise_error + subject.perform(nil, repository.id, params) + end end end end diff --git a/yarn.lock b/yarn.lock index ce6a0e94fc1..3989da9f800 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1774,10 +1774,9 @@ asynckit@^0.4.0: resolved "https://registry.yarnpkg.com/asynckit/-/asynckit-0.4.0.tgz#c79ed97f7f34cb8f2ba1bc9790bcc366474b4b79" integrity sha1-x57Zf380y48robyXkLzDZkdLS3k= -at.js@^1.5.4: +at.js@^1.5.4, "at.js@https://gitlab.com/gitlab-org/frontend/At.js.git#121ce9a557b51c33f5693ac8df52d2bda1e53cbe": version "1.5.4" - resolved "https://registry.yarnpkg.com/at.js/-/at.js-1.5.4.tgz#8fc60cc80eadbe4874449b166a818e7ae1d784c1" - integrity sha512-G8mgUb/PqShPoH8AyjuxsTGvIr1o716BtQUKDM44C8qN2W615y7KGJ68MlTGamd0J0D/m28emUkzagaHTdrGZw== + resolved "https://gitlab.com/gitlab-org/frontend/At.js.git#121ce9a557b51c33f5693ac8df52d2bda1e53cbe" atob@^2.1.1: version "2.1.2" |