Merge branch 'master' into 8-11-stable-ee

CHANGELOG-EE

 Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.11.0 (unreleased)
+  - Allow projects to be moved between repository storages
+  - Add rake task to remove old repository copies from repositories moved to another storage
   - Performance improvement of push rules
   - Change LdapGroupSyncWorker to use new LDAP group sync classes
   - [Elastic][Fix] Commit search breaks for some URLs on gitlab-ce project
+  - Removed unused GitLab GEO database index
+  - Enable monitoring for ES classes
+  - [Elastic] Improve code search
+  - [Elastic] Significant improvement of global search performance
+  - [Fix] Push rules check existing commits in some cases
 
-v 8.10.5 (unreleased)
+v 8.10.5
+  - Used cached value of project count in `Elastic::RepositoriesSearch` to reduce DB load. !637
 
 v 8.10.4
   - Fix available users in userselect dropdown when there is more than one userselect on the page. !604 (Rik de Groot)
@@ -28,6 +36,7 @@ v 8.10.0
   - Rename Git Hooks to Push Rules
   - Fix EE keys fingerprint add index migration if came from CE
   - Add todos for MR approvers !547
+  - Replace LDAP group sync exclusive lease with state machine
   - Prevent the author of an MR from being on the approvers list
   - Isolate EE LDAP library code in EE module (Part 1) !511
   - Make Elasticsearch indexer run as an async task

GITLAB_SHELL_VERSION

-3.2.1
+3.3.0

Gemfile

@@ -111,7 +111,7 @@ gem 'seed-fu', '~> 2.3.5'
 # Search
 gem 'elasticsearch-model'
 gem 'elasticsearch-rails'
-gem 'gitlab-elasticsearch-git', '~> 0.0.15', require: "elasticsearch/git"
+gem 'gitlab-elasticsearch-git', '~> 0.0.16', require: "elasticsearch/git"
 
 # Markdown and HTML processing
 gem 'html-pipeline', '~> 1.11.0'

Gemfile.lock

@@ -282,7 +282,7 @@ GEM
       mime-types (>= 1.19)
       rugged (>= 0.23.0b)
     github-markup (1.4.0)
-    gitlab-elasticsearch-git (0.0.15)
+    gitlab-elasticsearch-git (0.0.16)
       activemodel (~> 4.2)
       activesupport (~> 4.2)
       charlock_holmes (~> 0.7)
@@ -898,7 +898,7 @@ DEPENDENCIES
   gemojione (~> 3.0)
   github-linguist (~> 4.7.0)
   github-markup (~> 1.4)
-  gitlab-elasticsearch-git (~> 0.0.15)
+  gitlab-elasticsearch-git (~> 0.0.16)
   gitlab-flowdock-git-hook (~> 1.0.1)
   gitlab-license (~> 1.0)
   gitlab_git (~> 10.4.5)

app/models/ability.rb

@@ -184,6 +184,8 @@ class Ability
         # Push abilities on the users team role
         rules.push(*project_team_rules(project.team, user))
 
+        rules << :change_repository_storage if user.admin?
+
         owner = user.admin? ||
                 project.owner == user ||
                 (project.group && project.group.has_owner?(user))

app/models/concerns/elastic/application_search.rb

@@ -53,6 +53,10 @@ module Elastic
       def searchable?
         true
       end
+
+      def es_parent
+        project_id
+      end
     end
 
     module ClassMethods
@@ -65,6 +69,16 @@ module Elastic
         { fields: es_fields }
       end
 
+      def import_with_parent(options = {})
+        transform = lambda do |r|
+          { index: { _id: r.id, _parent: r.es_parent, data: r.__elasticsearch__.as_indexed_json } }
+        end
+
+        options.merge!(transform: transform)
+
+        self.import(options)
+      end
+
       def basic_query_hash(fields, query)
         query_hash = if query.present?
                        {
@@ -111,17 +125,42 @@ module Elastic
         }
       end
 
-      def project_ids_filter(query_hash, project_ids)
+      def project_ids_filter(query_hash, project_ids, public_and_internal_projects = true)
         if project_ids
+          condition = project_ids_condition(project_ids, public_and_internal_projects)
+
           query_hash[:query][:bool][:filter] = {
-            bool: {
-              must: [ { terms: { project_id: project_ids } } ]
+            has_parent: {
+              parent_type: "project",
+              query: {
+                bool: {
+                  should: condition
+                }
+              }
             }
           }
         end
 
         query_hash
       end
+
+      def project_ids_condition(project_ids, public_and_internal_projects)
+        conditions = [{
+          terms: { id: project_ids }
+        }]
+
+        if public_and_internal_projects
+          conditions << {
+            term: { visibility_level: Project::PUBLIC }
+          }
+
+          conditions << {
+            term: { visibility_level: Project::INTERNAL }
+          }
+        end
+
+        conditions
+      end
     end
   end
 end

app/models/concerns/elastic/issues_search.rb

@@ -5,9 +5,8 @@ module Elastic
     included do
       include ApplicationSearch
 
-      mappings do
+      mappings _parent: { type: 'project' } do
         indexes :id,          type: :integer
-
         indexes :iid,         type: :integer, index: :not_analyzed
         indexes :title,       type: :string,
                               index_options: 'offsets'
@@ -16,11 +15,9 @@ module Elastic
         indexes :created_at,  type: :date
         indexes :updated_at,  type: :date
         indexes :state,       type: :string
-
         indexes :project_id,  type: :integer
         indexes :author_id,   type: :integer
         indexes :assignee_id, type: :integer
-
         indexes :confidential, type: :boolean
       end
 
@@ -43,7 +40,7 @@ module Elastic
           query_hash = basic_query_hash(%w(title^2 description), query)
         end
 
-        query_hash = project_ids_filter(query_hash, options[:project_ids])
+        query_hash = project_ids_filter(query_hash, options[:project_ids], options[:public_and_internal_projects])
         query_hash = confidentiality_filter(query_hash, options[:current_user])
 
         self.__elasticsearch__.search(query_hash)

app/models/concerns/elastic/merge_requests_search.rb

@@ -5,23 +5,21 @@ module Elastic
     included do
       include ApplicationSearch
 
-      mappings do
-        indexes :id,            type: :integer
-
-        indexes :iid,           type: :integer
-        indexes :target_branch, type: :string,
-                                index_options: 'offsets'
-        indexes :source_branch, type: :string,
-                                index_options: 'offsets'
-        indexes :title,         type: :string,
-                                index_options: 'offsets'
-        indexes :description,   type: :string,
-                                index_options: 'offsets'
-        indexes :created_at,    type: :date
-        indexes :updated_at,    type: :date
-        indexes :state,         type: :string
-        indexes :merge_status,  type: :string
-
+      mappings _parent: { type: 'project' } do
+        indexes :id,                type: :integer
+        indexes :iid,               type: :integer
+        indexes :target_branch,     type: :string,
+                                    index_options: 'offsets'
+        indexes :source_branch,     type: :string,
+                                    index_options: 'offsets'
+        indexes :title,             type: :string,
+                                    index_options: 'offsets'
+        indexes :description,       type: :string,
+                                    index_options: 'offsets'
+        indexes :created_at,        type: :date
+        indexes :updated_at,        type: :date
+        indexes :state,             type: :string
+        indexes :merge_status,      type: :string
         indexes :source_project_id, type: :integer
         indexes :target_project_id, type: :integer
         indexes :author_id,         type: :integer
@@ -49,10 +47,14 @@ module Elastic
         ].each do |attr|
           data[attr.to_s] = self.send(attr)
         end
-        
+
         data
       end
 
+      def es_parent
+        target_project_id
+      end
+
       def self.elastic_search(query, options: {})
         if query =~ /#(\d+)\z/
           query_hash = iid_query_hash(query_hash, $1)
@@ -60,13 +62,7 @@ module Elastic
           query_hash = basic_query_hash(%w(title^2 description), query)
         end
 
-        if options[:project_ids]
-          query_hash[:query][:bool][:filter] = [{
-            terms: {
-              target_project_id: [options[:project_ids]].flatten
-            }
-          }]
-        end
+        query_hash = project_ids_filter(query_hash, options[:project_ids], options[:public_and_internal_projects])
 
         self.__elasticsearch__.search(query_hash)
       end

app/models/concerns/elastic/milestones_search.rb

@@ -5,7 +5,7 @@ module Elastic
     included do
       include ApplicationSearch
 
-      mappings do
+      mappings _parent: { type: 'project' } do
         indexes :id,          type: :integer
         indexes :title,       type: :string,
                               index_options: 'offsets'
@@ -27,7 +27,7 @@ module Elastic
 
         query_hash = basic_query_hash(options[:in], query)
 
-        query_hash = project_ids_filter(query_hash, options[:project_ids])
+        query_hash = project_ids_filter(query_hash, options[:project_ids], options[:public_and_internal_projects])
 
         self.__elasticsearch__.search(query_hash)
       end

app/models/concerns/elastic/notes_search.rb

@@ -5,7 +5,7 @@ module Elastic
     included do
       include ApplicationSearch
 
-      mappings do
+      mappings _parent: { type: 'project' } do
         indexes :id,          type: :integer
         indexes :note,        type: :string,
                               index_options: 'offsets'
@@ -56,7 +56,7 @@ module Elastic
           query_hash[:track_scores] = true
         end
 
-        query_hash = project_ids_filter(query_hash, options[:project_ids])
+        query_hash = project_ids_filter(query_hash, options[:project_ids], options[:public_and_internal_projects])
         query_hash = confidentiality_filter(query_hash, options[:current_user])
 
         query_hash[:sort] = [

app/models/concerns/elastic/projects_search.rb

@@ -7,7 +7,6 @@ module Elastic
 
       mappings do
         indexes :id,                  type: :integer
-
         indexes :name,                type: :string,
                                       index_options: 'offsets'
         indexes :path,                type: :string,
@@ -18,9 +17,7 @@ module Elastic
                                       index_options: 'offsets'
         indexes :description,         type: :string,
                                       index_options: 'offsets'
-
         indexes :namespace_id,        type: :integer
-
         indexes :created_at,          type: :date
         indexes :updated_at,          type: :date
         indexes :archived,            type: :boolean
@@ -87,8 +84,8 @@ module Elastic
 
         if options[:pids]
           filters << {
-            ids: {
-              values: options[:pids]
+            bool: {
+              should: project_ids_condition(options[:pids], options[:public_and_internal_projects])
             }
           }
         end

app/models/concerns/elastic/repositories_search.rb

@@ -12,7 +12,7 @@ module Elastic
       end
 
       def self.repositories_count
-        Project.count
+        Project.cached_count
       end
 
       def client_for_indexing

app/models/concerns/elastic/snippets_search.rb

@@ -7,7 +7,6 @@ module Elastic
 
       mappings do
         indexes :id,               type: :integer
-
         indexes :title,            type: :string,
                                    index_options: 'offsets'
         indexes :file_name,        type: :string,
@@ -17,7 +16,6 @@ module Elastic
         indexes :created_at,       type: :date
         indexes :updated_at,       type: :date
         indexes :state,            type: :string
-
         indexes :project_id,       type: :integer
         indexes :author_id,        type: :integer
         indexes :visibility_level, type: :integer

app/models/ee/group.rb

+# Group EE mixin
+#
+# This module is intended to encapsulate EE-specific model logic
+# and be included in the `Group` model
+module EE
+  module Group
+    extend ActiveSupport::Concern
+
+    included do
+      state_machine :ldap_sync_status, namespace: :ldap_sync, initial: :ready do
+        state :ready
+        state :started
+        state :failed
+
+        event :start do
+          transition [:ready, :failed] => :started
+        end
+
+        event :finish do
+          transition started: :ready
+        end
+
+        event :fail do
+          transition started: :failed
+        end
+
+        after_transition ready: :started do |group, _|
+          group.ldap_sync_last_sync_at = DateTime.now
+          group.save
+        end
+
+        after_transition started: :ready do |group, _|
+          current_time = DateTime.now
+          group.ldap_sync_last_update_at = current_time
+          group.ldap_sync_last_successful_update_at = current_time
+          group.ldap_sync_error = nil
+          group.save
+        end
+
+        after_transition started: :failed do |group, _|
+          group.ldap_sync_last_update_at = DateTime.now
+          group.save
+        end
+      end
+    end
+
+    def mark_ldap_sync_as_failed(error_message)
+      return false unless ldap_sync_started?
+
+      fail_ldap_sync
+      update_column(:ldap_sync_error, ::Gitlab::UrlSanitizer.sanitize(error_message))
+    end
+  end
+end

app/models/group.rb

 require 'carrierwave/orm/activerecord'
 
+# Contains methods common to both GitLab CE and EE.
+# All EE methods should be in `EE::Group` only.
 class Group < Namespace
+  include EE::Group
   include Gitlab::ConfigHelper
   include Gitlab::VisibilityLevel
   include AccessRequestable

app/models/project.rb

@@ -408,6 +408,12 @@ class Project < ActiveRecord::Base
 
       joins(join_body).reorder('join_note_counts.amount DESC')
     end
+
+    def cached_count
+      Rails.cache.fetch('total_project_count', expires_in: 5.minutes) do
+        Project.count
+      end
+    end
   end
 
   def repository_storage_path
@@ -1491,6 +1497,16 @@ class Project < ActiveRecord::Base
     end
   end
 
+  def change_repository_storage(new_repository_storage_key)
+    return if repository_read_only?
+    return if repository_storage == new_repository_storage_key
+
+    raise ArgumentError unless Gitlab.config.repositories.storages.keys.include?(new_repository_storage_key)
+
+    run_after_commit { ProjectUpdateRepositoryStorageWorker.perform_async(id, new_repository_storage_key) }
+    self.repository_read_only = true
+  end
+
   private
 
   def default_branch_protected?

app/models/repository.rb

@@ -666,6 +666,15 @@ class Repository
     commit(sha)
   end
 
+  # Returns a list of commits that are not present in any reference
+  def new_commits(newrev)
+    args = %W(#{Gitlab.config.git.bin_path} rev-list #{newrev} --not --all)
+
+    Gitlab::Popen.popen(args, path_to_repo).first.split("\n").map do |sha|
+      commit(sha.strip)
+    end
+  end
+
   def next_branch(name, opts = {})
     branch_ids = self.branch_names.map do |n|
       next 1 if n == name

app/services/projects/update_repository_storage_service.rb

+module Projects
+  class UpdateRepositoryStorageService < BaseService
+    include Gitlab::ShellAdapter
+
+    def initialize(project)
+      @project = project
+    end
+
+    def execute(new_repository_storage_key)
+      new_storage_path = Gitlab.config.repositories.storages[new_repository_storage_key]
+      result = move_storage(project.path_with_namespace, new_storage_path)
+
+      if project.wiki.repository_exists?
+        result &&= move_storage("#{project.path_with_namespace}.wiki", new_storage_path)
+      end
+
+      if result
+        mark_old_paths_for_archive
+
+        project.update(repository_storage: new_repository_storage_key, repository_read_only: false)
+      else
+        project.update(repository_read_only: false)
+      end
+    end
+
+    private
+
+    def move_storage(project_path, new_storage_path)
+      gitlab_shell.mv_storage(project.repository_storage_path, project_path, new_storage_path)
+    end
+
+    def mark_old_paths_for_archive
+      old_repository_storage_path = project.repository_storage_path
+      new_project_path = moved_path(project.path_with_namespace)
+
+      # Notice that the block passed to `run_after_commit` will run with `project`
+      # as its context
+      project.run_after_commit do
+        GitlabShellWorker.perform_async(:mv_repository,
+                                        old_repository_storage_path,
+                                        path_with_namespace,
+                                        new_project_path)
+
+        if wiki.repository_exists?
+          GitlabShellWorker.perform_async(:mv_repository,
+                                          old_repository_storage_path,
+                                          "#{path_with_namespace}.wiki",
+                                          "#{new_project_path}.wiki")
+        end
+      end
+    end
+
+    def moved_path(path)
+      "#{path}+#{project.id}+moved+#{Time.now.to_i}"
+    end
+  end
+end

app/services/projects/update_service.rb

@@ -13,13 +13,20 @@ module Projects
         end
       end
 
-      new_branch = params[:default_branch]
+      new_branch = params.delete(:default_branch)
+      new_repository_storage = params.delete(:repository_storage)
 
-      if project.repository.exists? && new_branch && new_branch != project.default_branch
-        project.change_head(new_branch)
+      if project.repository.exists?
+        if new_branch && new_branch != project.default_branch
+          project.change_head(new_branch)
+        end
+
+        if new_repository_storage && can?(current_user, :change_repository_storage, project)
+          project.change_repository_storage(new_repository_storage)
+        end
       end
 
-      if project.update_attributes(params.except(:default_branch))
+      if project.update_attributes(params)
         if project.previous_changes.include?('path')
           project.rename_repo
         end

app/services/search/global_service.rb

@@ -10,12 +10,21 @@ module Search
 
     def execute
       group = Group.find_by(id: params[:group_id]) if params[:group_id].present?
-      projects = ProjectsFinder.new.execute(current_user)
-      projects = projects.in_namespace(group.id) if group
 
       if current_application_settings.elasticsearch_search?
-        Gitlab::Elastic::SearchResults.new(current_user, projects.pluck(:id), params[:search])
+        projects = current_user ? current_user.authorized_projects : Project.none
+        projects = projects.in_namespace(group.id) if group
+
+        Gitlab::Elastic::SearchResults.new(
+          current_user,
+          params[:search],
+          projects.pluck(:id),
+          !group
+        )
       else
+        projects = ProjectsFinder.new.execute(current_user)
+        projects = projects.in_namespace(group.id) if group
+
         Gitlab::SearchResults.new(current_user, projects, params[:search])
       end
     end

app/services/search/project_service.rb

@@ -11,8 +11,8 @@ module Search
     def execute
       if current_application_settings.elasticsearch_search?
         Gitlab::Elastic::ProjectSearchResults.new(current_user,
-                                                  project.id,
                                                   params[:search],
+                                                  project.id,
                                                   params[:repository_ref])
       else
         Gitlab::ProjectSearchResults.new(current_user,

app/views/admin/dashboard/index.html.haml

@@ -131,7 +131,7 @@
           %h4 Projects
           .data
             = link_to admin_namespaces_projects_path do
-              %h1= number_with_delimiter(Project.count)
+              %h1= number_with_delimiter(Project.cached_count)
             %hr
             = link_to('New Project', new_project_path, class: "btn btn-new")
       .col-sm-4

app/views/groups/analytics/show.html.haml

 - page_title "Contribution Analytics"
 - header_title group_title(@group, "Contribution Analytics", group_analytics_path(@group))
+- content_for :page_specific_javascripts do
+  = page_specific_javascript_tag('lib/chart.js')
 
-.gray-content-block
+.sub-header-block
   .pull-right
     .dropdown.inline
       %button.dropdown-toggle.btn{type: 'button', 'data-toggle' => 'dropdown'}
         = icon('calendar-o')
         %b.caret
-      %ul.dropdown-menu
+      %ul.dropdown-menu.dropdown-menu-align-right
         %li
           = link_to group_analytics_path(@group, start_date: Date.today - 1.week) do
             Last week

app/workers/elastic_indexer_worker.rb

@@ -13,7 +13,12 @@ class ElasticIndexerWorker
     when /index|update/
       record = klass.find(record_id)
       record.__elasticsearch__.client = client
-      record.__elasticsearch__.__send__ "#{operation}_document"
+
+      if [Project, PersonalSnippet, ProjectSnippet, Snippet].include?(klass)
+        record.__elasticsearch__.__send__ "#{operation}_document"
+      else
+        record.__elasticsearch__.__send__ "#{operation}_document", parent: record.es_parent
+      end
 
       update_issue_notes(record, options["changed_fields"]) if klass == Issue
     when /delete/
@@ -27,7 +32,7 @@ class ElasticIndexerWorker
 
   def update_issue_notes(record, changed_fields)
     if changed_fields && (changed_fields & ISSUE_TRACKED_FIELDS).any?
-      Note.import query: -> { where(noteable: record) }
+      Note.import_with_parent query: -> { where(noteable: record) }
     end
   end
 

app/workers/project_update_repository_storage_worker.rb

+class ProjectUpdateRepositoryStorageWorker
+  include Sidekiq::Worker
+
+  sidekiq_options queue: :default
+
+  def perform(project_id, new_repository_storage_key)
+    project = Project.find(project_id)
+
+    ::Projects::UpdateRepositoryStorageService.new(project).execute(new_repository_storage_key)
+  end
+end

config/initializers/metrics.rb

@@ -148,6 +148,20 @@ if Gitlab::Metrics.enabled?
 
     config.instrument_methods(Gitlab::Highlight)
     config.instrument_instance_methods(Gitlab::Highlight)
+
+    config.instrument_instance_methods(Search::GlobalService)
+    config.instrument_instance_methods(Search::ProjectService)
+    config.instrument_instance_methods(Gitlab::Elastic::SearchResults)
+    config.instrument_instance_methods(Gitlab::Elastic::ProjectSearchResults)
+    config.instrument_instance_methods(Elastic::ApplicationSearch)
+    config.instrument_instance_methods(Elastic::IssuesSearch)
+    config.instrument_instance_methods(Elastic::MergeRequestsSearch)
+    config.instrument_instance_methods(Elastic::MilestonesSearch)
+    config.instrument_instance_methods(Elastic::NotesSearch)
+    config.instrument_instance_methods(Elastic::ProjectsSearch)
+    config.instrument_instance_methods(Elastic::RepositoriesSearch)
+    config.instrument_instance_methods(Elastic::SnippetsSearch)
+    config.instrument_instance_methods(Elastic::WikiRepositoriesSearch)
   end
 
   GC::Profiler.enable

db/migrate/20160713200638_add_repository_read_only_to_projects.rb

+class AddRepositoryReadOnlyToProjects < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :projects, :repository_read_only, :boolean
+  end
+end

db/migrate/20160718210912_add_ldap_sync_state_to_groups.rb

+# Migration type: online without errors (works on previous version and new one)
+class AddLdapSyncStateToGroups < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
+    add_column_with_default :namespaces, :ldap_sync_status, :string, default: 'ready'
+    add_column :namespaces, :ldap_sync_error, :string
+    add_column :namespaces, :ldap_sync_last_update_at, :datetime
+    add_column :namespaces, :ldap_sync_last_successful_update_at, :datetime
+    add_column :namespaces, :ldap_sync_last_sync_at, :datetime
+  end
+
+  def down
+    remove_column :namespaces, :ldap_sync_status
+    remove_column :namespaces, :ldap_sync_error
+    remove_column :namespaces, :ldap_sync_last_update_at
+    remove_column :namespaces, :ldap_sync_last_successful_update_at
+    remove_column :namespaces, :ldap_sync_last_sync_at
+  end
+end

db/migrate/20160718210939_add_ldap_sync_state_indices_to_groups.rb

+# Migration type: online without errors (works on previous version and new one)
+class AddLdapSyncStateIndicesToGroups < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
+    add_concurrent_index :namespaces, :ldap_sync_last_update_at
+    add_concurrent_index :namespaces, :ldap_sync_last_successful_update_at
+  end
+
+  def down
+    remove_index :namespaces, column: :ldap_sync_last_update_at if index_exists?(:namespaces, :ldap_sync_last_update_at)
+    remove_index :namespaces, column: :ldap_sync_last_successful_update_at if index_exists?(:namespaces, :ldap_sync_last_successful_update_at)
+  end
+end

db/migrate/20160718211006_remove_last_ldap_sync_status_index_from_groups.rb

+# Migration type: online without errors (works on previous version and new one)
+class RemoveLastLdapSyncStatusIndexFromGroups < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+  disable_ddl_transaction!
+
+  DOWNTIME = false
+
+  def up
+    remove_index :namespaces, column: :last_ldap_sync_at if index_exists?(:namespaces, :last_ldap_sync_at)
+  end
+
+  def down
+    add_concurrent_index :namespaces, :last_ldap_sync_at
+  end
+end

db/migrate/20160718211059_remove_last_ldap_sync_status_from_groups.rb

+# Migration type: online without errors (works on previous version and new one)
+class RemoveLastLdapSyncStatusFromGroups < ActiveRecord::Migration
+  DOWNTIME = false
+
+  def change
+    remove_column :namespaces, :last_ldap_sync_at, :datetime
+  end
+end

db/migrate/20160810153405_remove_geo_node_key_id_index.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class RemoveGeoNodeKeyIdIndex < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    remove_index(:geo_nodes, :geo_node_key_id)
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160804150737) do
+ActiveRecord::Schema.define(version: 20160810153405) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -87,17 +87,17 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.string   "health_check_access_token"
     t.boolean  "send_user_confirmation_email",          default: false
     t.integer  "container_registry_token_expire_delay", default: 5
-    t.boolean  "user_default_external",                 default: false,       null: false
     t.text     "after_sign_up_text"
+    t.boolean  "user_default_external",                 default: false,       null: false
     t.boolean  "elasticsearch_indexing",                default: false,       null: false
     t.boolean  "elasticsearch_search",                  default: false,       null: false
     t.string   "elasticsearch_host",                    default: "localhost"
     t.string   "elasticsearch_port",                    default: "9200"
     t.string   "repository_storage",                    default: "default"
     t.string   "enabled_git_access_protocol"
-    t.boolean  "usage_ping_enabled",                    default: true,        null: false
     t.boolean  "domain_blacklist_enabled",              default: false
     t.text     "domain_blacklist"
+    t.boolean  "usage_ping_enabled",                    default: true,        null: false
   end
 
   create_table "approvals", force: :cascade do |t|
@@ -193,8 +193,8 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.text     "artifacts_metadata"
     t.integer  "erased_by_id"
     t.datetime "erased_at"
-    t.string   "environment"
     t.datetime "artifacts_expire_at"
+    t.string   "environment"
     t.integer  "artifacts_size"
     t.string   "when"
     t.text     "yaml_variables"
@@ -500,7 +500,6 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.integer "system_hook_id"
   end
 
-  add_index "geo_nodes", ["geo_node_key_id"], name: "index_geo_nodes_on_geo_node_key_id", using: :btree
   add_index "geo_nodes", ["host"], name: "index_geo_nodes_on_host", using: :btree
   add_index "geo_nodes", ["primary"], name: "index_geo_nodes_on_primary", using: :btree
 
@@ -549,10 +548,10 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.integer  "iid"
     t.integer  "updated_by_id"
     t.integer  "weight"
-    t.integer  "moved_to_id"
     t.boolean  "confidential",  default: false
     t.datetime "deleted_at"
     t.date     "due_date"
+    t.integer  "moved_to_id"
   end
 
   add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree
@@ -746,23 +745,28 @@ ActiveRecord::Schema.define(version: 20160804150737) do
   add_index "milestones", ["title"], name: "index_milestones_on_title_trigram", using: :gin, opclasses: {"title"=>"gin_trgm_ops"}
 
   create_table "namespaces", force: :cascade do |t|
-    t.string   "name",                                   null: false
-    t.string   "path",                                   null: false
+    t.string   "name",                                                  null: false
+    t.string   "path",                                                  null: false
     t.integer  "owner_id"
     t.datetime "created_at"
     t.datetime "updated_at"
     t.string   "type"
-    t.string   "description",            default: "",    null: false
+    t.string   "description",                         default: "",      null: false
     t.string   "avatar"
-    t.boolean  "membership_lock",       default: false
-    t.boolean  "share_with_group_lock", default: false
-    t.integer  "visibility_level",      default: 20,    null: false
-    t.datetime "last_ldap_sync_at"
-    t.boolean  "request_access_enabled", default: true,  null: false
+    t.boolean  "membership_lock",                     default: false
+    t.boolean  "share_with_group_lock",               default: false
+    t.integer  "visibility_level",                    default: 20,      null: false
+    t.boolean  "request_access_enabled",              default: true,    null: false
+    t.string   "ldap_sync_status",                    default: "ready", null: false
+    t.string   "ldap_sync_error"
+    t.datetime "ldap_sync_last_update_at"
+    t.datetime "ldap_sync_last_successful_update_at"
+    t.datetime "ldap_sync_last_sync_at"
   end
 
   add_index "namespaces", ["created_at", "id"], name: "index_namespaces_on_created_at_and_id", using: :btree
-  add_index "namespaces", ["last_ldap_sync_at"], name: "index_namespaces_on_last_ldap_sync_at", using: :btree
+  add_index "namespaces", ["ldap_sync_last_successful_update_at"], name: "index_namespaces_on_ldap_sync_last_successful_update_at", using: :btree
+  add_index "namespaces", ["ldap_sync_last_update_at"], name: "index_namespaces_on_ldap_sync_last_update_at", using: :btree
   add_index "namespaces", ["name"], name: "index_namespaces_on_name", unique: true, using: :btree
   add_index "namespaces", ["name"], name: "index_namespaces_on_name_trigram", using: :gin, opclasses: {"name"=>"gin_trgm_ops"}
   add_index "namespaces", ["owner_id"], name: "index_namespaces_on_owner_id", using: :btree
@@ -886,10 +890,10 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.integer  "user_id",                    null: false
     t.string   "token",                      null: false
     t.string   "name",                       null: false
-    t.datetime "created_at",                 null: false
-    t.datetime "updated_at",                 null: false
     t.boolean  "revoked",    default: false
     t.datetime "expires_at"
+    t.datetime "created_at",                 null: false
+    t.datetime "updated_at",                 null: false
   end
 
   add_index "personal_access_tokens", ["token"], name: "index_personal_access_tokens_on_token", unique: true, using: :btree
@@ -919,11 +923,10 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.datetime "updated_at"
     t.integer  "creator_id"
     t.boolean  "issues_enabled",                     default: true,      null: false
+    t.boolean  "wall_enabled",                       default: true,      null: false
     t.boolean  "merge_requests_enabled",             default: true,      null: false
     t.boolean  "wiki_enabled",                       default: true,      null: false
     t.integer  "namespace_id"
-    t.string   "issues_tracker",                     default: "gitlab",  null: false
-    t.string   "issues_tracker_id"
     t.boolean  "snippets_enabled",                   default: true,      null: false
     t.datetime "last_activity_at"
     t.string   "import_url"
@@ -964,8 +967,9 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.boolean  "only_allow_merge_if_build_succeeds", default: false,     null: false
     t.boolean  "has_external_issue_tracker"
     t.string   "repository_storage",                 default: "default", null: false
-    t.boolean  "has_external_wiki"
     t.boolean  "request_access_enabled",             default: true,      null: false
+    t.boolean  "has_external_wiki"
+    t.boolean  "repository_read_only"
   end
 
   add_index "projects", ["builds_enabled", "shared_runners_enabled"], name: "index_projects_on_builds_enabled_and_shared_runners_enabled", using: :btree
@@ -1298,8 +1302,8 @@ ActiveRecord::Schema.define(version: 20160804150737) do
     t.boolean  "note_events",                          default: false,         null: false
     t.boolean  "enable_ssl_verification",              default: true
     t.boolean  "build_events",                         default: false,         null: false
-    t.boolean  "wiki_page_events",                     default: false,         null: false
     t.string   "token"
+    t.boolean  "wiki_page_events",                     default: false,         null: false
   end
 
   add_index "web_hooks", ["created_at", "id"], name: "index_web_hooks_on_created_at_and_id", using: :btree
@@ -1308,8 +1312,8 @@ ActiveRecord::Schema.define(version: 20160804150737) do
   add_foreign_key "path_locks", "projects"
   add_foreign_key "path_locks", "users"
   add_foreign_key "personal_access_tokens", "users"
-  add_foreign_key "remote_mirrors", "projects"
   add_foreign_key "protected_branch_merge_access_levels", "protected_branches"
   add_foreign_key "protected_branch_push_access_levels", "protected_branches"
+  add_foreign_key "remote_mirrors", "projects"
   add_foreign_key "u2f_registrations", "users"
 end

doc/api/projects.md

@@ -280,7 +280,8 @@ Parameters:
       "group_name": "Gitlab Org",
       "group_access_level": 10
     }
-  ]
+  ],
+  "repository_storage": "default"
 }
 ```
 
@@ -448,6 +449,7 @@ Parameters:
 - `visibility_level` (optional)
 - `import_url` (optional)
 - `public_builds` (optional)
+- `repository_storage` (optional, available only for admins)
 
 ### Create project for user
 
@@ -473,6 +475,7 @@ Parameters:
 - `visibility_level` (optional)
 - `import_url` (optional)
 - `public_builds` (optional)
+- `repository_storage` (optional, available only for admins)
 
 ### Edit project
 
@@ -499,6 +502,7 @@ Parameters:
 - `public` (optional) - if `true` same as setting visibility_level = 20
 - `visibility_level` (optional)
 - `public_builds` (optional)
+- `repository_storage` (optional, available only for admins)
 
 On success, method returns 200 with the updated project. If parameters are
 invalid, 400 is returned.

doc/raketasks/cleanup.md

@@ -22,3 +22,13 @@ sudo gitlab-rake gitlab:cleanup:repos
 # installation from source
 bundle exec rake gitlab:cleanup:repos RAILS_ENV=production
 ```
+
+Remove old repository copies from repositories moved to another storage.
+
+```
+# omnibus-gitlab
+sudo gitlab-rake gitlab:cleanup:moved
+
+# installation from source
+bundle exec rake gitlab:cleanup:moved RAILS_ENV=production
+```

features/group/analytics.feature

-Feature: Group Analytics
-  Background:
-    Given I sign in as "John Doe"
-    And "John Doe" is owner of group "Owned"
-
-  Scenario: I should see group "Owned" analytics page
-    When I visit group "Owned" page
-    And I click on group analytics
-    Then I should see group analytics page

features/steps/group/analytics.rb

-class Spinach::Features::GroupAnalytics < Spinach::FeatureSteps
-  include SharedAuthentication
-  include SharedPaths
-  include SharedGroup
-  include SharedUser
-
-  step 'I click on group analytics' do
-    click_link 'Contribution Analytics'
-  end
-
-  step 'I should see group analytics page' do
-    expect(page).to have_content "Contribution analytics for issues, merge requests and push"
-  end
-end

lib/api/entities.rb

@@ -94,6 +94,7 @@ module API
       expose :shared_with_groups do |project, options|
         SharedGroup.represent(project.project_group_links.all, options)
       end
+      expose :repository_storage, if: lambda { |_project, options| options[:user].try(:admin?) }
     end
 
     class ProjectMember < UserBasic

lib/api/projects.rb

@@ -105,6 +105,7 @@ module API
       #   visibility_level (optional) - 0 by default
       #   import_url (optional)
       #   public_builds (optional)
+      #   repository_storage (optional)
       # Example Request
       #   POST /projects
       post do
@@ -123,7 +124,8 @@ module API
                                      :public,
                                      :visibility_level,
                                      :import_url,
-                                     :public_builds]
+                                     :public_builds,
+                                     :repository_storage]
         attrs = map_public_to_visibility_level(attrs)
         @project = ::Projects::CreateService.new(current_user, attrs).execute
         if @project.saved?
@@ -155,6 +157,7 @@ module API
       #   visibility_level (optional)
       #   import_url (optional)
       #   public_builds (optional)
+      #   repository_storage (optional)
       # Example Request
       #   POST /projects/user/:user_id
       post "user/:user_id" do
@@ -172,7 +175,8 @@ module API
                                      :public,
                                      :visibility_level,
                                      :import_url,
-                                     :public_builds]
+                                     :public_builds,
+                                     :repository_storage]
         attrs = map_public_to_visibility_level(attrs)
         @project = ::Projects::CreateService.new(user, attrs).execute
         if @project.saved?
@@ -218,6 +222,7 @@ module API
       #   public (optional) - if true same as setting visibility_level = 20
       #   visibility_level (optional) - visibility level of a project
       #   public_builds (optional)
+      #   repository_storage (optional)
       # Example Request
       #   PUT /projects/:id
       put ':id' do
@@ -234,7 +239,8 @@ module API
                                      :shared_runners_enabled,
                                      :public,
                                      :visibility_level,
-                                     :public_builds]
+                                     :public_builds,
+                                     :repository_storage]
         attrs = map_public_to_visibility_level(attrs)
         authorize_admin_project
         authorize! :rename_project, user_project if attrs[:name].present?

lib/ee/gitlab/ldap/sync/group.rb

@@ -16,11 +16,13 @@ module EE
           end
 
           def update_permissions
-            lease = ::Gitlab::ExclusiveLease.new(
-              "ldap_group_sync:#{provider}:#{group.id}",
-              timeout: 3600
-            )
-            return unless lease.try_obtain
+            fail_stuck_group(group)
+
+            if group.ldap_sync_started?
+              logger.debug { "Group '#{group.name}' is not ready for LDAP sync. Skipping" }
+              return
+            end
+            group.start_ldap_sync
 
             logger.debug { "Syncing '#{group.name}' group" }
 
@@ -38,7 +40,7 @@ module EE
             update_existing_group_membership(group, access_levels)
             add_new_members(group, access_levels)
 
-            group.update(last_ldap_sync_at: Time.now)
+            group.finish_ldap_sync
 
             logger.debug { "Finished syncing '#{group.name}' group" }
           end
@@ -143,6 +145,14 @@ module EE
               .with_identity_provider(provider).preload(:user)
           end
 
+          def fail_stuck_group(group)
+            return false unless group.ldap_sync_started?
+
+            if group.ldap_sync_last_sync_at < 1.hour.ago
+              group.mark_ldap_sync_as_failed('The sync took too long to complete.')
+            end
+          end
+
           def logger
             Rails.logger
           end

lib/ee/gitlab/ldap/sync/groups.rb

@@ -73,7 +73,7 @@ module EE
           def groups_where_group_links_with_provider_ordered
             ::Group.where_group_links_with_provider(provider)
               .preload(:ldap_group_links)
-              .reorder('last_ldap_sync_at ASC, namespaces.id ASC')
+              .reorder('ldap_sync_last_successful_update_at ASC, namespaces.id ASC')
               .distinct
           end
 

lib/gitlab/backend/shell.rb

@@ -106,6 +106,20 @@ module Gitlab
                                    storage, "#{path}.git", "#{new_path}.git"])
     end
 
+    # Move repository storage
+    #
+    # current_storage - project's current storage path
+    # path - project path with namespace
+    # new_storage - new storage path
+    #
+    # Ex.
+    #   mv_storage("/path/to/storage", "randx/gitlab-ci", "/new/storage/path")
+    #
+    def mv_storage(current_storage, path, new_storage)
+      Gitlab::Utils.system_silent([gitlab_shell_projects_path, 'mv-storage',
+                                   current_storage, "#{path}.git", new_storage])
+    end
+
     # Fork repository to new namespace
     # forked_from_storage - forked-from project's storage path
     # path - project path with namespace

lib/gitlab/checks/change_access.rb

@@ -119,11 +119,8 @@ module Gitlab
           # if newrev is blank, the branch was deleted
           return if Gitlab::Git.blank_ref?(@newrev) || !push_rule.commit_validation?
 
-          # if oldrev is blank, the branch was just created
-          oldrev = Gitlab::Git.blank_ref?(@oldrev) ? project.default_branch : @oldrev
-
-          commits(oldrev).each do |commit|
-            next if commit_from_annex_sync?(commit.safe_message) || old_commit?(commit)
+          commits.each do |commit|
+            next if commit_from_annex_sync?(commit.safe_message)
 
             if error = check_commit(commit, push_rule)
               return error
@@ -140,7 +137,7 @@ module Gitlab
         # locks protect default branch only
         return if project.default_branch != branch_name(@ref)
 
-        commits(@oldrev).each do |commit|
+        commits.each do |commit|
           next if commit_from_annex_sync?(commit.safe_message)
 
           commit.raw_diffs.each do |diff|
@@ -216,12 +213,8 @@ module Gitlab
         nil
       end
 
-      def commits(oldrev)
-        if oldrev
-          project.repository.commits_between(oldrev, @newrev)
-        else
-          project.repository.commits(@newrev)
-        end
+      def commits
+        project.repository.new_commits(@newrev)
       end
 
       def commit_from_annex_sync?(commit_message)
@@ -230,11 +223,6 @@ module Gitlab
         # Commit message starting with <git-annex in > so avoid push rules on this
         commit_message.start_with?('git-annex in')
       end
-
-      def old_commit?(commit)
-        # We skip refs/tmp ref because we use it for Web UI commiting
-        commit.refs(project.repository).reject { |ref| ref.name.start_with?('refs/tmp') }.any?
-      end
     end
   end
 end

lib/gitlab/elastic/project_search_results.rb

@@ -6,7 +6,7 @@ module Gitlab
     class ProjectSearchResults < Gitlab::Elastic::SearchResults
       attr_reader :project, :repository_ref
 
-      def initialize(current_user, project_id, query, repository_ref = nil)
+      def initialize(current_user, query, project_id, repository_ref = nil)
         @current_user = current_user
         @project = Project.find(project_id)
 
@@ -16,6 +16,7 @@ module Gitlab
                             nil
                           end
         @query = query
+        @public_and_internal_projects = false
       end
 
       def objects(scope, page = nil)
@@ -90,7 +91,8 @@ module Gitlab
       def notes
         opt = {
           project_ids: limit_project_ids,
-          current_user: @current_user
+          current_user: @current_user,
+          public_and_internal_projects: @public_and_internal_projects
         }
 
         Note.elastic_search(query, options: opt)

lib/gitlab/elastic/search_results.rb

@@ -7,10 +7,11 @@ module Gitlab
       # It allows us to search only for projects user has access to
       attr_reader :limit_project_ids
 
-      def initialize(current_user, limit_project_ids, query)
+      def initialize(current_user, query, limit_project_ids, public_and_internal_projects = true)
         @current_user = current_user
-        @limit_project_ids = limit_project_ids || Project.all
+        @limit_project_ids = limit_project_ids
         @query = Shellwords.shellescape(query) if query.present?
+        @public_and_internal_projects = public_and_internal_projects
       end
 
       def objects(scope, page = nil)
@@ -56,7 +57,8 @@ module Gitlab
 
       def projects
         opt = {
-          pids: limit_project_ids
+          pids: limit_project_ids,
+          public_and_internal_projects: @public_and_internal_projects
         }
 
         @projects = Project.elastic_search(query, options: opt)
@@ -65,7 +67,8 @@ module Gitlab
       def issues
         opt = {
           project_ids: limit_project_ids,
-          current_user: current_user
+          current_user: current_user,
+          public_and_internal_projects: @public_and_internal_projects
         }
 
         Issue.elastic_search(query, options: opt)
@@ -73,7 +76,8 @@ module Gitlab
 
       def milestones
         opt = {
-          project_ids: limit_project_ids
+          project_ids: limit_project_ids,
+          public_and_internal_projects: @public_and_internal_projects
         }
 
         Milestone.elastic_search(query, options: opt)
@@ -81,7 +85,8 @@ module Gitlab
 
       def merge_requests
         opt = {
-          project_ids: limit_project_ids
+          project_ids: limit_project_ids,
+          public_and_internal_projects: @public_and_internal_projects
         }
 
         MergeRequest.elastic_search(query, options: opt)

lib/gitlab/git_access.rb

@@ -59,6 +59,10 @@ module Gitlab
     end
 
     def push_access_check(changes)
+      if project.repository_read_only?
+        return build_status_object(false, 'The repository is temporarily read-only. Please try again later.')
+      end
+
       if Gitlab::Geo.secondary?
         return build_status_object(false, "You can't push code on a secondary GitLab Geo node.")
       end

lib/tasks/gitlab/cleanup.rake

@@ -27,7 +27,7 @@ namespace :gitlab do
         all_dirs.each do |dir_path|
 
           if remove_flag
-            if FileUtils.rm_rf dir_path
+            if FileUtils.rm_rf(dir_path)
               puts "Removed...#{dir_path}".color(:red)
             else
               puts "Cannot remove #{dir_path}".color(:red)
@@ -43,6 +43,34 @@ namespace :gitlab do
       end
     end
 
+    desc "GitLab | Cleanup | Delete moved repositories"
+    task moved: :environment  do
+      warn_user_is_not_gitlab
+      remove_flag = ENV['REMOVE']
+
+      Gitlab.config.repositories.storages.each do |name, repo_root|
+        # Look for global repos (legacy, depth 1) and normal repos (depth 2)
+        IO.popen(%W(find #{repo_root.chomp('/')} -mindepth 1 -maxdepth 2 -name *+moved*.git)) do |find|
+          find.each_line do |path|
+            path.chomp!
+            if remove_flag
+              if FileUtils.rm_rf(path)
+                puts "Removed...#{path}".color(:green)
+              else
+                puts "Cannot remove #{path}".color(:red)
+              end
+            else
+              puts "Can be removed: #{path}".color(:green)
+            end
+          end
+        end
+      end
+
+      unless remove_flag
+        puts "To cleanup these repositories run this command with REMOVE=true".color(:yellow)
+      end
+    end
+
     desc "GitLab | Cleanup | Clean repositories"
     task repos: :environment  do
       warn_user_is_not_gitlab

lib/tasks/gitlab/elastic.rake

@@ -63,6 +63,7 @@ namespace :gitlab do
       projects.find_each do |project|
         unless project.wiki.empty?
           puts "Indexing wiki of #{project.name_with_namespace}..."
+
           begin
             project.wiki.index_blobs
             puts "Done!".color(:green)
@@ -78,10 +79,13 @@ namespace :gitlab do
       [Project, Issue, MergeRequest, Snippet, Note, Milestone].each do |klass|
         print "Indexing #{klass} records... "
 
-        if klass == Note
-          Note.searchable.import
-        else
+        case klass
+        when Note
+          Note.searchable.import_with_parent
+        when Project, Snippet
           klass.import
+        else
+          klass.import_with_parent
         end
 
         puts "done".color(:green)

spec/controllers/admin/users_controller_spec.rb

@@ -39,7 +39,7 @@ describe Admin::UsersController do
         user.ldap_block
       end
 
-      it 'will not unblock user' do
+      it 'does not unblock user' do
         put :unblock, id: user.username
         user.reload
         expect(user.blocked?).to be_truthy

spec/controllers/application_controller_spec.rb

@@ -5,7 +5,7 @@ describe ApplicationController do
     let(:user) { create(:user) }
     let(:controller) { ApplicationController.new }
 
-    it 'should redirect if the user is over their password expiry' do
+    it 'redirects if the user is over their password expiry' do
       user.password_expires_at = Time.new(2002)
       expect(user.ldap_user?).to be_falsey
       allow(controller).to receive(:current_user).and_return(user)
@@ -14,7 +14,7 @@ describe ApplicationController do
       controller.send(:check_password_expiration)
     end
 
-    it 'should not redirect if the user is under their password expiry' do
+    it 'does not redirect if the user is under their password expiry' do
       user.password_expires_at = Time.now + 20010101
       expect(user.ldap_user?).to be_falsey
       allow(controller).to receive(:current_user).and_return(user)
@@ -22,7 +22,7 @@ describe ApplicationController do
       controller.send(:check_password_expiration)
     end
 
-    it 'should not redirect if the user is over their password expiry but they are an ldap user' do
+    it 'does not redirect if the user is over their password expiry but they are an ldap user' do
       user.password_expires_at = Time.new(2002)
       allow(user).to receive(:ldap_user?).and_return(true)
       allow(controller).to receive(:current_user).and_return(user)

spec/controllers/groups/avatars_controller_spec.rb

@@ -9,7 +9,7 @@ describe Groups::AvatarsController do
     sign_in(user)
   end
 
-  it 'destroy should remove avatar from DB' do
+  it 'removes avatar from DB calling destroy' do
     delete :destroy, group_id: group.path
     @group = assigns(:group)
     expect(@group.avatar.present?).to be_falsey

spec/controllers/groups/milestones_controller_spec.rb

@@ -15,7 +15,7 @@ describe Groups::MilestonesController do
   end
 
   describe "#create" do
-    it "should create group milestone with Chinese title" do
+    it "creates group milestone with Chinese title" do
       post :create,
            group_id: group.id,
            milestone: { project_ids: [project.id, project2.id], title: title }

spec/controllers/profiles/avatars_controller_spec.rb

@@ -8,7 +8,7 @@ describe Profiles::AvatarsController do
     controller.instance_variable_set(:@user, user)
   end
 
-  it 'destroy should remove avatar from DB' do
+  it 'removes avatar from DB by calling destroy' do
     delete :destroy
     @user = assigns(:user)
     expect(@user.avatar.present?).to be_falsey

spec/controllers/profiles/keys_controller_spec.rb

@@ -6,7 +6,7 @@ describe Profiles::KeysController do
   describe '#new' do
     before { sign_in(user) }
 
-    it 'redirect to #index' do
+    it 'redirects to #index' do
       get :new
 
       expect(response).to redirect_to(profile_keys_path)
@@ -15,7 +15,7 @@ describe Profiles::KeysController do
 
   describe "#get_keys" do
     describe "non existant user" do
-      it "should generally not work" do
+      it "does not generally work" do
         get :get_keys, username: 'not-existent'
 
         expect(response).not_to be_success
@@ -23,19 +23,19 @@ describe Profiles::KeysController do
     end
 
     describe "user with no keys" do
-      it "should generally work" do
+      it "does generally work" do
         get :get_keys, username: user.username
 
         expect(response).to be_success
       end
 
-      it "should render all keys separated with a new line" do
+      it "renders all keys separated with a new line" do
         get :get_keys, username: user.username
 
         expect(response.body).to eq("")
       end
 
-      it "should respond with text/plain content type" do
+      it "responds with text/plain content type" do
         get :get_keys, username: user.username
         expect(response.content_type).to eq("text/plain")
       end
@@ -47,13 +47,13 @@ describe Profiles::KeysController do
         user.keys << create(:another_key)
       end
 
-      it "should generally work" do
+      it "does generally work" do
         get :get_keys, username: user.username
 
         expect(response).to be_success
       end
 
-      it "should render all keys separated with a new line" do
+      it "renders all keys separated with a new line" do
         get :get_keys, username: user.username
 
         expect(response.body).not_to eq("")
@@ -65,13 +65,13 @@ describe Profiles::KeysController do
         expect(response.body).to match(/AQDmTillFzNTrrGgwaCKaSj/)
       end
 
-      it "should not render the comment of the key" do
+      it "does not render the comment of the key" do
         get :get_keys, username: user.username
 
         expect(response.body).not_to match(/dummy@gitlab.com/)
       end
 
-      it "should respond with text/plain content type" do
+      it "responds with text/plain content type" do
         get :get_keys, username: user.username
         expect(response.content_type).to eq("text/plain")
       end

spec/controllers/projects/avatars_controller_spec.rb

@@ -10,7 +10,7 @@ describe Projects::AvatarsController do
     controller.instance_variable_set(:@project, project)
   end
 
-  it 'destroy should remove avatar from DB' do
+  it 'removes avatar from DB by calling destroy' do
     delete :destroy, namespace_id: project.namespace.id, project_id: project.id
     expect(project.avatar.present?).to be_falsey
     expect(project).to be_valid

spec/controllers/projects/commit_controller_spec.rb

@@ -47,25 +47,25 @@ describe Projects::CommitController do
     end
 
     shared_examples "export as" do |format|
-      it "should generally work" do
+      it "does generally work" do
         go(id: commit.id, format: format)
 
         expect(response).to be_success
       end
 
-      it "should generate it" do
+      it "generates it" do
         expect_any_instance_of(Commit).to receive(:"to_#{format}")
 
         go(id: commit.id, format: format)
       end
 
-      it "should render it" do
+      it "renders it" do
         go(id: commit.id, format: format)
 
         expect(response.body).to eq(commit.send(:"to_#{format}"))
       end
 
-      it "should not escape Html" do
+      it "does not escape Html" do
         allow_any_instance_of(Commit).to receive(:"to_#{format}").
           and_return('HTML entities &<>" ')
 
@@ -88,7 +88,7 @@ describe Projects::CommitController do
         expect(response.body).to start_with("diff --git")
       end
 
-      it "should really only be a git diff without whitespace changes" do
+      it "is only be a git diff without whitespace changes" do
         go(id: '66eceea0db202bb39c4e445e8ca28689645366c5', format: format, w: 1)
 
         expect(response.body).to start_with("diff --git")
@@ -103,13 +103,13 @@ describe Projects::CommitController do
       include_examples "export as", :patch
       let(:format) { :patch }
 
-      it "should really be a git email patch" do
+      it "is a git email patch" do
         go(id: commit.id, format: format)
 
         expect(response.body).to start_with("From #{commit.id}")
       end
 
-      it "should contain a git diff" do
+      it "contains a git diff" do
         go(id: commit.id, format: format)
 
         expect(response.body).to match(/^diff --git/)
@@ -147,7 +147,7 @@ describe Projects::CommitController do
 
   describe 'POST revert' do
     context 'when target branch is not provided' do
-      it 'should render the 404 page' do
+      it 'renders the 404 page' do
         post(:revert,
             namespace_id: project.namespace.to_param,
             project_id: project.to_param,
@@ -159,7 +159,7 @@ describe Projects::CommitController do
     end
 
     context 'when the revert was successful' do
-      it 'should redirect to the commits page' do
+      it 'redirects to the commits page' do
         post(:revert,
             namespace_id: project.namespace.to_param,
             project_id: project.to_param,
@@ -180,7 +180,7 @@ describe Projects::CommitController do
             id: commit.id)
       end
 
-      it 'should redirect to the commit page' do
+      it 'redirects to the commit page' do
         # Reverting a commit that has been already reverted.
         post(:revert,
             namespace_id: project.namespace.to_param,
@@ -196,7 +196,7 @@ describe Projects::CommitController do
 
   describe 'POST cherry_pick' do
     context 'when target branch is not provided' do
-      it 'should render the 404 page' do
+      it 'renders the 404 page' do
         post(:cherry_pick,
             namespace_id: project.namespace.to_param,
             project_id: project.to_param,
@@ -208,7 +208,7 @@ describe Projects::CommitController do
     end
 
     context 'when the cherry-pick was successful' do
-      it 'should redirect to the commits page' do
+      it 'redirects to the commits page' do
         post(:cherry_pick,
             namespace_id: project.namespace.to_param,
             project_id: project.to_param,
@@ -229,7 +229,7 @@ describe Projects::CommitController do
             id: master_pickable_commit.id)
       end
 
-      it 'should redirect to the commit page' do
+      it 'redirects to the commit page' do
         # Cherry-picking a commit that has been already cherry-picked.
         post(:cherry_pick,
             namespace_id: project.namespace.to_param,

spec/controllers/projects/commits_controller_spec.rb

@@ -11,7 +11,7 @@ describe Projects::CommitsController do
 
   describe "GET show" do
     context "as atom feed" do
-      it "should render as atom" do
+      it "renders as atom" do
         get(:show,
             namespace_id: project.namespace.to_param,
             project_id: project.to_param,

spec/controllers/projects/compare_controller_spec.rb

@@ -11,7 +11,7 @@ describe Projects::CompareController do
     project.team << [user, :master]
   end
 
-  it 'compare should show some diffs' do
+  it 'compare shows some diffs' do
     get(:show,
         namespace_id: project.namespace.to_param,
         project_id: project.to_param,
@@ -23,7 +23,7 @@ describe Projects::CompareController do
     expect(assigns(:commits).length).to be >= 1
   end
 
-  it 'compare should show some diffs with ignore whitespace change option' do
+  it 'compare shows some diffs with ignore whitespace change option' do
     get(:show,
         namespace_id: project.namespace.to_param,
         project_id: project.to_param,
@@ -41,7 +41,7 @@ describe Projects::CompareController do
   end
 
   describe 'non-existent refs' do
-    it 'invalid source ref' do
+    it 'uses invalid source ref' do
       get(:show,
           namespace_id: project.namespace.to_param,
           project_id: project.to_param,
@@ -53,7 +53,7 @@ describe Projects::CompareController do
       expect(assigns(:commits)).to eq([])
     end
 
-    it 'invalid target ref' do
+    it 'uses invalid target ref' do
       get(:show,
           namespace_id: project.namespace.to_param,
           project_id: project.to_param,

spec/controllers/projects/forks_controller_spec.rb

@@ -16,7 +16,7 @@ describe Projects::ForksController do
     context 'when fork is public' do
       before { forked_project.update_attribute(:visibility_level, Project::PUBLIC) }
 
-      it 'should be visible for non logged in users' do
+      it 'is visible for non logged in users' do
         get_forks
 
         expect(assigns[:forks]).to be_present
@@ -28,7 +28,7 @@ describe Projects::ForksController do
         forked_project.update_attributes(visibility_level: Project::PRIVATE, group: group)
       end
 
-      it 'should not be visible for non logged in users' do
+      it 'is not be visible for non logged in users' do
         get_forks
 
         expect(assigns[:forks]).to be_blank
@@ -38,7 +38,7 @@ describe Projects::ForksController do
         before { sign_in(project.creator) }
 
         context 'when user is not a Project member neither a group member' do
-          it 'should not see the Project listed' do
+          it 'does not see the Project listed' do
             get_forks
 
             expect(assigns[:forks]).to be_blank
@@ -48,7 +48,7 @@ describe Projects::ForksController do
         context 'when user is a member of the Project' do
           before { forked_project.team << [project.creator, :developer] }
 
-          it 'should see the project listed' do
+          it 'sees the project listed' do
             get_forks
 
             expect(assigns[:forks]).to be_present
@@ -58,7 +58,7 @@ describe Projects::ForksController do
         context 'when user is a member of the Group' do
           before { forked_project.group.add_developer(project.creator) }
 
-          it 'should see the project listed' do
+          it 'sees the project listed' do
             get_forks
 
             expect(assigns[:forks]).to be_present

spec/controllers/projects/issues_controller_spec.rb

@@ -30,7 +30,7 @@ describe Projects::IssuesController do
         expect(response).to have_http_status(200)
       end
 
-      it "return 301 if request path doesn't match project path" do
+      it "returns 301 if request path doesn't match project path" do
         get :index, namespace_id: project.namespace.path, project_id: project.path.upcase
 
         expect(response).to redirect_to(namespace_project_issues_path(project.namespace, project))
@@ -119,21 +119,21 @@ describe Projects::IssuesController do
     let!(:request_forgery_timing_attack) { create(:issue, :confidential, project: project, assignee: assignee) }
 
     describe 'GET #index' do
-      it 'should not list confidential issues for guests' do
+      it 'does not list confidential issues for guests' do
         sign_out(:user)
         get_issues
 
         expect(assigns(:issues)).to eq [issue]
       end
 
-      it 'should not list confidential issues for non project members' do
+      it 'does not list confidential issues for non project members' do
         sign_in(non_member)
         get_issues
 
         expect(assigns(:issues)).to eq [issue]
       end
 
-      it 'should not list confidential issues for project members with guest role' do
+      it 'does not list confidential issues for project members with guest role' do
         sign_in(member)
         project.team << [member, :guest]
 
@@ -142,7 +142,7 @@ describe Projects::IssuesController do
         expect(assigns(:issues)).to eq [issue]
       end
 
-      it 'should list confidential issues for author' do
+      it 'lists confidential issues for author' do
         sign_in(author)
         get_issues
 
@@ -150,7 +150,7 @@ describe Projects::IssuesController do
         expect(assigns(:issues)).not_to include request_forgery_timing_attack
       end
 
-      it 'should list confidential issues for assignee' do
+      it 'lists confidential issues for assignee' do
         sign_in(assignee)
         get_issues
 
@@ -158,7 +158,7 @@ describe Projects::IssuesController do
         expect(assigns(:issues)).to include request_forgery_timing_attack
       end
 
-      it 'should list confidential issues for project members' do
+      it 'lists confidential issues for project members' do
         sign_in(member)
         project.team << [member, :developer]
 
@@ -168,7 +168,7 @@ describe Projects::IssuesController do
         expect(assigns(:issues)).to include request_forgery_timing_attack
       end
 
-      it 'should list confidential issues for admin' do
+      it 'lists confidential issues for admin' do
         sign_in(admin)
         get_issues
 

spec/controllers/projects/merge_requests_controller_spec.rb

@@ -122,7 +122,7 @@ describe Projects::MergeRequestsController do
 
   describe "GET show" do
     shared_examples "export merge as" do |format|
-      it "should generally work" do
+      it "does generally work" do
         get(:show,
             namespace_id: project.namespace.to_param,
             project_id: project.to_param,
@@ -132,7 +132,7 @@ describe Projects::MergeRequestsController do
         expect(response).to be_success
       end
 
-      it "should generate it" do
+      it "generates it" do
         expect_any_instance_of(MergeRequest).to receive(:"to_#{format}")
 
         get(:show,
@@ -142,7 +142,7 @@ describe Projects::MergeRequestsController do
             format: format)
       end
 
-      it "should render it" do
+      it "renders it" do
         get(:show,
             namespace_id: project.namespace.to_param,
             project_id: project.to_param,
@@ -152,7 +152,7 @@ describe Projects::MergeRequestsController do
         expect(response.body).to eq(merge_request.send(:"to_#{format}").to_s)
       end
 
-      it "should not escape Html" do
+      it "does not escape Html" do
         allow_any_instance_of(MergeRequest).to receive(:"to_#{format}").
           and_return('HTML entities &<>" ')
 
@@ -204,7 +204,7 @@ describe Projects::MergeRequestsController do
 
     context 'when filtering by opened state' do
       context 'with opened merge requests' do
-        it 'should list those merge requests' do
+        it 'lists those merge requests' do
           get_merge_requests
 
           expect(assigns(:merge_requests)).to include(merge_request)
@@ -217,7 +217,7 @@ describe Projects::MergeRequestsController do
           merge_request.reopen!
         end
 
-        it 'should list those merge requests' do
+        it 'lists those merge requests' do
           get_merge_requests
 
           expect(assigns(:merge_requests)).to include(merge_request)

spec/controllers/projects/milestones_controller_spec.rb

@@ -14,7 +14,7 @@ describe Projects::MilestonesController do
   end
 
   describe "#destroy" do
-    it "should remove milestone" do
+    it "removes milestone" do
       expect(issue.milestone_id).to eq(milestone.id)
 
       delete :destroy, namespace_id: project.namespace.id, project_id: project.id, id: milestone.iid, format: :js

spec/controllers/projects/project_members_controller_spec.rb

@@ -167,7 +167,7 @@ describe Projects::ProjectMembersController do
           sign_in(user)
         end
 
-        it 'cannot remove himself from the project' do
+        it 'does not remove himself from the project' do
           delete :leave, namespace_id: project.namespace,
                          project_id: project
 

spec/controllers/projects/protected_branches_controller_spec.rb

@@ -3,7 +3,7 @@ require('spec_helper')
 describe Projects::ProtectedBranchesController do
   describe "GET #index" do
     let(:project) { create(:project_empty_repo, :public) }
-    it "redirect empty repo to projects page" do
+    it "redirects empty repo to projects page" do
       get(:index, namespace_id: project.namespace.to_param, project_id: project.to_param)
     end
   end

spec/controllers/projects/raw_controller_spec.rb

@@ -24,7 +24,7 @@ describe Projects::RawController do
     context 'image header' do
       let(:id) { 'master/files/images/6049019_460s.jpg' }
 
-      it 'set image content type header' do
+      it 'sets image content type header' do
         get(:show,
             namespace_id: public_project.namespace.to_param,
             project_id: public_project.to_param,

spec/controllers/projects/services_controller_spec.rb

@@ -19,7 +19,7 @@ describe Projects::ServicesController do
 
     describe "#test" do
       context 'success' do
-        it "should redirect and show success message" do
+        it "redirects and show success message" do
           expect(service).to receive(:test).and_return({ success: true, result: 'done' })
           get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
           expect(response.status).to redirect_to('/')
@@ -28,7 +28,7 @@ describe Projects::ServicesController do
       end
 
       context 'failure' do
-        it "should redirect and show failure message" do
+        it "redirects and show failure message" do
           expect(service).to receive(:test).and_return({ success: false, result: 'Bad test' })
           get :test, namespace_id: project.namespace.id, project_id: project.id, id: service.id, format: :html
           expect(response.status).to redirect_to('/')

spec/controllers/projects_controller_spec.rb

@@ -128,7 +128,7 @@ describe ProjectsController do
     context "when the url contains .atom" do
       let(:public_project_with_dot_atom) { build(:project, :public, name: 'my.atom', path: 'my.atom') }
 
-      it 'expect an error creating the project' do
+      it 'expects an error creating the project' do
         expect(public_project_with_dot_atom).not_to be_valid
       end
     end
@@ -222,7 +222,7 @@ describe ProjectsController do
           create(:forked_project_link, forked_to_project: project_fork)
         end
 
-        it 'should remove fork from project' do
+        it 'removes fork from project' do
           delete(:remove_fork,
               namespace_id: project_fork.namespace.to_param,
               id: project_fork.to_param, format: :js)
@@ -236,7 +236,7 @@ describe ProjectsController do
       context 'when project not forked' do
         let(:unforked_project) { create(:project, namespace: user.namespace) }
 
-        it 'should do nothing if project was not forked' do
+        it 'does nothing if project was not forked' do
           delete(:remove_fork,
               namespace_id: unforked_project.namespace.to_param,
               id: unforked_project.to_param, format: :js)
@@ -256,7 +256,7 @@ describe ProjectsController do
   end
 
   describe "GET refs" do
-    it "should get a list of branches and tags" do
+    it "gets a list of branches and tags" do
       get :refs, namespace_id: public_project.namespace.path, id: public_project.path
 
       parsed_body = JSON.parse(response.body)
@@ -265,7 +265,7 @@ describe ProjectsController do
       expect(parsed_body["Commits"]).to be_nil
     end
 
-    it "should get a list of branches, tags and commits" do
+    it "gets a list of branches, tags and commits" do
       get :refs, namespace_id: public_project.namespace.path, id: public_project.path, ref: "123456"
 
       parsed_body = JSON.parse(response.body)

spec/factories/projects.rb

@@ -44,6 +44,10 @@ FactoryGirl.define do
         project.create_repository
       end
     end
+
+    trait :read_only_repository do
+      repository_read_only true
+    end
   end
 
   # Project with empty repository

spec/factories_spec.rb

@@ -9,7 +9,7 @@ describe 'factories' do
         expect { entity }.not_to raise_error
       end
 
-      it 'should be valid', if: factory.build_class < ActiveRecord::Base do
+      it 'is valid', if: factory.build_class < ActiveRecord::Base do
         expect(entity).to be_valid
       end
     end

spec/features/admin/admin_abuse_reports_spec.rb

@@ -11,7 +11,7 @@ describe "Admin::AbuseReports", feature: true, js: true  do
       end
 
       describe 'in the abuse report view' do
-        it "should present a link to the user's profile" do
+        it "presents a link to the user's profile" do
           visit admin_abuse_reports_path
 
           expect(page).to have_link user.name, href: user_path(user)
@@ -19,7 +19,7 @@ describe "Admin::AbuseReports", feature: true, js: true  do
       end
 
       describe 'in the profile page of the user' do
-        it 'should show a link to the admin view of the user' do
+        it 'shows a link to the admin view of the user' do
           visit user_path(user)
 
           expect(page).to have_link '', href: admin_user_path(user)

spec/features/admin/admin_hooks_spec.rb

@@ -9,7 +9,7 @@ describe "Admin::Hooks", feature: true do
   end
 
   describe "GET /admin/hooks" do
-    it "should be ok" do
+    it "is ok" do
       visit admin_root_path
 
       page.within ".layout-nav" do
@@ -19,7 +19,7 @@ describe "Admin::Hooks", feature: true do
       expect(current_path).to eq(admin_hooks_path)
     end
 
-    it "should have hooks list" do
+    it "has hooks list" do
       visit admin_hooks_path
       expect(page).to have_content(@system_hook.url)
     end
@@ -33,7 +33,7 @@ describe "Admin::Hooks", feature: true do
       expect { click_button "Add System Hook" }.to change(SystemHook, :count).by(1)
     end
 
-    it "should open new hook popup" do
+    it "opens new hook popup" do
       expect(current_path).to eq(admin_hooks_path)
       expect(page).to have_content(@url)
     end

spec/features/admin/admin_projects_spec.rb

@@ -11,11 +11,11 @@ describe "Admin::Projects", feature: true  do
       visit admin_namespaces_projects_path
     end
 
-    it "should be ok" do
+    it "is ok" do
       expect(current_path).to eq(admin_namespaces_projects_path)
     end
 
-    it "should have projects list" do
+    it "has projects list" do
       expect(page).to have_content(@project.name)
     end
   end
@@ -26,7 +26,7 @@ describe "Admin::Projects", feature: true  do
       click_link "#{@project.name}"
     end
 
-    it "should have project info" do
+    it "has project info" do
       expect(page).to have_content(@project.path)
       expect(page).to have_content(@project.name)
     end

spec/features/admin/admin_users_spec.rb

@@ -8,11 +8,11 @@ describe "Admin::Users", feature: true  do
       visit admin_users_path
     end
 
-    it "should be ok" do
+    it "is ok" do
       expect(current_path).to eq(admin_users_path)
     end
 
-    it "should have users list" do
+    it "has users list" do
       expect(page).to have_content(@user.email)
       expect(page).to have_content(@user.name)
     end
@@ -66,11 +66,11 @@ describe "Admin::Users", feature: true  do
       fill_in "user_email", with: "bigbang@mail.com"
     end
 
-    it "should create new user" do
+    it "creates new user" do
       expect { click_button "Create user" }.to change {User.count}.by(1)
     end
 
-    it "should apply defaults to user" do
+    it "applies defaults to user" do
       click_button "Create user"
       user = User.find_by(username: 'bang')
       expect(user.projects_limit).
@@ -79,20 +79,20 @@ describe "Admin::Users", feature: true  do
         to eq(Gitlab.config.gitlab.default_can_create_group)
     end
 
-    it "should create user with valid data" do
+    it "creates user with valid data" do
       click_button "Create user"
       user = User.find_by(username: 'bang')
       expect(user.name).to eq('Big Bang')
       expect(user.email).to eq('bigbang@mail.com')
     end
 
-    it "should call send mail" do
+    it "calls send mail" do
       expect_any_instance_of(NotificationService).to receive(:new_user)
 
       click_button "Create user"
     end
 
-    it "should send valid email to user with email & password" do
+    it "sends valid email to user with email & password" do
       perform_enqueued_jobs do
         click_button "Create user"
       end
@@ -106,7 +106,7 @@ describe "Admin::Users", feature: true  do
   end
 
   describe "GET /admin/users/:id" do
-    it "should have user info" do
+    it "has user info" do
       visit admin_users_path
       click_link @user.name
 
@@ -123,13 +123,13 @@ describe "Admin::Users", feature: true  do
           expect(page).to have_content('Impersonate')
         end
 
-        it 'should not show impersonate button for admin itself' do
+        it 'does not show impersonate button for admin itself' do
           visit admin_user_path(@user)
 
           expect(page).not_to have_content('Impersonate')
         end
 
-        it 'should not show impersonate button for blocked user' do
+        it 'does not show impersonate button for blocked user' do
           another_user.block
 
           visit admin_user_path(another_user)
@@ -153,7 +153,7 @@ describe "Admin::Users", feature: true  do
           expect(icon).not_to eql nil
         end
 
-        it 'can log out of impersonated user back to original user' do
+        it 'logs out of impersonated user back to original user' do
           find(:css, 'li.impersonation a').click
 
           expect(page.find(:css, '.header-user .profile-link')['data-user']).to eql(@user.username)
@@ -197,7 +197,7 @@ describe "Admin::Users", feature: true  do
       click_link "edit_user_#{@simple_user.id}"
     end
 
-    it "should have user edit page" do
+    it "has user edit page" do
       expect(page).to have_content('Name')
       expect(page).to have_content('Password')
     end
@@ -212,12 +212,12 @@ describe "Admin::Users", feature: true  do
         click_button "Save changes"
       end
 
-      it "should show page with  new data" do
+      it "shows page with  new data" do
         expect(page).to have_content('bigbang@mail.com')
         expect(page).to have_content('Big Bang')
       end
 
-      it "should change user entry" do
+      it "changes user entry" do
         @simple_user.reload
         expect(@simple_user.name).to eq('Big Bang')
         expect(@simple_user.is_admin?).to be_truthy

spec/features/atom/dashboard_spec.rb

@@ -5,7 +5,7 @@ describe "Dashboard Feed", feature: true  do
     let!(:user) { create(:user, name: "Jonh") }
 
     context "projects atom feed via private token" do
-      it "should render projects atom feed" do
+      it "renders projects atom feed" do
         visit dashboard_projects_path(:atom, private_token: user.private_token)
         expect(body).to have_selector('feed title')
       end
@@ -23,11 +23,11 @@ describe "Dashboard Feed", feature: true  do
         visit dashboard_projects_path(:atom, private_token: user.private_token)
       end
 
-      it "should have issue opened event" do
+      it "has issue opened event" do
         expect(body).to have_content("#{user.name} opened issue ##{issue.iid}")
       end
 
-      it "should have issue comment event" do
+      it "has issue comment event" do
         expect(body).
           to have_content("#{user.name} commented on issue ##{issue.iid}")
       end

spec/features/atom/issues_spec.rb

@@ -9,7 +9,7 @@ describe 'Issues Feed', feature: true  do
     before { project.team << [user, :developer] }
 
     context 'when authenticated' do
-      it 'should render atom feed' do
+      it 'renders atom feed' do
         login_with user
         visit namespace_project_issues_path(project.namespace, project, :atom)
 
@@ -22,7 +22,7 @@ describe 'Issues Feed', feature: true  do
     end
 
     context 'when authenticated via private token' do
-      it 'should render atom feed' do
+      it 'renders atom feed' do
         visit namespace_project_issues_path(project.namespace, project, :atom,
                                             private_token: user.private_token)
 

spec/features/atom/users_spec.rb

@@ -5,7 +5,7 @@ describe "User Feed", feature: true  do
     let!(:user) { create(:user) }
 
     context 'user atom feed via private token' do
-      it "should render user atom feed" do
+      it "renders user atom feed" do
         visit user_path(user, :atom, private_token: user.private_token)
         expect(body).to have_selector('feed title')
       end
@@ -43,24 +43,24 @@ describe "User Feed", feature: true  do
         visit user_path(user, :atom, private_token: user.private_token)
       end
 
-      it 'should have issue opened event' do
+      it 'has issue opened event' do
         expect(body).to have_content("#{safe_name} opened issue ##{issue.iid}")
       end
 
-      it 'should have issue comment event' do
+      it 'has issue comment event' do
         expect(body).
           to have_content("#{safe_name} commented on issue ##{issue.iid}")
       end
 
-      it 'should have XHTML summaries in issue descriptions' do
+      it 'has XHTML summaries in issue descriptions' do
         expect(body).to match /we have a bug!<\/p>\n\n<hr ?\/>\n\n<p>I guess/
       end
 
-      it 'should have XHTML summaries in notes' do
+      it 'has XHTML summaries in notes' do
         expect(body).to match /Bug confirmed <img[^>]*\/>/
       end
 
-      it 'should have XHTML summaries in merge request descriptions' do
+      it 'has XHTML summaries in merge request descriptions' do
         expect(body).to match /Here is the fix: <\/p><div[^>]*><a[^>]*><img[^>]*\/><\/a><\/div>/
       end
     end

spec/features/ci_lint_spec.rb

@@ -17,7 +17,7 @@ describe 'CI Lint' do
         File.read(Rails.root.join('spec/support/gitlab_stubs/gitlab_ci.yml'))
       end
 
-      it 'Yaml parsing' do
+      it 'parses Yaml' do
         within "table" do
           expect(page).to have_content('Job - rspec')
           expect(page).to have_content('Job - spinach')

spec/features/commits_spec.rb

@@ -52,7 +52,7 @@ describe 'Commits' do
             visit namespace_project_commits_path(project.namespace, project, :master)
           end
 
-          it 'should show build status' do
+          it 'shows build status' do
             page.within("//li[@id='commit-#{pipeline.short_sha}']") do
               expect(page).to have_css(".ci-status-link")
             end

spec/features/compare_spec.rb

@@ -11,11 +11,11 @@ describe "Compare", js: true do
   end
 
   describe "branches" do
-    it "should pre-populate fields" do
+    it "pre-populates fields" do
       expect(page.find_field("from").value).to eq("master")
     end
 
-    it "should compare branches" do
+    it "compares branches" do
       fill_in "from", with: "fea"
       find("#from").click
 
@@ -28,7 +28,7 @@ describe "Compare", js: true do
   end
 
   describe "tags" do
-    it "should compare tags" do
+    it "compares tags" do
       fill_in "from", with: "v1.0"
       find("#from").click
 

spec/features/dashboard/label_filter_spec.rb

@@ -16,7 +16,7 @@ describe 'Dashboard > label filter', feature: true, js: true do
   end
 
   context 'duplicate labels' do
-    it 'should remove duplicate labels' do
+    it 'removes duplicate labels' do
       page.within('.labels-filter') do
         click_button 'Label'
       end

spec/features/dashboard_issues_spec.rb

@@ -16,7 +16,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do
       visit_issues
     end
 
-    it 'should show all issues with no milestone' do
+    it 'shows all issues with no milestone' do
       show_milestone_dropdown
 
       click_link 'No Milestone'
@@ -24,7 +24,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do
       expect(page).to have_selector('.issue', count: 1)
     end
 
-    it 'should show all issues with any milestone' do
+    it 'shows all issues with any milestone' do
       show_milestone_dropdown
 
       click_link 'Any Milestone'
@@ -32,7 +32,7 @@ describe "Dashboard Issues filtering", feature: true, js: true do
       expect(page).to have_selector('.issue', count: 2)
     end
 
-    it 'should show all issues with the selected milestone' do
+    it 'shows all issues with the selected milestone' do
       show_milestone_dropdown
 
       page.within '.dropdown-content' do

spec/features/gitlab_flavored_markdown_spec.rb

@@ -23,25 +23,25 @@ describe "GitLab Flavored Markdown", feature: true do
   end
 
   describe "for commits" do
-    it "should render title in commits#index" do
+    it "renders title in commits#index" do
       visit namespace_project_commits_path(project.namespace, project, 'master', limit: 1)
 
       expect(page).to have_link(issue.to_reference)
     end
 
-    it "should render title in commits#show" do
+    it "renders title in commits#show" do
       visit namespace_project_commit_path(project.namespace, project, commit)
 
       expect(page).to have_link(issue.to_reference)
     end
 
-    it "should render description in commits#show" do
+    it "renders description in commits#show" do
       visit namespace_project_commit_path(project.namespace, project, commit)
 
       expect(page).to have_link(fred.to_reference)
     end
 
-    it "should render title in repositories#branches" do
+    it "renders title in repositories#branches" do
       visit namespace_project_branches_path(project.namespace, project)
 
       expect(page).to have_link(issue.to_reference)
@@ -62,19 +62,19 @@ describe "GitLab Flavored Markdown", feature: true do
                       description: "ask #{fred.to_reference} for details")
     end
 
-    it "should render subject in issues#index" do
+    it "renders subject in issues#index" do
       visit namespace_project_issues_path(project.namespace, project)
 
       expect(page).to have_link(@other_issue.to_reference)
     end
 
-    it "should render subject in issues#show" do
+    it "renders subject in issues#show" do
       visit namespace_project_issue_path(project.namespace, project, @issue)
 
       expect(page).to have_link(@other_issue.to_reference)
     end
 
-    it "should render details in issues#show" do
+    it "renders details in issues#show" do
       visit namespace_project_issue_path(project.namespace, project, @issue)
 
       expect(page).to have_link(fred.to_reference)
@@ -86,13 +86,13 @@ describe "GitLab Flavored Markdown", feature: true do
       @merge_request = create(:merge_request, source_project: project, target_project: project, title: "fix #{issue.to_reference}")
     end
 
-    it "should render title in merge_requests#index" do
+    it "renders title in merge_requests#index" do
       visit namespace_project_merge_requests_path(project.namespace, project)
 
       expect(page).to have_link(issue.to_reference)
     end
 
-    it "should render title in merge_requests#show" do
+    it "renders title in merge_requests#show" do
       visit namespace_project_merge_request_path(project.namespace, project, @merge_request)
 
       expect(page).to have_link(issue.to_reference)
@@ -107,19 +107,19 @@ describe "GitLab Flavored Markdown", feature: true do
                           description: "ask #{fred.to_reference} for details")
     end
 
-    it "should render title in milestones#index" do
+    it "renders title in milestones#index" do
       visit namespace_project_milestones_path(project.namespace, project)
 
       expect(page).to have_link(issue.to_reference)
     end
 
-    it "should render title in milestones#show" do
+    it "renders title in milestones#show" do
       visit namespace_project_milestone_path(project.namespace, project, @milestone)
 
       expect(page).to have_link(issue.to_reference)
     end
 
-    it "should render description in milestones#show" do
+    it "renders description in milestones#show" do
       visit namespace_project_milestone_path(project.namespace, project, @milestone)
 
       expect(page).to have_link(fred.to_reference)

spec/features/groups/analytics.rb

+require 'spec_helper'
+
+feature 'Groups > Contribution Analytics', js: true, feature: true do
+  let(:user) { create(:user) }
+  let(:group) { create(:group) }
+  let(:empty_project) { create(:empty_project, namespace: group) }
+
+  before do
+    group.add_owner(user)
+    login_with(user)
+  end
+
+  describe 'visit Contribution Analytics page for group' do
+    it 'displays Contribution Analytics' do
+      visit group_path(group)
+
+      click_link 'Contribution Analytics'
+
+      expect(page).to have_content "Contribution analytics for issues, merge requests and push"
+    end
+  end
+end

spec/features/help_pages_spec.rb

@@ -5,7 +5,7 @@ describe 'Help Pages', feature: true do
     before do
       login_as :user
     end
-    it 'replace the variable $your_email with the email of the user' do
+    it 'replaces the variable $your_email with the email of the user' do
       visit help_page_path('ssh/README')
       expect(page).to have_content("ssh-keygen -t rsa -C \"#{@user.email}\"")
     end

spec/features/issues/award_emoji_spec.rb

@@ -21,32 +21,32 @@ describe 'Awards Emoji', feature: true do
       visit namespace_project_issue_path(project.namespace, project, issue)
     end
 
-    it 'should increment the thumbsdown emoji', js: true do
+    it 'increments the thumbsdown emoji', js: true do
       find('[data-emoji="thumbsdown"]').click
       sleep 2
       expect(thumbsdown_emoji).to have_text("1")
     end
 
     context 'click the thumbsup emoji' do
-      it 'should increment the thumbsup emoji', js: true do
+      it 'increments the thumbsup emoji', js: true do
         find('[data-emoji="thumbsup"]').click
         sleep 2
         expect(thumbsup_emoji).to have_text("1")
       end
 
-      it 'should decrement the thumbsdown emoji', js: true do
+      it 'decrements the thumbsdown emoji', js: true do
         expect(thumbsdown_emoji).to have_text("0")
       end
     end
 
     context 'click the thumbsdown emoji' do
-      it 'should increment the thumbsdown emoji', js: true do
+      it 'increments the thumbsdown emoji', js: true do
         find('[data-emoji="thumbsdown"]').click
         sleep 2
         expect(thumbsdown_emoji).to have_text("1")
       end
 
-      it 'should decrement the thumbsup emoji', js: true do
+      it 'decrements the thumbsup emoji', js: true do
         expect(thumbsup_emoji).to have_text("0")
       end
     end

spec/features/issues/award_spec.rb

@@ -11,7 +11,7 @@ feature 'Issue awards', js: true, feature: true do
       visit namespace_project_issue_path(project.namespace, project, issue)
     end
 
-    it 'should add award to issue' do
+    it 'adds award to issue' do
       first('.js-emoji-btn').click
       expect(page).to have_selector('.js-emoji-btn.active')
       expect(first('.js-emoji-btn')).to have_content '1'
@@ -20,7 +20,7 @@ feature 'Issue awards', js: true, feature: true do
       expect(first('.js-emoji-btn')).to have_content '1'
     end
 
-    it 'should remove award from issue' do
+    it 'removes award from issue' do
       first('.js-emoji-btn').click
       find('.js-emoji-btn.active').click
       expect(first('.js-emoji-btn')).to have_content '0'
@@ -29,7 +29,7 @@ feature 'Issue awards', js: true, feature: true do
       expect(first('.js-emoji-btn')).to have_content '0'
     end
 
-    it 'should only have one menu on the page' do
+    it 'only has one menu on the page' do
       first('.js-add-award').click
       expect(page).to have_selector('.emoji-menu')
 
@@ -42,7 +42,7 @@ feature 'Issue awards', js: true, feature: true do
       visit namespace_project_issue_path(project.namespace, project, issue)
     end
 
-    it 'should not see award menu button' do
+    it 'does not see award menu button' do
       expect(page).not_to have_selector('.js-award-holder')
     end
   end

spec/features/issues/bulk_assignment_labels_spec.rb

@@ -175,7 +175,7 @@ feature 'Issues > Labels bulk assignment', feature: true do
           visit namespace_project_issues_path(project.namespace, project)
         end
 
-        it 'labels are kept' do
+        it 'keeps labels' do
           expect(find("#issue_#{issue1.id}")).to have_content 'bug'
           expect(find("#issue_#{issue2.id}")).to have_content 'feature'
 
@@ -197,7 +197,7 @@ feature 'Issues > Labels bulk assignment', feature: true do
           visit namespace_project_issues_path(project.namespace, project)
         end
 
-        it 'existing label is kept and new label is present' do
+        it 'keeps existing label and new label is present' do
           expect(find("#issue_#{issue1.id}")).to have_content 'bug'
 
           check 'check_all_issues'
@@ -222,7 +222,7 @@ feature 'Issues > Labels bulk assignment', feature: true do
           visit namespace_project_issues_path(project.namespace, project)
         end
 
-        it 'existing label is kept and new label is present' do
+        it 'keeps existing label and new label is present' do
           expect(find("#issue_#{issue1.id}")).to have_content 'bug'
           expect(find("#issue_#{issue1.id}")).to have_content 'bug'
           expect(find("#issue_#{issue2.id}")).to have_content 'feature'
@@ -252,7 +252,7 @@ feature 'Issues > Labels bulk assignment', feature: true do
           visit namespace_project_issues_path(project.namespace, project)
         end
 
-        it 'labels are kept' do
+        it 'keeps labels' do
           expect(find("#issue_#{issue1.id}")).to have_content 'bug'
           expect(find("#issue_#{issue1.id}")).to have_content 'First Release'
           expect(find("#issue_#{issue2.id}")).to have_content 'feature'

spec/features/issues/filter_by_labels_spec.rb

@@ -37,25 +37,25 @@ feature 'Issue filtering by Labels', feature: true do
       wait_for_ajax
     end
 
-    it 'should show issue "Bugfix1" and "Bugfix2" in issues list' do
+    it 'shows issue "Bugfix1" and "Bugfix2" in issues list' do
       expect(page).to have_content "Bugfix1"
       expect(page).to have_content "Bugfix2"
     end
 
-    it 'should not show "Feature1" in issues list' do
+    it 'does not show "Feature1" in issues list' do
       expect(page).not_to have_content "Feature1"
     end
 
-    it 'should show label "bug" in filtered-labels' do
+    it 'shows label "bug" in filtered-labels' do
       expect(find('.filtered-labels')).to have_content "bug"
     end
 
-    it 'should not show label "feature" and "enhancement" in filtered-labels' do
+    it 'does not show label "feature" and "enhancement" in filtered-labels' do
       expect(find('.filtered-labels')).not_to have_content "feature"
       expect(find('.filtered-labels')).not_to have_content "enhancement"
     end
 
-    it 'should remove label "bug"' do
+    it 'removes label "bug"' do
       find('.js-label-filter-remove').click
       wait_for_ajax
       expect(find('.filtered-labels', visible: false)).to have_no_content "bug"
@@ -71,20 +71,20 @@ feature 'Issue filtering by Labels', feature: true do
       wait_for_ajax
     end
 
-    it 'should show issue "Feature1" in issues list' do
+    it 'shows issue "Feature1" in issues list' do
       expect(page).to have_content "Feature1"
     end
 
-    it 'should not show "Bugfix1" and "Bugfix2" in issues list' do
+    it 'does not show "Bugfix1" and "Bugfix2" in issues list' do
       expect(page).not_to have_content "Bugfix2"
       expect(page).not_to have_content "Bugfix1"
     end
 
-    it 'should show label "feature" in filtered-labels' do
+    it 'shows label "feature" in filtered-labels' do
       expect(find('.filtered-labels')).to have_content "feature"
     end
 
-    it 'should not show label "bug" and "enhancement" in filtered-labels' do
+    it 'does not show label "bug" and "enhancement" in filtered-labels' do
       expect(find('.filtered-labels')).not_to have_content "bug"
       expect(find('.filtered-labels')).not_to have_content "enhancement"
     end
@@ -99,20 +99,20 @@ feature 'Issue filtering by Labels', feature: true do
       wait_for_ajax
     end
 
-    it 'should show issue "Bugfix2" in issues list' do
+    it 'shows issue "Bugfix2" in issues list' do
       expect(page).to have_content "Bugfix2"
     end
 
-    it 'should not show "Feature1" and "Bugfix1" in issues list' do
+    it 'does not show "Feature1" and "Bugfix1" in issues list' do
       expect(page).not_to have_content "Feature1"
       expect(page).not_to have_content "Bugfix1"
     end
 
-    it 'should show label "enhancement" in filtered-labels' do
+    it 'shows label "enhancement" in filtered-labels' do
       expect(find('.filtered-labels')).to have_content "enhancement"
     end
 
-    it 'should not show label "feature" and "bug" in filtered-labels' do
+    it 'does not show label "feature" and "bug" in filtered-labels' do
       expect(find('.filtered-labels')).not_to have_content "bug"
       expect(find('.filtered-labels')).not_to have_content "feature"
     end
@@ -128,21 +128,21 @@ feature 'Issue filtering by Labels', feature: true do
       wait_for_ajax
     end
 
-    it 'should not show "Bugfix1" or "Feature1" in issues list' do
+    it 'does not show "Bugfix1" or "Feature1" in issues list' do
       expect(page).not_to have_content "Bugfix1"
       expect(page).not_to have_content "Feature1"
     end
 
-    it 'should show label "enhancement" and "feature" in filtered-labels' do
+    it 'shows label "enhancement" and "feature" in filtered-labels' do
       expect(find('.filtered-labels')).to have_content "enhancement"
       expect(find('.filtered-labels')).to have_content "feature"
     end
 
-    it 'should not show label "bug" in filtered-labels' do
+    it 'does not show label "bug" in filtered-labels' do
       expect(find('.filtered-labels')).not_to have_content "bug"
     end
 
-    it 'should remove label "enhancement"' do
+    it 'removes label "enhancement"' do
       find('.js-label-filter-remove', match: :first).click
       wait_for_ajax
       expect(find('.filtered-labels')).to have_no_content "enhancement"
@@ -159,20 +159,20 @@ feature 'Issue filtering by Labels', feature: true do
       wait_for_ajax
     end
 
-    it 'should show issue "Bugfix2" in issues list' do
+    it 'shows issue "Bugfix2" in issues list' do
       expect(page).to have_content "Bugfix2"
     end
 
-    it 'should not show "Feature1"' do
+    it 'does not show "Feature1"' do
       expect(page).not_to have_content "Feature1"
     end
 
-    it 'should show label "bug" and "enhancement" in filtered-labels' do
+    it 'shows label "bug" and "enhancement" in filtered-labels' do
       expect(find('.filtered-labels')).to have_content "bug"
       expect(find('.filtered-labels')).to have_content "enhancement"
     end
 
-    it 'should not show label "feature" in filtered-labels' do
+    it 'does not show label "feature" in filtered-labels' do
       expect(find('.filtered-labels')).not_to have_content "feature"
     end
   end
@@ -191,7 +191,7 @@ feature 'Issue filtering by Labels', feature: true do
       end
     end
 
-    it 'should allow user to remove filtered labels' do
+    it 'allows user to remove filtered labels' do
       first('.js-label-filter-remove').click
       wait_for_ajax
 
@@ -201,7 +201,7 @@ feature 'Issue filtering by Labels', feature: true do
   end
 
   context 'dropdown filtering', js: true do
-    it 'should filter by label name' do
+    it 'filters by label name' do
       page.within '.labels-filter' do
         click_button 'Label'
         wait_for_ajax

spec/features/issues/filter_by_milestone_spec.rb

@@ -15,7 +15,7 @@ feature 'Issue filtering by Milestone', feature: true do
   end
 
   context 'filters by upcoming milestone', js: true do
-    it 'should not show issues with no expiry' do
+    it 'does not show issues with no expiry' do
       create(:issue, project: project)
       create(:issue, project: project, milestone: milestone)
 
@@ -25,7 +25,7 @@ feature 'Issue filtering by Milestone', feature: true do
       expect(page).to have_css('.issue', count: 0)
     end
 
-    it 'should show issues in future' do
+    it 'shows issues in future' do
       milestone = create(:milestone, project: project, due_date: Date.tomorrow)
       create(:issue, project: project)
       create(:issue, project: project, milestone: milestone)
@@ -36,7 +36,7 @@ feature 'Issue filtering by Milestone', feature: true do
       expect(page).to have_css('.issue', count: 1)
     end
 
-    it 'should not show issues in past' do
+    it 'does not show issues in past' do
       milestone = create(:milestone, project: project, due_date: Date.yesterday)
       create(:issue, project: project)
       create(:issue, project: project, milestone: milestone)

spec/features/issues/filter_issues_spec.rb

@@ -26,17 +26,17 @@ describe 'Filter issues', feature: true do
     end
 
     context 'assignee', js: true do
-      it 'should update to current user' do
+      it 'updates to current user' do
         expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name)
       end
 
-      it 'should not change when closed link is clicked' do
+      it 'does not change when closed link is clicked' do
         find('.issues-state-filters a', text: "Closed").click
 
         expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name)
       end
 
-      it 'should not change when all link is clicked' do
+      it 'does not change when all link is clicked' do
         find('.issues-state-filters a', text: "All").click
 
         expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name)
@@ -56,17 +56,17 @@ describe 'Filter issues', feature: true do
     end
 
     context 'milestone', js: true do
-      it 'should update to current milestone' do
+      it 'updates to current milestone' do
         expect(find('.js-milestone-select .dropdown-toggle-text')).to have_content(milestone.title)
       end
 
-      it 'should not change when closed link is clicked' do
+      it 'does not change when closed link is clicked' do
         find('.issues-state-filters a', text: "Closed").click
 
         expect(find('.js-milestone-select .dropdown-toggle-text')).to have_content(milestone.title)
       end
 
-      it 'should not change when all link is clicked' do
+      it 'does not change when all link is clicked' do
         find('.issues-state-filters a', text: "All").click
 
         expect(find('.js-milestone-select .dropdown-toggle-text')).to have_content(milestone.title)
@@ -81,7 +81,7 @@ describe 'Filter issues', feature: true do
       wait_for_ajax
     end
 
-    it 'should filter by any label' do
+    it 'filters by any label' do
       find('.dropdown-menu-labels a', text: 'Any Label').click
       page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click
       wait_for_ajax
@@ -89,7 +89,7 @@ describe 'Filter issues', feature: true do
       expect(find('.labels-filter')).to have_content 'Label'
     end
 
-    it 'should filter by no label' do
+    it 'filters by no label' do
       find('.dropdown-menu-labels a', text: 'No Label').click
       page.first('.labels-filter .dropdown-title .dropdown-menu-close-icon').click
       wait_for_ajax
@@ -100,7 +100,7 @@ describe 'Filter issues', feature: true do
       expect(find('.js-label-select .dropdown-toggle-text')).to have_content('No Label')
     end
 
-    it 'should filter by no label' do
+    it 'filters by no label' do
       find('.dropdown-menu-labels a', text: label.title).click
       page.within '.labels-filter' do
         expect(page).to have_content label.title
@@ -128,19 +128,19 @@ describe 'Filter issues', feature: true do
     end
 
     context 'assignee and label', js: true do
-      it 'should update to current assignee and label' do
+      it 'updates to current assignee and label' do
         expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name)
         expect(find('.js-label-select .dropdown-toggle-text')).to have_content(label.title)
       end
 
-      it 'should not change when closed link is clicked' do
+      it 'does not change when closed link is clicked' do
         find('.issues-state-filters a', text: "Closed").click
 
         expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name)
         expect(find('.js-label-select .dropdown-toggle-text')).to have_content(label.title)
       end
 
-      it 'should not change when all link is clicked' do
+      it 'does not change when all link is clicked' do
         find('.issues-state-filters a', text: "All").click
 
         expect(find('.js-assignee-search .dropdown-toggle-text')).to have_content(user.name)
@@ -168,7 +168,7 @@ describe 'Filter issues', feature: true do
     end
 
     context 'only text', js: true do
-      it 'should filter issues by searched text' do
+      it 'filters issues by searched text' do
         fill_in 'issue_search', with: 'Bug'
 
         page.within '.issues-list' do
@@ -176,7 +176,7 @@ describe 'Filter issues', feature: true do
         end
       end
 
-      it 'should not show any issues' do
+      it 'does not show any issues' do
         fill_in 'issue_search', with: 'testing'
 
         page.within '.issues-list' do
@@ -186,7 +186,7 @@ describe 'Filter issues', feature: true do
     end
 
     context 'text and dropdown options', js: true do
-      it 'should filter by text and label' do
+      it 'filters by text and label' do
         fill_in 'issue_search', with: 'Bug'
 
         page.within '.issues-list' do
@@ -204,7 +204,7 @@ describe 'Filter issues', feature: true do
         end
       end
 
-      it 'should filter by text and milestone' do
+      it 'filters by text and milestone' do
         fill_in 'issue_search', with: 'Bug'
 
         page.within '.issues-list' do
@@ -221,7 +221,7 @@ describe 'Filter issues', feature: true do
         end
       end
 
-      it 'should filter by text and assignee' do
+      it 'filters by text and assignee' do
         fill_in 'issue_search', with: 'Bug'
 
         page.within '.issues-list' do
@@ -238,7 +238,7 @@ describe 'Filter issues', feature: true do
         end
       end
 
-      it 'should filter by text and author' do
+      it 'filters by text and author' do
         fill_in 'issue_search', with: 'Bug'
 
         page.within '.issues-list' do
@@ -269,7 +269,7 @@ describe 'Filter issues', feature: true do
       visit namespace_project_issues_path(project.namespace, project)
     end
 
-    it 'should be able to filter and sort issues' do
+    it 'is able to filter and sort issues' do
       click_button 'Label'
       wait_for_ajax
       page.within '.labels-filter' do

spec/features/issues/issue_sidebar_spec.rb

@@ -17,7 +17,7 @@ feature 'Issue Sidebar', feature: true do
     end
 
     describe 'when clicking on edit labels', js: true do
-      it 'dropdown has an option to create a new label' do
+      it 'shows dropdown option to create a new label' do
         find('.block.labels .edit-link').click
 
         page.within('.block.labels') do
@@ -27,7 +27,7 @@ feature 'Issue Sidebar', feature: true do
     end
 
     context 'creating a new label', js: true do
-      it 'option to crate a new label is present' do
+      it 'shows option to crate a new label is present' do
         page.within('.block.labels') do
           find('.edit-link').click
 
@@ -35,7 +35,7 @@ feature 'Issue Sidebar', feature: true do
         end
       end
 
-      it 'dropdown switches to "create label" section' do
+      it 'shows dropdown switches to "create label" section' do
         page.within('.block.labels') do
           find('.edit-link').click
           click_link 'Create new'
@@ -44,7 +44,7 @@ feature 'Issue Sidebar', feature: true do
         end
       end
 
-      it 'new label is added' do
+      it 'adds new label' do
         page.within('.block.labels') do
           find('.edit-link').click
           sleep 1
@@ -79,7 +79,7 @@ feature 'Issue Sidebar', feature: true do
       visit_issue(project, issue)
     end
 
-    it 'should update weight in sidebar to 1' do
+    it 'updates weight in sidebar to 1' do
       page.within '.weight' do
         click_link 'Edit'
         click_link '1'
@@ -90,7 +90,7 @@ feature 'Issue Sidebar', feature: true do
       end
     end
 
-    it 'should update weight in sidebar to no weight' do
+    it 'updates weight in sidebar to no weight' do
       page.within '.weight' do
         click_link 'Edit'
         click_link 'No Weight'

spec/features/issues/new_branch_button_spec.rb

@@ -41,7 +41,7 @@ feature 'Start new branch from an issue', feature: true do
   end
 
   context "for visiters" do
-    it 'no button is shown', js: true do
+    it 'shows no buttons', js: true do
       visit namespace_project_issue_path(project.namespace, project, issue)
 
       expect(page).not_to have_css('#new-branch')

spec/features/issues/todo_spec.rb

@@ -11,7 +11,7 @@ feature 'Manually create a todo item from issue', feature: true, js: true do
     visit namespace_project_issue_path(project.namespace, project, issue)
   end
 
-  it 'should create todo when clicking button' do
+  it 'creates todo when clicking button' do
     page.within '.issuable-sidebar' do
       click_button 'Add Todo'
       expect(page).to have_content 'Mark Done'
@@ -28,7 +28,7 @@ feature 'Manually create a todo item from issue', feature: true, js: true do
     end
   end
 
-  it 'should mark a todo as done' do
+  it 'marks a todo as done' do
     page.within '.issuable-sidebar' do
       click_button 'Add Todo'
       click_button 'Mark Done'

spec/features/issues/update_issues_spec.rb

@@ -13,7 +13,7 @@ feature 'Multiple issue updating from issues#index', feature: true do
   end
 
   context 'status', js: true do
-    it 'should be set to closed' do
+    it 'sets to closed' do
       visit namespace_project_issues_path(project.namespace, project)
 
       find('#check_all_issues').click
@@ -24,7 +24,7 @@ feature 'Multiple issue updating from issues#index', feature: true do
       expect(page).to have_selector('.issue', count: 0)
     end
 
-    it 'should be set to open' do
+    it 'sets to open' do
       create_closed
       visit namespace_project_issues_path(project.namespace, project, state: 'closed')
 
@@ -38,7 +38,7 @@ feature 'Multiple issue updating from issues#index', feature: true do
   end
 
   context 'assignee', js: true do
-    it 'should update to current user' do
+    it 'updates to current user' do
       visit namespace_project_issues_path(project.namespace, project)
 
       find('#check_all_issues').click
@@ -52,7 +52,7 @@ feature 'Multiple issue updating from issues#index', feature: true do
       end
     end
 
-    it 'should update to unassigned' do
+    it 'updates to unassigned' do
       create_assigned
       visit namespace_project_issues_path(project.namespace, project)
 
@@ -68,7 +68,7 @@ feature 'Multiple issue updating from issues#index', feature: true do
   context 'milestone', js: true do
     let(:milestone)  { create(:milestone, project: project) }
 
-    it 'should update milestone' do
+    it 'updates milestone' do
       visit namespace_project_issues_path(project.namespace, project)
 
       find('#check_all_issues').click
@@ -80,7 +80,7 @@ feature 'Multiple issue updating from issues#index', feature: true do
       expect(find('.issue')).to have_content milestone.title
     end
 
-    it 'should set to no milestone' do
+    it 'sets to no milestone' do
       create_with_milestone
       visit namespace_project_issues_path(project.namespace, project)
 

spec/features/issues_spec.rb

@@ -26,7 +26,7 @@ describe 'Issues', feature: true do
       find('.js-zen-enter').click
     end
 
-    it 'should open new issue popup' do
+    it 'opens new issue popup' do
       expect(page).to have_content("Issue ##{issue.iid}")
     end
 
@@ -71,7 +71,7 @@ describe 'Issues', feature: true do
         visit new_namespace_project_issue_path(project.namespace, project)
       end
 
-      it 'should save with due date' do
+      it 'saves with due date' do
         date = Date.today.at_beginning_of_month
 
         fill_in 'issue_title', with: 'bug 345'
@@ -99,7 +99,7 @@ describe 'Issues', feature: true do
         visit edit_namespace_project_issue_path(project.namespace, project, issue)
       end
 
-      it 'should save with due date' do
+      it 'saves with due date' do
         date = Date.today.at_beginning_of_month
 
         expect(find('#issuable-due-date').value).to eq date.to_s
@@ -155,7 +155,7 @@ describe 'Issues', feature: true do
 
     let(:issue) { @issue }
 
-    it 'should allow filtering by issues with no specified assignee' do
+    it 'allows filtering by issues with no specified assignee' do
       visit namespace_project_issues_path(project.namespace, project, assignee_id: IssuableFinder::NONE)
 
       expect(page).to have_content 'foobar'
@@ -163,7 +163,7 @@ describe 'Issues', feature: true do
       expect(page).not_to have_content 'gitlab'
     end
 
-    it 'should allow filtering by a specified assignee' do
+    it 'allows filtering by a specified assignee' do
       visit namespace_project_issues_path(project.namespace, project, assignee_id: @user.id)
 
       expect(page).not_to have_content 'foobar'
@@ -443,7 +443,7 @@ describe 'Issues', feature: true do
       visit namespace_project_issue_path(project.namespace, project, issue)
     end
 
-    it 'should allow user to update to a weight' do
+    it 'allows user to update to a weight' do
       page.within('.weight') do
         expect(page).to have_content "None"
         click_link 'Edit'
@@ -535,7 +535,7 @@ describe 'Issues', feature: true do
         visit new_namespace_project_issue_path(project.namespace, project)
       end
 
-      it 'should upload file when dragging into textarea' do
+      it 'uploads file when dragging into textarea' do
         drop_in_dropzone test_image_file
 
         # Wait for the file to upload
@@ -583,7 +583,7 @@ describe 'Issues', feature: true do
         visit namespace_project_issue_path(project.namespace, project, issue)
       end
 
-      it 'should add due date to issue' do
+      it 'adds due date to issue' do
         page.within '.due_date' do
           click_link 'Edit'
 
@@ -595,7 +595,7 @@ describe 'Issues', feature: true do
         end
       end
 
-      it 'should remove due date from issue' do
+      it 'removes due date from issue' do
         page.within '.due_date' do
           click_link 'Edit'
 

spec/features/login_spec.rb

@@ -131,7 +131,7 @@ feature 'Login', feature: true do
         expect_any_instance_of(Object).to receive(:omniauth_authorize_path).with(:user, "saml").and_return('/users/auth/saml')
       end
 
-      it 'should show 2FA prompt after OAuth login' do
+      it 'shows 2FA prompt after OAuth login' do
         stub_omniauth_config(enabled: true, auto_link_saml_user: true, allow_single_sign_on: ['saml'], providers: [saml_config])
         user = create(:omniauth_user, :two_factor, extern_uid: 'my-uid', provider: 'saml')
         login_via('saml', user, 'my-uid')

spec/features/merge_requests/award_spec.rb

@@ -11,7 +11,7 @@ feature 'Merge request awards', js: true, feature: true do
       visit namespace_project_merge_request_path(project.namespace, project, merge_request)
     end
 
-    it 'should add award to merge request' do
+    it 'adds award to merge request' do
       first('.js-emoji-btn').click
       expect(page).to have_selector('.js-emoji-btn.active')
       expect(first('.js-emoji-btn')).to have_content '1'
@@ -20,7 +20,7 @@ feature 'Merge request awards', js: true, feature: true do
       expect(first('.js-emoji-btn')).to have_content '1'
     end
 
-    it 'should remove award from merge request' do
+    it 'removes award from merge request' do
       first('.js-emoji-btn').click
       find('.js-emoji-btn.active').click
       expect(first('.js-emoji-btn')).to have_content '0'
@@ -29,7 +29,7 @@ feature 'Merge request awards', js: true, feature: true do
       expect(first('.js-emoji-btn')).to have_content '0'
     end
 
-    it 'should only have one menu on the page' do
+    it 'has only one menu on the page' do
       first('.js-add-award').click
       expect(page).to have_selector('.emoji-menu')
 
@@ -42,7 +42,7 @@ feature 'Merge request awards', js: true, feature: true do
       visit namespace_project_merge_request_path(project.namespace, project, merge_request)
     end
 
-    it 'should not see award menu button' do
+    it 'does not see award menu button' do
       expect(page).not_to have_selector('.js-award-holder')
     end
   end

spec/features/merge_requests/edit_mr_spec.rb

@@ -14,7 +14,7 @@ feature 'Edit Merge Request', feature: true do
       visit edit_namespace_project_merge_request_path(project.namespace, project, merge_request)
     end
 
-    it 'form should have class js-quick-submit' do
+    it 'form has class js-quick-submit' do
       expect(page).to have_selector('.js-quick-submit')
     end
   end

spec/features/merge_requests/filter_by_milestone_spec.rb

@@ -21,7 +21,7 @@ feature 'Merge Request filtering by Milestone', feature: true do
   end
 
   context 'filters by upcoming milestone', js: true do
-    it 'should not show issues with no expiry' do
+    it 'does not show issues with no expiry' do
       create(:merge_request, :with_diffs, source_project: project)
       create(:merge_request, :simple, source_project: project, milestone: milestone)
 
@@ -31,7 +31,7 @@ feature 'Merge Request filtering by Milestone', feature: true do
       expect(page).to have_css('.merge-request', count: 0)
     end
 
-    it 'should show issues in future' do
+    it 'shows issues in future' do
       milestone = create(:milestone, project: project, due_date: Date.tomorrow)
       create(:merge_request, :with_diffs, source_project: project)
       create(:merge_request, :simple, source_project: project, milestone: milestone)
@@ -42,7 +42,7 @@ feature 'Merge Request filtering by Milestone', feature: true do
       expect(page).to have_css('.merge-request', count: 1)
     end
 
-    it 'should not show issues in past' do
+    it 'does not show issues in past' do
       milestone = create(:milestone, project: project, due_date: Date.yesterday)
       create(:merge_request, :with_diffs, source_project: project)
       create(:merge_request, :simple, source_project: project, milestone: milestone)

spec/features/merge_requests/merge_when_build_succeeds_spec.rb

@@ -73,7 +73,7 @@ feature 'Merge When Build Succeeds', feature: true, js: true do
   end
 
   context 'Build is not active' do
-    it "should not allow for enabling" do
+    it "does not allow for enabling" do
       visit_merge_request(merge_request)
       expect(page).not_to have_link "Merge When Build Succeeds"
     end