Add latest changes from gitlab-org/gitlab@master

.gitlab/issue_templates/Migrations.md

+# 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

.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'

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]

app/controllers/projects/static_site_editor_controller.rb

+# frozen_string_literal: true
+
+class Projects::StaticSiteEditorController < Projects::ApplicationController
+  layout 'fullscreen'
+
+  prepend_before_action :authenticate_user!, only: [:show]
+
+  def show
+  end
+end

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

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

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

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

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

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

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

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

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

app/services/snippets/update_service.rb

@@ -35,9 +35,7 @@ module Snippets
     private
 
     def save_and_commit(snippet)
-      snippet.with_transaction_returning_status do
-        snippet.save.tap do |saved|
-          break false unless saved
+      return false unless snippet.save
 
       # In order to avoid non migrated snippets scenarios,
       # if the snippet does not have a repository we created it
@@ -50,13 +48,30 @@ module Snippets
       # If the snippet repository exists we commit always
       # the changes
       create_commit(snippet) if snippet.repository_exists?
-        end
+
+      true
     rescue => e
-        snippet.errors.add(:repository, e.message)
+      # Restore old attributes
+      unless snippet.previous_changes.empty?
+        snippet.previous_changes.each { |attr, value| snippet[attr] = value[0] }
+        snippet.save
+      end
+
+      snippet.errors.add(:repository, 'Error updating the snippet')
       log_error(e.message)
 
-        false
+      # 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

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

+#static-site-editor{ data: {} }

changelogs/unreleased/fj-213436-move-update-outside-transaction.yml

+---
+title: Fix race condition updating snippet without repository
+merge_request: 28851
+author:
+type: fixed

changelogs/unreleased/notification-service-spec.yml

+---
+title: Update duplicate specs in notification service spec
+merge_request: 28742
+author: Rajendra Kadam
+type: fixed

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

danger/telemetry/Dangerfile

 # 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})

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

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

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.

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.

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
 

lib/gitlab/path_regex.rb

@@ -98,6 +98,7 @@ module Gitlab
       preview
       raw
       refs
+      sse
       tree
       update
       wikis

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
+      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
+          commit_url(object, **options)
+        when Group
+          instance.group_canonical_url(object, **options)
         when Issue
-        issue_url(object)
+          instance.issue_url(object, **options)
         when MergeRequest
-        merge_request_url(object)
+          instance.merge_request_url(object, **options)
+        when Milestone
+          instance.milestone_url(object, **options)
         when Note
-        note_url
-      when WikiPage
-        wiki_page_url
+          note_url(object, **options)
+        when Project
+          instance.project_url(object, **options)
         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)
+          snippet_url(object, **options)
         when User
-        user_url(object)
+          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
 
-    private
+      def commit_url(commit, **options)
+        return '' unless commit.project
 
-    def initialize(object, opts = {})
-      @object = object
-      @opts = opts
+        instance.commit_url(commit, **options)
       end
 
-    def commit_url(opts = {})
-      return '' if object.project.nil?
+      def note_url(note, **options)
+        if note.for_commit?
+          return '' unless note.project
 
-      namespace_project_commit_url({
-        namespace_id: object.project.namespace,
-        project_id: object.project,
-        id: object.id
-      }.merge!(opts))
+          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
 
-    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))
-
-      elsif object.for_merge_request?
-        merge_request_url(object.noteable, anchor: dom_id(object))
-
-      elsif object.for_snippet?
-        gitlab_snippet_url(object.noteable, anchor: dom_id(object))
+      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
-
-    def wiki_page_url
-      project_wiki_url(object.wiki.project, object.slug)
     end
   end
 end

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

spec/controllers/projects/static_site_editor_controller_spec.rb

+# 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

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

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

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

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]));
   },
 };

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

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

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

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
 

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])

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)
+      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

spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb

 # 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
-    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
-    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
+  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
 
-      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")
+  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 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