Merge branch 'ce-to-ee-2018-09-21' into 'master'

CE upstream - 2018-09-21 21:21 UTC

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

app/assets/stylesheets/bootstrap_migration.scss

@@ -93,7 +93,6 @@ hr {
 }
 
 .form-group.row .col-form-label {
-  padding-top: 0;
   // Bootstrap 4 aligns labels to the left
   // for horizontal forms
   @include media-breakpoint-up(md) {

app/helpers/sorting_helper.rb

@@ -37,7 +37,8 @@ module SortingHelper
       sort_value_name             => sort_title_name,
       sort_value_oldest_activity  => sort_title_oldest_activity,
       sort_value_oldest_created   => sort_title_oldest_created,
-      sort_value_recently_created => sort_title_recently_created
+      sort_value_recently_created => sort_title_recently_created,
+      sort_value_most_stars       => sort_title_most_stars
     }
 
     if current_controller?('admin/projects')
@@ -248,6 +249,10 @@ module SortingHelper
     s_('SortOptions|Last Contact')
   end
 
+  def sort_title_most_stars
+    s_('SortOptions|Most stars')
+  end
+
   # Values.
   def sort_value_access_level_asc
     'access_level_asc'
@@ -372,4 +377,8 @@ module SortingHelper
   def sort_value_contacted_date
     'contacted_asc'
   end
+
+  def sort_value_most_stars
+    'stars_desc'
+  end
 end

app/models/project.rb

@@ -336,7 +336,7 @@ class Project < ActiveRecord::Base
 
   # last_activity_at is throttled every minute, but last_repository_updated_at is updated with every push
   scope :sorted_by_activity, -> { reorder("GREATEST(COALESCE(last_activity_at, '1970-01-01'), COALESCE(last_repository_updated_at, '1970-01-01')) DESC") }
-  scope :sorted_by_stars, -> { reorder('projects.star_count DESC') }
+  scope :sorted_by_stars, -> { reorder(star_count: :desc) }
 
   scope :in_namespace, ->(namespace_ids) { where(namespace_id: namespace_ids) }
   scope :personal, ->(user) { where(namespace_id: user.namespace_id) }
@@ -486,6 +486,8 @@ class Project < ActiveRecord::Base
         reorder(last_activity_at: :desc)
       when 'latest_activity_asc'
         reorder(last_activity_at: :asc)
+      when 'stars_desc'
+        sorted_by_stars
       else
         order_by(method)
       end

app/services/projects/update_remote_mirror_service.rb

@@ -12,7 +12,6 @@ module Projects
       begin
         remote_mirror.ensure_remote!
         repository.fetch_remote(remote_mirror.remote_name, no_tags: true)
-        project.update_root_ref(remote_mirror.remote_name)
 
         opts = {}
         if remote_mirror.only_protected_branches?

app/views/admin/appearances/_form.html.haml

@@ -5,7 +5,7 @@
     %legend
       Navigation bar:
   .form-group.row
-    = f.label :header_logo, 'Header logo', class: 'col-sm-2 col-form-label'
+    = f.label :header_logo, 'Header logo', class: 'col-sm-2 col-form-label pt-0'
     .col-sm-10
       - if @appearance.header_logo?
         = image_tag @appearance.header_logo_url, class: 'appearance-light-logo-preview'
@@ -22,7 +22,7 @@
     %legend
       Favicon:
   .form-group.row
-    = f.label :favicon, 'Favicon', class: 'col-sm-2 col-form-label'
+    = f.label :favicon, 'Favicon', class: 'col-sm-2 col-form-label pt-0'
     .col-sm-10
       - if @appearance.favicon?
         = image_tag @appearance.favicon_url, class: 'appearance-light-logo-preview'
@@ -53,7 +53,7 @@
       .hint
         Description parsed with #{link_to "GitLab Flavored Markdown", help_page_path('user/markdown'), target: '_blank'}.
   .form-group.row
-    = f.label :logo, class: 'col-sm-2 col-form-label'
+    = f.label :logo, class: 'col-sm-2 col-form-label pt-0'
     .col-sm-10
       - if @appearance.logo?
         = image_tag @appearance.logo_url, class: 'appearance-logo-preview'

app/views/admin/applications/_form.html.haml

@@ -21,14 +21,14 @@
           for local tests
 
   = content_tag :div, class: 'form-group row' do
-    = f.label :trusted, class: 'col-sm-2 col-form-label'
+    = f.label :trusted, class: 'col-sm-2 col-form-label pt-0'
     .col-sm-10
       = f.check_box :trusted
       %span.form-text.text-muted
         Trusted applications are automatically authorized on GitLab OAuth flow.
 
   .form-group.row
-    = f.label :scopes, class: 'col-sm-2 col-form-label'
+    = f.label :scopes, class: 'col-sm-2 col-form-label pt-0'
     .col-sm-10
       = render 'shared/tokens/scopes_form', prefix: 'doorkeeper_application', token: application, scopes: @scopes
 

app/views/groups/_group_admin_settings.html.haml

 .form-group.row
-  = f.label :lfs_enabled, 'Large File Storage', class: 'col-form-label col-sm-2'
+  = f.label :lfs_enabled, 'Large File Storage', class: 'col-form-label col-sm-2 pt-0'
   .col-sm-10
     .form-check
       = f.check_box :lfs_enabled, checked: @group.lfs_enabled?, class: 'form-check-input'
@@ -13,7 +13,7 @@
 = render partial: 'groups/ee/project_creation_level', locals: { form: f, group: @group }
 
 .form-group.row
-  = f.label :require_two_factor_authentication, 'Two-factor authentication', class: 'col-form-label col-sm-2'
+  = f.label :require_two_factor_authentication, 'Two-factor authentication', class: 'col-form-label col-sm-2 pt-0'
   .col-sm-10
     .form-check
       = f.check_box :require_two_factor_authentication, class: 'form-check-input'

app/views/groups/settings/_permissions.html.haml

@@ -10,7 +10,7 @@
         = render 'shared/allow_request_access', form: f
 
     .form-group.row
-      %label.col-form-label.col-sm-2
+      %label.col-form-label.col-sm-2.pt-0
         = s_('GroupSettings|Share with group lock')
       .col-sm-10
         .form-check

app/views/shared/_visibility_level.html.haml

@@ -2,7 +2,7 @@
 
 .form-group.row.visibility-level-setting
   - if with_label
-    = f.label :visibility_level, class: 'col-form-label col-sm-2' do
+    = f.label :visibility_level, class: 'col-form-label col-sm-2 pt-0' do
       Visibility Level
       = link_to icon('question-circle'), help_page_path("public_access/public_access")
   %div{ :class => (with_label ? "col-sm-10" : "col-sm-12") }

changelogs/unreleased/51725-push-mirrors-default-branch-reset-to-master.yml

+---
+title: Doesn't synchronize the default branch for push mirrors
+merge_request: 21861
+author:
+type: fixed

changelogs/unreleased/add-most-stars-for-filter-option.yml

+---
+title: Allows to sort projects by most stars
+merge_request: 21762
+author: Jacopo Beschi @jacopo-beschi
+type: added

changelogs/unreleased/align-form-labels.yml

+---
+title: Align form labels following Bootstrap 4 docs
+merge_request: 21752
+author:
+type: fixed

changelogs/unreleased/fix-leading-slash-in-redirects-plus-rubocop.yml

+---
+title: Fix leading slash in redirects and add rubocop cop
+merge_request: 21828
+author: Sanad Liaquat
+type: fixed

config/routes/instance_statistics.rb

 # frozen_string_literal: true
 
 namespace :instance_statistics do
-  root to: redirect('/-/instance_statistics/conversational_development_index')
+  root to: redirect('-/instance_statistics/conversational_development_index')
 
   resources :cohorts, only: :index
   resources :conversational_development_index, only: :index

config/routes/wiki.rb

@@ -2,7 +2,7 @@ scope(controller: :wikis) do
   scope(path: 'wikis', as: :wikis) do
     get :git_access
     get :pages
-    get '/', to: redirect('/%{namespace_id}/%{project_id}/wikis/home')
+    get '/', to: redirect('%{namespace_id}/%{project_id}/wikis/home')
     post '/', to: 'wikis#create'
   end
 

doc/api/settings.md

@@ -224,6 +224,7 @@ are listed in the descriptions of the relevant settings.
 | `mirror_capacity_threshold`              | integer          | no                                   | **(Premium)** Minimum capacity to be available before scheduling more mirrors preemptively |
 | `mirror_max_capacity`                    | integer          | no                                   | **(Premium)** Maximum number of mirrors that can be synchronizing at the same time. |
 | `mirror_max_delay`                       | integer          | no                                   | **(Premium)** Maximum time (in minutes) between updates that a mirror can have when scheduled to synchronize. |
+=======
 | `pages_domain_verification_enabled`      | boolean          | no                                   | Require users to prove ownership of custom domains. Domain verification is an essential security measure for public GitLab sites. Users are required to demonstrate they control a domain before it is enabled. |
 | `password_authentication_enabled_for_git` | boolean         | no                                   | Enable authentication for Git over HTTP(S) via a GitLab account password. Default is `true`. |
 | `password_authentication_enabled_for_web` | boolean         | no                                   | Enable authentication for the web interface via a GitLab account password. Default is `true`. |

doc/user/profile/account/delete_account.md

 # Deleting a User Account
 
+NOTE: **Note:**
+Deleting a user will delete all projects in that user namespace.
+
 - As a user, you can delete your own account by navigating to **Settings** > **Account** and selecting **Delete account**
 - As an admin, you can delete a user account by navigating to the **Admin Area**, selecting the **Users** tab, selecting a user, and clicking on **Delete user**
 

doc/user/project/cycle_analytics.md

@@ -154,7 +154,7 @@ You can [read more about permissions][permissions] in general.
 
 Learn more about Cycle Analytics in the following resources:
 
-- [Cycle Analytics feature page](https://about.gitlab.com/solutions/cycle-analytics/)
+- [Cycle Analytics feature page](https://about.gitlab.com/features/cycle-analytics/)
 - [Cycle Analytics feature preview](https://about.gitlab.com/2016/09/16/feature-preview-introducing-cycle-analytics/)
 - [Cycle Analytics feature highlight](https://about.gitlab.com/2016/09/21/cycle-analytics-feature-highlight/)
 

lib/gitlab/gitaly_client.rb

@@ -174,10 +174,29 @@ module Gitlab
     end
     private_class_method :current_transaction_labels
 
+    # For some time related tasks we can't rely on `Time.now` since it will be
+    # affected by Timecop in some tests, and the clock of some gitaly-related
+    # components (grpc's c-core and gitaly server) use system time instead of
+    # timecop's time, so tests will fail.
+    # `Time.at(Process.clock_gettime(Process::CLOCK_REALTIME))` will circumvent
+    # timecop.
+    def self.real_time
+      Time.at(Process.clock_gettime(Process::CLOCK_REALTIME))
+    end
+    private_class_method :real_time
+
+    def self.authorization_token(storage)
+      token = token(storage).to_s
+      issued_at = real_time.to_i.to_s
+      hmac = OpenSSL::HMAC.hexdigest(OpenSSL::Digest::SHA256.new, token, issued_at)
+
+      "v2.#{hmac}.#{issued_at}"
+    end
+    private_class_method :authorization_token
+
     def self.request_kwargs(storage, timeout, remote_storage: nil)
-      encoded_token = Base64.strict_encode64(token(storage).to_s)
       metadata = {
-        'authorization' => "Bearer #{encoded_token}",
+        'authorization' => "Bearer #{authorization_token(storage)}",
         'client_name' => CLIENT_NAME
       }
 
@@ -195,12 +214,7 @@ module Gitlab
 
       return result unless timeout > 0
 
-      # Do not use `Time.now` for deadline calculation, since it
-      # will be affected by Timecop in some tests, but grpc's c-core
-      # uses system time instead of timecop's time, so tests will fail
-      # `Time.at(Process.clock_gettime(Process::CLOCK_REALTIME))` will
-      # circumvent timecop
-      deadline = Time.at(Process.clock_gettime(Process::CLOCK_REALTIME)) + timeout
+      deadline = real_time + timeout
       result[:deadline] = deadline
 
       result

locale/gitlab.pot

@@ -6997,6 +6997,9 @@ msgstr ""
 msgid "SortOptions|Most popular"
 msgstr ""
 
+msgid "SortOptions|Most stars"
+msgstr ""
+
 msgid "SortOptions|Name"
 msgstr ""
 

rubocop/cop/avoid_route_redirect_leading_slash.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    # Checks for a leading '/' in route redirects
+    # For more information see: https://gitlab.com/gitlab-org/gitlab-ce/issues/50645
+    #
+    # @example
+    #   # bad
+    #   root to: redirect('/-/instance/statistics/conversational_development_index')
+    #
+    #   # good
+    #   root to: redirect('-/instance/statistics/conversational_development_index')
+    #
+
+    class AvoidRouteRedirectLeadingSlash < RuboCop::Cop::Cop
+      MSG = 'Do not use a leading "/" in route redirects'
+
+      def_node_matcher :leading_slash_in_redirect?, <<~PATTERN
+        (send nil? :redirect (str #has_leading_slash?))
+      PATTERN
+
+      def on_send(node)
+        return unless in_routes?(node)
+        return unless leading_slash_in_redirect?(node)
+
+        add_offense(node)
+      end
+
+      def has_leading_slash?(str)
+        str.start_with?("/")
+      end
+
+      def in_routes?(node)
+        path = node.location.expression.source_buffer.name
+        dirname = File.dirname(path)
+        filename = File.basename(path)
+        dirname.end_with?('config/routes') || filename.end_with?('routes.rb')
+      end
+
+      def autocorrect(node)
+        lambda do |corrector|
+          corrector.replace(node.loc.expression, remove_leading_slash(node))
+        end
+      end
+
+      def remove_leading_slash(node)
+        node.source.sub('/', '')
+      end
+    end
+  end
+end

rubocop/rubocop.rb

@@ -7,6 +7,7 @@ require_relative 'cop/gitlab/union'
 require_relative 'cop/include_sidekiq_worker'
 require_relative 'cop/avoid_return_from_blocks'
 require_relative 'cop/avoid_break_from_strong_memoize'
+require_relative 'cop/avoid_route_redirect_leading_slash'
 require_relative 'cop/line_break_around_conditional_block'
 require_relative 'cop/prefer_class_methods_over_module'
 require_relative 'cop/migration/add_column'

spec/features/dashboard/projects_spec.rb

@@ -103,6 +103,14 @@ describe 'Dashboard Projects' do
       expect(page).not_to have_content(project.name)
       expect(page).to have_content(project3.name)
     end
+
+    it 'sorts projects by most stars when sorting by most stars' do
+      project_with_most_stars = create(:project, namespace: user.namespace, star_count: 10)
+
+      visit dashboard_projects_path(sort: :stars_desc)
+
+      expect(first('.project-row')).to have_content(project_with_most_stars.title)
+    end
   end
 
   context 'when on Starred projects tab' do

spec/models/project_spec.rb

@@ -1201,6 +1201,18 @@ describe Project do
     it { expect(project.builds_enabled?).to be_truthy }
   end
 
+  describe '.sort_by_attribute' do
+    it 'reorders the input relation by start count desc' do
+      project1 = create(:project, star_count: 2)
+      project2 = create(:project, star_count: 1)
+      project3 = create(:project)
+
+      projects = described_class.sort_by_attribute(:stars_desc)
+
+      expect(projects).to eq([project1, project2, project3])
+    end
+  end
+
   describe '.with_shared_runners' do
     subject { described_class.with_shared_runners }
 

spec/rubocop/cop/avoid_route_redirect_leading_slash_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+require 'rubocop'
+require_relative '../../../rubocop/cop/avoid_route_redirect_leading_slash'
+
+describe RuboCop::Cop::AvoidRouteRedirectLeadingSlash do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  before do
+    allow(cop).to receive(:in_routes?).and_return(true)
+  end
+
+  it 'registers an offense when redirect has a leading slash' do
+    expect_offense(<<~PATTERN.strip_indent)
+      root to: redirect("/-/route")
+               ^^^^^^^^^^^^^^^^^^^^ Do not use a leading "/" in route redirects
+    PATTERN
+  end
+
+  it 'does not register an offense when redirect does not have a leading slash' do
+    expect_no_offenses(<<~PATTERN.strip_indent)
+      root to: redirect("-/route")
+    PATTERN
+  end
+
+  it 'autocorrect `/-/route` to `-/route`' do
+    expect(autocorrect_source('redirect("/-/route")')).to eq('redirect("-/route")')
+  end
+end

spec/services/projects/update_remote_mirror_service_spec.rb

@@ -17,7 +17,6 @@ describe Projects::UpdateRemoteMirrorService do
 
     it "ensures the remote exists" do
       stub_fetch_remote(project, remote_name: remote_name)
-      stub_find_remote_root_ref(project, remote_name: remote_name)
 
       expect(remote_mirror).to receive(:ensure_remote!)
 
@@ -25,8 +24,6 @@ describe Projects::UpdateRemoteMirrorService do
     end
 
     it "fetches the remote repository" do
-      stub_find_remote_root_ref(project, remote_name: remote_name)
-
       expect(project.repository)
         .to receive(:fetch_remote)
         .with(remote_mirror.remote_name, no_tags: true)
@@ -34,26 +31,8 @@ describe Projects::UpdateRemoteMirrorService do
       service.execute(remote_mirror)
     end
 
-    it "updates the default branch when HEAD has changed" do
-      stub_fetch_remote(project, remote_name: remote_name)
-      stub_find_remote_root_ref(project, remote_name: remote_name, ref: "existing-branch")
-
-      expect { service.execute(remote_mirror) }
-        .to change { project.default_branch }
-        .from("master")
-        .to("existing-branch")
-    end
-
-    it "does not update the default branch when HEAD does not change" do
-      stub_fetch_remote(project, remote_name: remote_name)
-      stub_find_remote_root_ref(project, remote_name: remote_name, ref: "master")
-
-      expect { service.execute(remote_mirror) }.not_to change { project.default_branch }
-    end
-
     it "returns success when updated succeeds" do
       stub_fetch_remote(project, remote_name: remote_name)
-      stub_find_remote_root_ref(project, remote_name: remote_name)
 
       result = service.execute(remote_mirror)
 
@@ -63,7 +42,6 @@ describe Projects::UpdateRemoteMirrorService do
     context 'when syncing all branches' do
       it "push all the branches the first time" do
         stub_fetch_remote(project, remote_name: remote_name)
-        stub_find_remote_root_ref(project, remote_name: remote_name)
 
         expect(remote_mirror).to receive(:update_repository).with({})
 
@@ -74,7 +52,6 @@ describe Projects::UpdateRemoteMirrorService do
     context 'when only syncing protected branches' do
       it "sync updated protected branches" do
         stub_fetch_remote(project, remote_name: remote_name)
-        stub_find_remote_root_ref(project, remote_name: remote_name)
         protected_branch = create_protected_branch(project)
         remote_mirror.only_protected_branches = true
 
@@ -92,13 +69,6 @@ describe Projects::UpdateRemoteMirrorService do
     end
   end
 
-  def stub_find_remote_root_ref(project, ref: 'master', remote_name:)
-    allow(project.repository)
-      .to receive(:find_remote_root_ref)
-      .with(remote_name)
-      .and_return(ref)
-  end
-
   def stub_fetch_remote(project, remote_name:)
     allow(project.repository)
       .to receive(:fetch_remote)