diff options
40 files changed, 498 insertions, 420 deletions
diff --git a/.gitlab/issue_templates/Migrations.md b/.gitlab/issue_templates/Migrations.md new file mode 100644 index 00000000000..378e3b01f6a --- /dev/null +++ b/.gitlab/issue_templates/Migrations.md @@ -0,0 +1,67 @@ +# Project Name | Migration Tracker +<!-- Please edit this header with your project / organization's name. --> + +## Background + +<!-- +Please add information here about why you're planning on migrating. Include any initial announcements that have been made about the decision or status. +--> + +### Goals + +<!-- What are some of the goals of your migration to GitLab? Delete this section if you don't want to enumerate goals. --> + +## Quick Facts + +<!-- Please complete as many items in this list as possible. If you're not sure yet, add "TBD" (To be Decided) or "Unknown" --> + + * **Timeline.** - + * **Product.** - GitLab Gold/Ultimate or Commnunity Edition + * **Project's License.** What kind of OSI-approved license does your project use? + +## Current Tooling and Replacements + +<!-- +Please fill in the table to give an overview of your current tooling. Here's a description of what to include in each column: + +- Tool: which tool or platform you are currently using +- Feature: which particular feature you are using in that tool or platform +- GitLab feature: equivalent GitLab feature (the GitLab team can help fill this in, as well as the info in the next column) +- GitLab edition: in which GitLab edition (CE or EE) is this feature available? + +Here's an example of a replacements overview from one of the projects which migrated to GitLab: https://gitlab.com/gitlab-org/gitlab/-/issues/25657#gitlab-replacements + +--> + +| Tool | Feature | GitLab feature | GitLab edition | +| --- | --- | --- | --- | +| | | | | + +## Collaborators + +<!-- Please add names of collaborators in the format: Name, Title, Role (what will you be helping to do, or how should you be involved), GitLab username --> + +## Related Issues + +<!-- Add any related issues that are important for your project by adding the title of the issue and a link to it (preferably as an embedded link). You will probably keep editing this section as the migration progresses, so don't worry if it's mostly blank for now. + +Here is an example of what this list might look like once populated: https://gitlab.com/gitlab-org/gitlab-foss/-/issues/55039#outstanding-issues +--> + +### Blockers + * [ ] ADD_LINK_TO_ISSUE_HERE + +### Urgent + * [ ] + +### Important but not urgent + * [ ] + +### Nice to have + * [ ] + + +------ + +/label ~Open-Source ~movingtogitlab +/cc @nuritzi
\ No newline at end of file diff --git a/.rubocop.yml b/.rubocop.yml index 6dd06984de6..ac7a6a05a28 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -385,4 +385,3 @@ Performance/ChainArrayAllocation: RSpec/RepeatedExample: Exclude: - 'spec/features/merge_request/user_posts_diff_notes_spec.rb' - - 'spec/services/notification_service_spec.rb' diff --git a/app/controllers/admin/application_settings_controller.rb b/app/controllers/admin/application_settings_controller.rb index 1ee3c52ad35..210d488f5a3 100644 --- a/app/controllers/admin/application_settings_controller.rb +++ b/app/controllers/admin/application_settings_controller.rb @@ -192,6 +192,7 @@ class Admin::ApplicationSettingsController < Admin::ApplicationController params[:application_setting][:import_sources]&.delete("") params[:application_setting][:restricted_visibility_levels]&.delete("") params[:application_setting].delete(:elasticsearch_aws_secret_access_key) if params[:application_setting][:elasticsearch_aws_secret_access_key].blank? + params[:application_setting][:required_instance_ci_template] = nil if params[:application_setting][:required_instance_ci_template].blank? # TODO Remove domain_blacklist_raw in APIv5 (See https://gitlab.com/gitlab-org/gitlab-foss/issues/67204) params.delete(:domain_blacklist_raw) if params[:domain_blacklist_file] params.delete(:domain_blacklist_raw) if params[:domain_blacklist] diff --git a/app/controllers/projects/static_site_editor_controller.rb b/app/controllers/projects/static_site_editor_controller.rb new file mode 100644 index 00000000000..597bfccf422 --- /dev/null +++ b/app/controllers/projects/static_site_editor_controller.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class Projects::StaticSiteEditorController < Projects::ApplicationController + layout 'fullscreen' + + prepend_before_action :authenticate_user!, only: [:show] + + def show + end +end diff --git a/app/models/concerns/has_repository.rb b/app/models/concerns/has_repository.rb index 35faa87e876..6a09741b903 100644 --- a/app/models/concerns/has_repository.rb +++ b/app/models/concerns/has_repository.rb @@ -110,7 +110,7 @@ module HasRepository end def web_url(only_path: nil) - raise NotImplementedError + Gitlab::UrlBuilder.build(self, only_path: only_path) end def repository_size_checker diff --git a/app/models/group.rb b/app/models/group.rb index 5e6e3032251..203ed1694b7 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -172,8 +172,8 @@ class Group < Namespace "#{self.class.reference_prefix}#{full_path}" end - def web_url - Gitlab::Routing.url_helpers.group_canonical_url(self) + def web_url(only_path: nil) + Gitlab::UrlBuilder.build(self, only_path: only_path) end def human_name diff --git a/app/models/personal_snippet.rb b/app/models/personal_snippet.rb index 5940265b17a..1b5be8698b1 100644 --- a/app/models/personal_snippet.rb +++ b/app/models/personal_snippet.rb @@ -2,8 +2,4 @@ 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 eb4412decba..15b8d5db214 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -1121,10 +1121,6 @@ class Project < ApplicationRecord end end - def web_url(only_path: nil) - Gitlab::Routing.url_helpers.project_url(self, only_path: only_path) - end - def readme_url readme_path = repository.readme_path if readme_path diff --git a/app/models/project_snippet.rb b/app/models/project_snippet.rb index 6045ec71c6e..ffb08e10f1f 100644 --- a/app/models/project_snippet.rb +++ b/app/models/project_snippet.rb @@ -5,8 +5,4 @@ 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/project_wiki.rb b/app/models/project_wiki.rb index f0967b87345..4b888648b9e 100644 --- a/app/models/project_wiki.rb +++ b/app/models/project_wiki.rb @@ -44,8 +44,8 @@ class ProjectWiki # @deprecated use full_path when you need it for an URL route or disk_path when you want to point to the filesystem alias_method :path_with_namespace, :full_path - def web_url - Gitlab::Routing.url_helpers.project_wiki_url(@project, :home) + def web_url(only_path: nil) + Gitlab::UrlBuilder.build(self, only_path: only_path) end def url_to_repo diff --git a/app/presenters/commit_presenter.rb b/app/presenters/commit_presenter.rb index 94fc8ac8e39..9ded00fcb7a 100644 --- a/app/presenters/commit_presenter.rb +++ b/app/presenters/commit_presenter.rb @@ -18,7 +18,7 @@ class CommitPresenter < Gitlab::View::Presenter::Delegated end def web_url - Gitlab::UrlBuilder.new(commit).url + url_builder.build(commit) end def signature_html diff --git a/app/presenters/issue_presenter.rb b/app/presenters/issue_presenter.rb index 3d55b00ac3b..004813d0374 100644 --- a/app/presenters/issue_presenter.rb +++ b/app/presenters/issue_presenter.rb @@ -4,20 +4,14 @@ class IssuePresenter < Gitlab::View::Presenter::Delegated presents :issue def web_url - url_builder.url + url_builder.build(issue) end def issue_path - url_builder.issue_path(issue) + url_builder.build(issue, only_path: true) end def subscribed? issue.subscribed?(current_user, issue.project) end - - private - - def url_builder - @url_builder ||= Gitlab::UrlBuilder.new(issue) - end end diff --git a/app/presenters/milestone_presenter.rb b/app/presenters/milestone_presenter.rb index 7d9045ddebe..6bf8203702f 100644 --- a/app/presenters/milestone_presenter.rb +++ b/app/presenters/milestone_presenter.rb @@ -4,12 +4,6 @@ class MilestonePresenter < Gitlab::View::Presenter::Delegated presents :milestone def milestone_path - url_builder.milestone_path(milestone) - end - - private - - def url_builder - @url_builder ||= Gitlab::UrlBuilder.new(milestone) + url_builder.build(milestone, only_path: true) end end diff --git a/app/services/snippets/update_service.rb b/app/services/snippets/update_service.rb index 5794332a45b..e56b20c6057 100644 --- a/app/services/snippets/update_service.rb +++ b/app/services/snippets/update_service.rb @@ -35,28 +35,43 @@ module Snippets private def save_and_commit(snippet) - snippet.with_transaction_returning_status do - snippet.save.tap do |saved| - break false unless saved - - # In order to avoid non migrated snippets scenarios, - # if the snippet does not have a repository we created it - # We don't need to check if the repository exists - # because `create_repository` already handles it - if Feature.enabled?(:version_snippets, current_user) - create_repository_for(snippet) - end - - # If the snippet repository exists we commit always - # the changes - create_commit(snippet) if snippet.repository_exists? - end - rescue => e - snippet.errors.add(:repository, e.message) - log_error(e.message) + return false unless snippet.save + + # In order to avoid non migrated snippets scenarios, + # if the snippet does not have a repository we created it + # We don't need to check if the repository exists + # because `create_repository` already handles it + if Feature.enabled?(:version_snippets, current_user) + create_repository_for(snippet) + end + + # If the snippet repository exists we commit always + # the changes + create_commit(snippet) if snippet.repository_exists? + + true + rescue => e + # Restore old attributes + unless snippet.previous_changes.empty? + snippet.previous_changes.each { |attr, value| snippet[attr] = value[0] } + snippet.save + end - false + snippet.errors.add(:repository, 'Error updating the snippet') + log_error(e.message) + + # If the commit action failed we remove it because + # we don't want to leave empty repositories + # around, to allow cloning them. + if repository_empty?(snippet) + snippet.repository.remove + snippet.snippet_repository&.delete end + + # Purge any existing value for repository_exists? + snippet.repository.expire_exists_cache + + false end def create_repository_for(snippet) @@ -81,5 +96,13 @@ module Snippets file_path: params[:file_name], content: params[:content] }] end + + # Because we are removing repositories we don't want to remove + # any existing repository with data. Therefore, we cannot + # rely on cached methods for that check in order to avoid losing + # data. + def repository_empty?(snippet) + snippet.repository._uncached_exists? && !snippet.repository._uncached_has_visible_content? + end end end diff --git a/app/views/projects/static_site_editor/show.html.haml b/app/views/projects/static_site_editor/show.html.haml new file mode 100644 index 00000000000..574cdd0bf88 --- /dev/null +++ b/app/views/projects/static_site_editor/show.html.haml @@ -0,0 +1 @@ +#static-site-editor{ data: {} } diff --git a/changelogs/unreleased/fj-213436-move-update-outside-transaction.yml b/changelogs/unreleased/fj-213436-move-update-outside-transaction.yml new file mode 100644 index 00000000000..f37c6367b3c --- /dev/null +++ b/changelogs/unreleased/fj-213436-move-update-outside-transaction.yml @@ -0,0 +1,5 @@ +--- +title: Fix race condition updating snippet without repository +merge_request: 28851 +author: +type: fixed diff --git a/changelogs/unreleased/notification-service-spec.yml b/changelogs/unreleased/notification-service-spec.yml new file mode 100644 index 00000000000..50029b0c09a --- /dev/null +++ b/changelogs/unreleased/notification-service-spec.yml @@ -0,0 +1,5 @@ +--- +title: Update duplicate specs in notification service spec +merge_request: 28742 +author: Rajendra Kadam +type: fixed diff --git a/config/routes/repository.rb b/config/routes/repository.rb index 38d6cdbbaf8..8a635745907 100644 --- a/config/routes/repository.rb +++ b/config/routes/repository.rb @@ -67,6 +67,10 @@ scope format: false do end end + scope controller: :static_site_editor do + get '/sse/*id', action: :show, as: :show_sse + end + get '/tree/*id', to: 'tree#show', as: :tree get '/raw/*id', to: 'raw#show', as: :raw get '/blame/*id', to: 'blame#show', as: :blame diff --git a/danger/telemetry/Dangerfile b/danger/telemetry/Dangerfile index 227621cc1dd..f308fb206bb 100644 --- a/danger/telemetry/Dangerfile +++ b/danger/telemetry/Dangerfile @@ -1,13 +1,12 @@ # frozen_string_literal: true TELEMETRY_CHANGED_FILES_MESSAGE = <<~MSG -This merge request adds or changes files for which a -review from the Data team and Telemetry team is recommended. -@gitlab-org/growth/telemetry group is mentioned in order to notify team members. +This merge request includes changes for which a review from the Data team and Telemetry team is recommended. +Please reach out to @gitlab-org/growth/telemetry/engineers group for a review. MSG USAGE_DATA_FILES_MESSAGE = <<~MSG -For the following files, a review from the [Data team and Telemetry team](https://gitlab.com/groups/gitlab-org/growth/telemetry/-/group_members?with_inherited_permissions=exclude) is recommended: +For the following files, a review from the [Data team and Telemetry team](https://gitlab.com/groups/gitlab-org/growth/telemetry/engineers/-/group_members?with_inherited_permissions=exclude) is recommended: MSG usage_data_changed_files = git.modified_files.grep(%r{usage_data}) diff --git a/doc/administration/logs.md b/doc/administration/logs.md index 5a6eb95de42..889a8b2d6b0 100644 --- a/doc/administration/logs.md +++ b/doc/administration/logs.md @@ -434,6 +434,10 @@ I, [2015-02-13T06:17:00.679433 #9291] INFO -- : Moving existing hooks directory User clone/fetch activity using SSH transport appears in this log as `executing git command <gitaly-upload-pack...`. +## `current` + +This file lives in `/var/log/gitlab/gitaly/current` and is produced by [runit](http://smarden.org/runit/). `runit` is packaged with Omnibus and a brief explanation of its purpose is available [in the omnibus documentation](https://docs.gitlab.com/omnibus/architecture/#runit). [Log files are rotated](http://smarden.org/runit/svlogd.8.html), renamed in unix timestamp format and `gzip`-compressed (e.g. `@1584057562.s`). + ## `unicorn_stderr.log` This file lives in `/var/log/gitlab/unicorn/unicorn_stderr.log` for diff --git a/doc/user/application_security/dependency_scanning/index.md b/doc/user/application_security/dependency_scanning/index.md index c83e69ed6c4..651a7730cdb 100644 --- a/doc/user/application_security/dependency_scanning/index.md +++ b/doc/user/application_security/dependency_scanning/index.md @@ -167,7 +167,7 @@ The following variables are used for configuring specific analyzers (used for a | `DS_PIP_VERSION` | `gemnasium-python` | | Force the install of a specific pip version (example: `"19.3"`), otherwise the pip installed in the Docker image is used. ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12811) in GitLab 12.7) | | `DS_PIP_DEPENDENCY_PATH` | `gemnasium-python` | | Path to load Python pip dependencies from. ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12412) in GitLab 12.2) | | `DS_PYTHON_VERSION` | `retire.js` | | Version of Python. If set to 2, dependencies are installed using Python 2.7 instead of Python 3.6. ([Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12296) in GitLab 12.1)| -| `MAVEN_CLI_OPTS` | `gemnasium-maven` | `"-DskipTests --batch-mode"` | List of command line arguments that will be passed to `maven` by the analyzer. See an example for [using private repos](#using-private-maven-repos). | +| `MAVEN_CLI_OPTS` | `gemnasium-maven` | `"-DskipTests --batch-mode"` | List of command line arguments that will be passed to `maven` by the analyzer. See an example for [using private repos](../index.md#using-private-maven-repos). | | `BUNDLER_AUDIT_UPDATE_DISABLED` | `bundler-audit` | `"false"` | Disable automatic updates for the `bundler-audit` analyzer. Useful if you're running Dependency Scanning in an offline, air-gapped environment.| | `BUNDLER_AUDIT_ADVISORY_DB_URL` | `bundler-audit` | `https://github.com/rubysec/ruby-advisory-db` | URL of the advisory database used by bundler-audit. | | `BUNDLER_AUDIT_ADVISORY_DB_REF_NAME` | `bundler-audit` | `master` | Git ref for the advisory database specified by `BUNDLER_AUDIT_ADVISORY_DB_URL`. | @@ -177,28 +177,9 @@ The following variables are used for configuring specific analyzers (used for a ### Using private Maven repos If you have a private Maven repository which requires login credentials, -you can use the `MAVEN_CLI_OPTS` environment variable to pass variables -specified in your settings (e.g., username, password, etc.). - -For example, if you have a settings file in your project source (e.g., `mysettings.xml`) -that looks like the following, you can specify the variables -[by adding an entry under your project's settings](../../../ci/variables/README.md#via-the-ui), -so that you don't have to expose your private data in `.gitlab-ci.yml` (e.g., adding -`MAVEN_CLI_OPTS` with value `--settings mysettings.xml -Dprivate.username=foo -Dprivate.password=bar`). - -```xml -<!-- mysettings.xml --> -<settings> - ... - <servers> - <server> - <id>private_server</id> - <username>${private.username}</username> - <password>${private.password}</password> - </server> - </servers> -</settings> -``` +you can use the `MAVEN_CLI_OPTS` environment variable. + +Read more on [how to use private Maven repos](../index.md#using-private-maven-repos). ### Disabling Docker in Docker for Dependency Scanning @@ -217,6 +198,14 @@ variables: This will create individual `<analyzer-name>-dependency_scanning` jobs for each analyzer that runs in your CI/CD pipeline. +By removing Docker-in-Docker (DIND), GitLab relies on [Linguist](https://github.com/github/linguist) +to start relevant analyzers depending on the detected repository language(s) instead of the +[orchestrator](https://gitlab.com/gitlab-org/security-products/dependency-scanning/). However, there +are some differences in the way repository languages are detected between DIND and non-DIND. You can +observe these differences by checking both Linguist and the common library. For instance, Linguist +looks for `*.java` files to spin up the [gemnasium-maven](https://gitlab.com/gitlab-org/security-products/analyzers/gemnasium-maven) +image, while orchestrator only looks for the existence of `pom.xml` or `build.gradle`. + ## Interacting with the vulnerabilities Once a vulnerability is found, you can interact with it. Read more on how to diff --git a/doc/user/application_security/index.md b/doc/user/application_security/index.md index 299507ff6c4..dadff8583db 100644 --- a/doc/user/application_security/index.md +++ b/doc/user/application_security/index.md @@ -251,6 +251,35 @@ environment. Read how to [operate the Secure scanners in an offline environment](offline_deployments/index.md). +## Using private Maven repos + +If you have a private Apache Maven repository that requires login credentials, +you can use the `MAVEN_CLI_OPTS` environment variable +to pass a username and password. You can set it under your project's settings +so that your credentials aren't exposed in `.gitlab-ci.yml`. + +If the username is `myuser` and the password is `verysecret` then you would +[set the following variable](../../ci/variables/README.md#via-the-ui) +under your project's settings: + +| Type | Key | Value | +| ---- | --- | ----- | +| Variable | `MAVEN_CLI_OPTS` | `--settings mysettings.xml -Drepository.password=verysecret -Drepository.user=myuser` | + +```xml +<!-- mysettings.xml --> +<settings> + ... + <servers> + <server> + <id>private_server</id> + <username>${private.username}</username> + <password>${private.password}</password> + </server> + </servers> +</settings> +``` + ## Outdated security reports > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/4913) in GitLab 12.7. diff --git a/doc/user/application_security/sast/index.md b/doc/user/application_security/sast/index.md index 64a8b1b40dd..9c6098e4e04 100644 --- a/doc/user/application_security/sast/index.md +++ b/doc/user/application_security/sast/index.md @@ -166,18 +166,10 @@ it via [custom environment variables](#custom-environment-variables). #### Using a variable to pass username and password to a private Maven repository -If you have a private Apache Maven repository that requires login credentials, -you can use the `MAVEN_CLI_OPTS` [environment variable](#available-variables) -to pass a username and password. You can set it under your project's settings -so that your credentials aren't exposed in `.gitlab-ci.yml`. +If you have a private Maven repository which requires login credentials, +you can use the `MAVEN_CLI_OPTS` environment variable. -If the username is `myuser` and the password is `verysecret` then you would -[set the following variable](../../../ci/variables/README.md#via-the-ui) -under your project's settings: - -| Type | Key | Value | -| ---- | --- | ----- | -| Variable | `MAVEN_CLI_OPTS` | `-Drepository.password=verysecret -Drepository.user=myuser` | +Read more on [how to use private Maven repos](../index.md#using-private-maven-repos). ### Disabling Docker in Docker for SAST @@ -194,6 +186,15 @@ variables: This will create individual `<analyzer-name>-sast` jobs for each analyzer that runs in your CI/CD pipeline. +By removing Docker-in-Docker (DIND), GitLab relies on [Linguist](https://github.com/github/linguist) +to start relevant analyzers depending on the detected repository language(s) instead of the +[orchestrator](https://gitlab.com/gitlab-org/security-products/dependency-scanning/). However, there +are some differences in the way repository languages are detected between DIND and non-DIND. You can +observe these differences by checking both Linguist and the common library. For instance, Linguist +looks for `*.java` files to spin up the [spotbugs](https://gitlab.com/gitlab-org/security-products/analyzers/spotbugs) +image, while orchestrator only looks for the existence of `pom.xml`, `build.xml`, `gradlew`, +`grailsw`, or `mvnw`. + #### Enabling kubesec analyzer > [Introduced](https://gitlab.com/gitlab-org/gitlab/issues/12752) in GitLab Ultimate 12.6. diff --git a/doc/user/compliance/license_compliance/index.md b/doc/user/compliance/license_compliance/index.md index ef20e074b3e..87dc5406c89 100644 --- a/doc/user/compliance/license_compliance/index.md +++ b/doc/user/compliance/license_compliance/index.md @@ -191,35 +191,10 @@ If you still need to run tests during `mvn install`, add `-DskipTests=false` to #### Using private Maven repos -If you have a private Maven repository that requires login credentials, you can use the -`MAVEN_CLI_OPTS` variable to specify a custom [`settings.xml`](http://maven.apache.org/settings.html) -file. - -For example, you may have a settings file like this in your project source: - -```xml -<settings> - <servers> - <server> - <id>my-server</id> - <username>${private.username}</username> - <username>${private.password}</username> - </server> - </servers> -</settings> -``` - -You can use this file through the following declaration in your `gitlab-ci.yml` file: - -```yaml -license_scanning: - variables: - MAVEN_CLI_OPTS: --settings settings.xml -Dprivate.username=foo -Dprivate.password=bar -``` +If you have a private Maven repository which requires login credentials, +you can use the `MAVEN_CLI_OPTS` environment variable. -NOTE: **Note:** -If you don't want to expose the credentials in your `.gitlab-ci.yml` file, then -you can [set the variable in your project's settings](../../../ci/variables/README.md#via-the-ui). +Read more on [how to use private Maven repos](../../application_security/index.md#using-private-maven-repos). ### Selecting the version of Python diff --git a/lib/gitlab/path_regex.rb b/lib/gitlab/path_regex.rb index d95da262eea..df48e347511 100644 --- a/lib/gitlab/path_regex.rb +++ b/lib/gitlab/path_regex.rb @@ -98,6 +98,7 @@ module Gitlab preview raw refs + sse tree update wikis diff --git a/lib/gitlab/url_builder.rb b/lib/gitlab/url_builder.rb index cc53e3b7577..4c56b9bb3c9 100644 --- a/lib/gitlab/url_builder.rb +++ b/lib/gitlab/url_builder.rb @@ -2,76 +2,74 @@ module Gitlab class UrlBuilder + include Singleton include Gitlab::Routing include GitlabRoutingHelper - include ActionView::RecordIdentifier - attr_reader :object, :opts + delegate :build, to: :class - def self.build(object, opts = {}) - new(object, opts).url - end + class << self + include ActionView::RecordIdentifier - def url - # Objects are sometimes wrapped in a BatchLoader instance - case object.itself - when Commit - commit_url - when Issue - issue_url(object) - when MergeRequest - merge_request_url(object) - when Note - note_url - when WikiPage - wiki_page_url - when Snippet - opts[:raw].present? ? gitlab_raw_snippet_url(object) : gitlab_snippet_url(object) - when Milestone - milestone_url(object) - when ::Ci::Build - project_job_url(object.project, object) - when User - user_url(object) - else - raise NotImplementedError.new("No URL builder defined for #{object.inspect}") + def build(object, **options) + # Objects are sometimes wrapped in a BatchLoader instance + case object.itself + when ::Ci::Build + instance.project_job_url(object.project, object, **options) + when Commit + commit_url(object, **options) + when Group + instance.group_canonical_url(object, **options) + when Issue + instance.issue_url(object, **options) + when MergeRequest + instance.merge_request_url(object, **options) + when Milestone + instance.milestone_url(object, **options) + when Note + note_url(object, **options) + when Project + instance.project_url(object, **options) + when Snippet + snippet_url(object, **options) + when User + instance.user_url(object, **options) + when ProjectWiki + instance.project_wiki_url(object.project, :home, **options) + when WikiPage + instance.project_wiki_url(object.wiki.project, object.slug, **options) + else + raise NotImplementedError.new("No URL builder defined for #{object.inspect}") + end end - end - - private - def initialize(object, opts = {}) - @object = object - @opts = opts - end - - def commit_url(opts = {}) - return '' if object.project.nil? + def commit_url(commit, **options) + return '' unless commit.project - namespace_project_commit_url({ - namespace_id: object.project.namespace, - project_id: object.project, - id: object.id - }.merge!(opts)) - end - - def note_url - if object.for_commit? - commit_url(id: object.commit_id, anchor: dom_id(object)) - - elsif object.for_issue? - issue_url(object.noteable, anchor: dom_id(object)) + instance.commit_url(commit, **options) + end - elsif object.for_merge_request? - merge_request_url(object.noteable, anchor: dom_id(object)) + def note_url(note, **options) + if note.for_commit? + return '' unless note.project - elsif object.for_snippet? - gitlab_snippet_url(object.noteable, anchor: dom_id(object)) + instance.project_commit_url(note.project, note.commit_id, anchor: dom_id(note), **options) + elsif note.for_issue? + instance.issue_url(note.noteable, anchor: dom_id(note), **options) + elsif note.for_merge_request? + instance.merge_request_url(note.noteable, anchor: dom_id(note), **options) + elsif note.for_snippet? + instance.gitlab_snippet_url(note.noteable, anchor: dom_id(note), **options) + end end - end - def wiki_page_url - project_wiki_url(object.wiki.project, object.slug) + def snippet_url(snippet, **options) + if options.delete(:raw).present? + instance.gitlab_raw_snippet_url(snippet, **options) + else + instance.gitlab_snippet_url(snippet, **options) + end + end end end end diff --git a/lib/gitlab/view/presenter/base.rb b/lib/gitlab/view/presenter/base.rb index 5e70afe730a..5d241b9b4e9 100644 --- a/lib/gitlab/view/presenter/base.rb +++ b/lib/gitlab/view/presenter/base.rb @@ -26,6 +26,10 @@ module Gitlab self end + def url_builder + Gitlab::UrlBuilder.instance + end + class_methods do def presenter? true diff --git a/spec/controllers/projects/static_site_editor_controller_spec.rb b/spec/controllers/projects/static_site_editor_controller_spec.rb new file mode 100644 index 00000000000..7f1b67fc734 --- /dev/null +++ b/spec/controllers/projects/static_site_editor_controller_spec.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::StaticSiteEditorController do + let(:project) { create(:project, :public, :repository) } + + describe 'GET show' do + let(:default_params) do + { + namespace_id: project.namespace, + project_id: project, + id: 'master/README.md' + } + end + + context 'User roles' do + context 'anonymous' do + before do + get :show, params: default_params + end + + it 'redirects to sign in and returns' do + expect(response).to redirect_to(new_user_session_path) + end + end + + %w[guest developer maintainer].each do |role| + context "as #{role}" do + let(:user) { create(:user) } + + before do + project.add_role(user, role) + sign_in(user) + get :show, params: default_params + end + + it 'renders the edit page' do + expect(response).to render_template(:show) + end + end + end + end + end +end diff --git a/spec/factories/milestones.rb b/spec/factories/milestones.rb index 151d286cc29..ae98ac1bbd7 100644 --- a/spec/factories/milestones.rb +++ b/spec/factories/milestones.rb @@ -25,6 +25,14 @@ FactoryBot.define do due_date { Date.new(2000, 1, 30) } end + trait :on_project do + project + end + + trait :on_group do + group + end + after(:build, :stub) do |milestone, evaluator| if evaluator.group milestone.group = evaluator.group @@ -44,5 +52,7 @@ FactoryBot.define do factory :active_milestone, traits: [:active] factory :closed_milestone, traits: [:closed] + factory :project_milestone, traits: [:on_project] + factory :group_milestone, traits: [:on_group] end end diff --git a/spec/features/projects/snippets/user_updates_snippet_spec.rb b/spec/features/projects/snippets/user_updates_snippet_spec.rb index 58ca922b9cb..bad3fde8a4a 100644 --- a/spec/features/projects/snippets/user_updates_snippet_spec.rb +++ b/spec/features/projects/snippets/user_updates_snippet_spec.rb @@ -54,11 +54,9 @@ describe 'Projects > Snippets > User updates a snippet', :js do end context 'when the git operation fails' do - let(:error_message) { 'foobar' } - before do allow_next_instance_of(Snippets::UpdateService) do |instance| - allow(instance).to receive(:create_commit).and_raise(StandardError, error_message) + allow(instance).to receive(:create_commit).and_raise(StandardError) end fill_in('project_snippet_title', with: 'Snippet new title') @@ -67,7 +65,7 @@ describe 'Projects > Snippets > User updates a snippet', :js do end it 'renders edit page and displays the error' do - expect(page.find('.flash-container span').text).to eq(error_message) + expect(page.find('.flash-container span').text).to eq('Error updating the snippet') expect(page).to have_content('Edit Snippet') end end diff --git a/spec/features/snippets/user_edits_snippet_spec.rb b/spec/features/snippets/user_edits_snippet_spec.rb index 1ec18e3e0e3..0bbb92b1f3f 100644 --- a/spec/features/snippets/user_edits_snippet_spec.rb +++ b/spec/features/snippets/user_edits_snippet_spec.rb @@ -85,11 +85,9 @@ describe 'User edits snippet', :js do end context 'when the git operation fails' do - let(:error_message) { 'foobar' } - before do allow_next_instance_of(Snippets::UpdateService) do |instance| - allow(instance).to receive(:create_commit).and_raise(StandardError, error_message) + allow(instance).to receive(:create_commit).and_raise(StandardError) end fill_in 'personal_snippet_title', with: 'New Snippet Title' @@ -98,7 +96,7 @@ describe 'User edits snippet', :js do end it 'renders edit page and displays the error' do - expect(page.find('.flash-container span').text).to eq(error_message) + expect(page.find('.flash-container span').text).to eq('Error updating the snippet') expect(page).to have_content('Edit Snippet') end end diff --git a/spec/frontend/__mocks__/@gitlab/ui.js b/spec/frontend/__mocks__/@gitlab/ui.js index 7f893bf7ed7..d65fab80d3b 100644 --- a/spec/frontend/__mocks__/@gitlab/ui.js +++ b/spec/frontend/__mocks__/@gitlab/ui.js @@ -31,6 +31,6 @@ export const GlPopover = { }, }, render(h) { - return h('div', this.$attrs, this.$slots.default); + return h('div', this.$attrs, Object.keys(this.$slots).map(s => this.$slots[s])); }, }; diff --git a/spec/lib/gitlab/url_builder_spec.rb b/spec/lib/gitlab/url_builder_spec.rb index c2eb1b4c25d..1b23f331b89 100644 --- a/spec/lib/gitlab/url_builder_spec.rb +++ b/spec/lib/gitlab/url_builder_spec.rb @@ -3,186 +3,116 @@ require 'spec_helper' describe Gitlab::UrlBuilder do - describe '.build' do - context 'when passing a Commit' do - it 'returns a proper URL' do - commit = build_stubbed(:commit) - - url = described_class.build(commit) - - expect(url).to eq "#{Settings.gitlab['url']}/#{commit.project.full_path}/-/commit/#{commit.id}" - end - end - - context 'when passing a batch loaded Commit' do - it 'returns a proper URL' do - commit = BatchLoader.for(:commit).batch do |batch, loader| - batch.each { |commit| loader.call(:commit, build_stubbed(:commit)) } - end + subject { described_class } - url = described_class.build(commit) + describe '#build' do + it 'delegates to the class method' do + expect(subject).to receive(:build).with(:foo, bar: :baz) - expect(url).to eq "#{Settings.gitlab['url']}/#{commit.project.full_path}/-/commit/#{commit.id}" - end + subject.instance.build(:foo, bar: :baz) end + end - context 'when passing an Issue' do - it 'returns a proper URL' do - issue = build_stubbed(:issue, iid: 42) - - url = described_class.build(issue) - - expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.full_path}/-/issues/#{issue.iid}" - end + describe '.build' do + using RSpec::Parameterized::TableSyntax + + where(:factory, :path_generator) do + :project | ->(project) { "/#{project.full_path}" } + :commit | ->(commit) { "/#{commit.project.full_path}/-/commit/#{commit.id}" } + :issue | ->(issue) { "/#{issue.project.full_path}/-/issues/#{issue.iid}" } + :merge_request | ->(merge_request) { "/#{merge_request.project.full_path}/-/merge_requests/#{merge_request.iid}" } + :project_milestone | ->(milestone) { "/#{milestone.project.full_path}/-/milestones/#{milestone.iid}" } + :project_snippet | ->(snippet) { "/#{snippet.project.full_path}/snippets/#{snippet.id}" } + :project_wiki | ->(wiki) { "/#{wiki.project.full_path}/-/wikis/home" } + :ci_build | ->(build) { "/#{build.project.full_path}/-/jobs/#{build.id}" } + + :group | ->(group) { "/groups/#{group.full_path}" } + :group_milestone | ->(milestone) { "/groups/#{milestone.group.full_path}/-/milestones/#{milestone.iid}" } + + :user | ->(user) { "/#{user.full_path}" } + :personal_snippet | ->(snippet) { "/snippets/#{snippet.id}" } + :wiki_page | ->(wiki_page) { "#{wiki_page.wiki.wiki_base_path}/#{wiki_page.slug}" } + + :note_on_commit | ->(note) { "/#{note.project.full_path}/-/commit/#{note.commit_id}#note_#{note.id}" } + :diff_note_on_commit | ->(note) { "/#{note.project.full_path}/-/commit/#{note.commit_id}#note_#{note.id}" } + :discussion_note_on_commit | ->(note) { "/#{note.project.full_path}/-/commit/#{note.commit_id}#note_#{note.id}" } + :legacy_diff_note_on_commit | ->(note) { "/#{note.project.full_path}/-/commit/#{note.commit_id}#note_#{note.id}" } + + :note_on_issue | ->(note) { "/#{note.project.full_path}/-/issues/#{note.noteable.iid}#note_#{note.id}" } + :discussion_note_on_issue | ->(note) { "/#{note.project.full_path}/-/issues/#{note.noteable.iid}#note_#{note.id}" } + + :note_on_merge_request | ->(note) { "/#{note.project.full_path}/-/merge_requests/#{note.noteable.iid}#note_#{note.id}" } + :diff_note_on_merge_request | ->(note) { "/#{note.project.full_path}/-/merge_requests/#{note.noteable.iid}#note_#{note.id}" } + :discussion_note_on_merge_request | ->(note) { "/#{note.project.full_path}/-/merge_requests/#{note.noteable.iid}#note_#{note.id}" } + :legacy_diff_note_on_merge_request | ->(note) { "/#{note.project.full_path}/-/merge_requests/#{note.noteable.iid}#note_#{note.id}" } + + :note_on_project_snippet | ->(note) { "/#{note.project.full_path}/snippets/#{note.noteable_id}#note_#{note.id}" } + :discussion_note_on_project_snippet | ->(note) { "/#{note.project.full_path}/snippets/#{note.noteable_id}#note_#{note.id}" } + :discussion_note_on_personal_snippet | ->(note) { "/snippets/#{note.noteable_id}#note_#{note.id}" } + :note_on_personal_snippet | ->(note) { "/snippets/#{note.noteable_id}#note_#{note.id}" } end - context 'when passing a Milestone' do - let(:group) { create(:group) } - let(:project) { create(:project, :public, namespace: group) } - - context 'belonging to a project' do - it 'returns a proper URL' do - milestone = create(:milestone, project: project) + with_them do + let(:object) { build_stubbed(factory) } + let(:path) { path_generator.call(object) } - url = described_class.build(milestone) - - expect(url).to eq "#{Settings.gitlab['url']}/#{milestone.project.full_path}/-/milestones/#{milestone.iid}" - end + it 'returns the full URL' do + expect(subject.build(object)).to eq("#{Gitlab.config.gitlab.url}#{path}") end - context 'belonging to a group' do - it 'returns a proper URL' do - milestone = create(:milestone, group: group) - - url = described_class.build(milestone) - - expect(url).to eq "#{Settings.gitlab['url']}/groups/#{milestone.group.full_path}/-/milestones/#{milestone.iid}" - end + it 'returns only the path if only_path is given' do + expect(subject.build(object, only_path: true)).to eq(path) end end - context 'when passing a MergeRequest' do - it 'returns a proper URL' do - merge_request = build_stubbed(:merge_request, iid: 42) + context 'when passing a commit without a project' do + let(:commit) { build_stubbed(:commit) } - url = described_class.build(merge_request) + it 'returns an empty string' do + allow(commit).to receive(:project).and_return(nil) - expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.full_path}/-/merge_requests/#{merge_request.iid}" + expect(subject.build(commit)).to eq('') end end - context 'when passing a ProjectSnippet' do - it 'returns a proper URL' do - project_snippet = create(:project_snippet) + context 'when passing a commit note without a project' do + let(:note) { build_stubbed(:note_on_commit) } - url = described_class.build(project_snippet) + it 'returns an empty string' do + allow(note).to receive(:project).and_return(nil) - expect(url).to eq "#{Settings.gitlab['url']}/#{project_snippet.project.full_path}/snippets/#{project_snippet.id}" + expect(subject.build(note)).to eq('') end end - context 'when passing a PersonalSnippet' do - it 'returns a proper URL' do - personal_snippet = create(:personal_snippet) + context 'when passing a Snippet' do + let(:snippet) { build_stubbed(:personal_snippet) } - url = described_class.build(personal_snippet) + it 'returns a raw snippet URL if requested' do + url = subject.build(snippet, raw: true) - expect(url).to eq "#{Settings.gitlab['url']}/snippets/#{personal_snippet.id}" + expect(url).to eq "#{Gitlab.config.gitlab.url}/snippets/#{snippet.id}/raw" end end - context 'when passing a Note' do - context 'on a Commit' do - it 'returns a proper URL' do - note = build_stubbed(:note_on_commit) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.full_path}/-/commit/#{note.commit_id}#note_#{note.id}" - end - end - - context 'on a Commit Diff' do - it 'returns a proper URL' do - note = build_stubbed(:diff_note_on_commit) - - url = described_class.build(note) + context 'when passing an unsupported class' do + let(:object) { Object.new } - expect(url).to eq "#{Settings.gitlab['url']}/#{note.project.full_path}/-/commit/#{note.commit_id}#note_#{note.id}" - end + it 'raises an exception' do + expect { subject.build(object) }.to raise_error(NotImplementedError) end + end - context 'on an Issue' do - it 'returns a proper URL' do - issue = create(:issue, iid: 42) - note = build_stubbed(:note_on_issue, noteable: issue) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{issue.project.full_path}/-/issues/#{issue.iid}#note_#{note.id}" - end - end - - context 'on a MergeRequest' do - it 'returns a proper URL' do - merge_request = create(:merge_request, iid: 42) - note = build_stubbed(:note_on_merge_request, noteable: merge_request) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.full_path}/-/merge_requests/#{merge_request.iid}#note_#{note.id}" - end - end - - context 'on a MergeRequest Diff' do - it 'returns a proper URL' do - merge_request = create(:merge_request, iid: 42) - note = build_stubbed(:diff_note_on_merge_request, noteable: merge_request) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{merge_request.project.full_path}/-/merge_requests/#{merge_request.iid}#note_#{note.id}" - end - end - - context 'on a ProjectSnippet' do - it 'returns a proper URL' do - project_snippet = create(:project_snippet) - note = build_stubbed(:note_on_project_snippet, noteable: project_snippet) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/#{project_snippet.project.full_path}/snippets/#{note.noteable_id}#note_#{note.id}" - end - end - - context 'on a PersonalSnippet' do - it 'returns a proper URL' do - personal_snippet = create(:personal_snippet) - note = build_stubbed(:note_on_personal_snippet, noteable: personal_snippet) - - url = described_class.build(note) - - expect(url).to eq "#{Settings.gitlab['url']}/snippets/#{note.noteable_id}#note_#{note.id}" - end - end - - context 'on another object' do - it 'returns a proper URL' do - project = build_stubbed(:project) - - expect { described_class.build(project) } - .to raise_error(NotImplementedError, "No URL builder defined for #{project.inspect}") + context 'when passing a batch loaded model' do + let(:project) { build_stubbed(:project) } + let(:object) do + BatchLoader.for(:project).batch do |batch, loader| + batch.each { |_| loader.call(:project, project) } end end - end - - context 'when passing a WikiPage' do - it 'returns a proper URL' do - wiki_page = build(:wiki_page) - url = described_class.build(wiki_page) - expect(url).to eq "#{Gitlab.config.gitlab.url}#{wiki_page.wiki.wiki_base_path}/#{wiki_page.slug}" + it 'returns the URL for the real object' do + expect(subject.build(object, only_path: true)).to eq("/#{project.full_path}") end end end diff --git a/spec/models/personal_snippet_spec.rb b/spec/models/personal_snippet_spec.rb index 4a949a75cbd..a055f107e33 100644 --- a/spec/models/personal_snippet_spec.rb +++ b/spec/models/personal_snippet_spec.rb @@ -21,8 +21,6 @@ describe PersonalSnippet 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 09b4ec3677c..719a74f995d 100644 --- a/spec/models/project_snippet_spec.rb +++ b/spec/models/project_snippet_spec.rb @@ -37,8 +37,6 @@ describe ProjectSnippet 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 0904ebca670..21c074cdce2 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -116,9 +116,6 @@ describe Project do let_it_be(:container) { create(:project, :repository, path: 'somewhere') } let(:stubbed_container) { build_stubbed(:project) } 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 diff --git a/spec/models/project_wiki_spec.rb b/spec/models/project_wiki_spec.rb index 2d660d1deab..9f5fd3a9495 100644 --- a/spec/models/project_wiki_spec.rb +++ b/spec/models/project_wiki_spec.rb @@ -28,7 +28,7 @@ describe ProjectWiki do describe '#web_url' do it 'returns the full web URL to the wiki' do - expect(subject.web_url).to eq("#{Gitlab.config.gitlab.url}/#{project.full_path}/-/wikis/home") + expect(subject.web_url).to eq(Gitlab::UrlBuilder.build(subject)) end end diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb index 9fa8f807330..86f37e9204c 100644 --- a/spec/services/notification_service_spec.rb +++ b/spec/services/notification_service_spec.rb @@ -1022,24 +1022,6 @@ describe NotificationService, :mailer do should_not_email(@u_lazy_participant) end - it 'emails new assignee' do - issue.assignees = [@u_mentioned] - notification.reassigned_issue(issue, @u_disabled, [@u_mentioned]) - - expect(issue.assignees.first).to be @u_mentioned - should_email(issue.assignees.first) - should_email(@u_watcher) - should_email(@u_guest_watcher) - should_email(@u_guest_custom) - should_email(@u_participant_mentioned) - should_email(@subscriber) - should_email(@u_custom_global) - should_not_email(@unsubscriber) - should_not_email(@u_participating) - should_not_email(@u_disabled) - should_not_email(@u_lazy_participant) - end - it 'does not email new assignee if they are the current user' do issue.assignees = [@u_mentioned] notification.reassigned_issue(issue, @u_mentioned, [@u_mentioned]) diff --git a/spec/services/snippets/update_service_spec.rb b/spec/services/snippets/update_service_spec.rb index 9c88e741d51..05fb725c065 100644 --- a/spec/services/snippets/update_service_spec.rb +++ b/spec/services/snippets/update_service_spec.rb @@ -139,18 +139,80 @@ describe Snippets::UpdateService do subject end end + end - it 'returns error when the commit action fails' do - error_message = 'foobar' + shared_examples 'commit operation fails' do + let_it_be(:gitlab_shell) { Gitlab::Shell.new } - allow_next_instance_of(SnippetRepository) do |instance| - allow(instance).to receive(:multi_files_action).and_raise(SnippetRepository::CommitError, error_message) - end + before do + allow(service).to receive(:create_commit).and_raise(SnippetRepository::CommitError) + end + it 'returns error' do response = subject expect(response).to be_error - expect(response.payload[:snippet].errors[:repository].to_sentence).to eq error_message + expect(response.payload[:snippet].errors[:repository].to_sentence).to eq 'Error updating the snippet' + end + + context 'when repository is empty' do + before do + allow(service).to receive(:repository_empty?).and_return(true) + end + + it 'destroys the created repository in disk' do + subject + + expect(gitlab_shell.repository_exists?(snippet.repository.storage, "#{snippet.disk_path}.git")).to be_falsey + end + + it 'destroys the SnippetRepository object' do + subject + + expect(snippet.reload.snippet_repository).to be_nil + end + + it 'expires the repository exists method cache' do + response = subject + + expect(response).to be_error + expect(response.payload[:snippet].repository_exists?).to be_falsey + end + end + + context 'when repository is not empty' do + before do + allow(service).to receive(:repository_empty?).and_return(false) + end + + it 'does not destroy the repository' do + subject + + expect(gitlab_shell.repository_exists?(snippet.repository.storage, "#{snippet.disk_path}.git")).to be_truthy + end + + it 'does not destroy the snippet repository' do + subject + + expect(snippet.reload.snippet_repository).not_to be_nil + end + + it 'expires the repository exists method cache' do + response = subject + + expect(response).to be_error + expect(response.payload[:snippet].repository_exists?).to be_truthy + end + end + + it 'rolls back any snippet modifications' do + option_keys = options.stringify_keys.keys + orig_attrs = snippet.attributes.select { |k, v| k.in?(option_keys) } + + subject + + current_attrs = snippet.attributes.select { |k, v| k.in?(option_keys) } + expect(orig_attrs).to eq current_attrs end end @@ -186,12 +248,13 @@ describe Snippets::UpdateService do response = subject expect(response).to be_error - expect(response.payload[:snippet].errors[:repository].to_sentence).to eq error_message + expect(response.payload[:snippet].errors[:repository].to_sentence).to eq 'Error updating the snippet' end end it 'returns error if snippet does not have a snippet_repository' do allow(snippet).to receive(:snippet_repository).and_return(nil) + allow(snippet).to receive(:track_snippet_repository).and_return(nil) expect(subject).to be_error end @@ -219,11 +282,13 @@ describe Snippets::UpdateService do it_behaves_like 'public visibility level restrictions apply' it_behaves_like 'snippet update data is tracked' it_behaves_like 'updates repository content' + it_behaves_like 'commit operation fails' context 'when snippet does not have a repository' do let!(:snippet) { create(:project_snippet, author: user, project: project) } it_behaves_like 'creates repository and creates file' + it_behaves_like 'commit operation fails' end end @@ -235,11 +300,13 @@ describe Snippets::UpdateService do it_behaves_like 'public visibility level restrictions apply' it_behaves_like 'snippet update data is tracked' it_behaves_like 'updates repository content' + it_behaves_like 'commit operation fails' context 'when snippet does not have a repository' do let!(:snippet) { create(:personal_snippet, author: user, project: project) } it_behaves_like 'creates repository and creates file' + it_behaves_like 'commit operation fails' 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 368ec0694fd..41de499f590 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 @@ -1,6 +1,11 @@ # frozen_string_literal: true RSpec.shared_examples 'model with repository' do + let(:container) { raise NotImplementedError } + let(:stubbed_container) { raise NotImplementedError } + let(:expected_full_path) { raise NotImplementedError } + let(:expected_web_url_path) { expected_full_path } + describe '#commits_by' do let(:commits) { container.repository.commits('HEAD', limit: 3).commits } let(:commit_shas) { commits.map(&:id) } @@ -46,74 +51,33 @@ RSpec.shared_examples 'model with repository' do end end - describe '#ssh_url_to_repo' do - it 'returns container ssh address' do - expect(container.ssh_url_to_repo).to eq container.url_to_repo + describe '#url_to_repo' do + it 'returns the SSH URL to the repository' do + expect(container.url_to_repo).to eq("#{Gitlab.config.gitlab_shell.ssh_path_prefix}#{expected_web_url_path}.git") end end - describe '#http_url_to_repo' do - subject { container.http_url_to_repo } - - context 'when a custom HTTP clone URL root is not set' do - it 'returns the url to the repo without a username' do - expect(subject).to eq("#{container.web_url}.git") - expect(subject).not_to include('@') - end + describe '#ssh_url_to_repo' do + it 'returns the SSH URL to the repository' do + expect(container.ssh_url_to_repo).to eq(container.url_to_repo) end + end - context 'when a custom HTTP clone URL root is set' do - before do - stub_application_setting(custom_http_clone_url_root: custom_http_clone_url_root) - end - - context 'when custom HTTP clone URL root has a relative URL root' do - context 'when custom HTTP clone URL root ends with a slash' 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_web_url_path}.git") - end - end - - context 'when custom HTTP clone URL root does not end with a slash' 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_web_url_path}.git") - end - end - end - - context 'when custom HTTP clone URL root does not have a relative URL root' do - context 'when custom HTTP clone URL root ends with a slash' 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_web_url_path}.git") - end - end - - context 'when custom HTTP clone URL root does not end with a slash' 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_web_url_path}.git") - end - end - end + describe '#http_url_to_repo' do + it 'returns the HTTP URL to the repository' do + expect(container.http_url_to_repo).to eq("#{Gitlab.config.gitlab.url}/#{expected_web_url_path}.git") end end describe '#repository' do it 'returns valid repo' do - expect(container.repository).to be_kind_of(expected_repository_klass) + expect(container.repository).to be_kind_of(Repository) end end describe '#storage' do it 'returns valid storage' do - expect(container.storage).to be_kind_of(expected_storage_klass) + expect(container.storage).to be_kind_of(Storage::Hashed) end end |