Merge branch 'ce-to-ee-2018-08-01' into 'master'

CE upstream - 2018-08-01 09:23 UTC

Closes #1756

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

app/controllers/projects/labels_controller.rb

@@ -160,7 +160,10 @@ class Projects::LabelsController < Projects::ApplicationController
 
   def find_labels
     @available_labels ||=
-      LabelsFinder.new(current_user, project_id: @project.id, include_ancestor_groups: params[:include_ancestor_groups]).execute
+      LabelsFinder.new(current_user,
+                       project_id: @project.id,
+                       include_ancestor_groups: params[:include_ancestor_groups],
+                       search: params[:search]).execute
   end
 
   def authorize_admin_labels!

app/finders/labels_finder.rb

@@ -14,6 +14,7 @@ class LabelsFinder < UnionFinder
     @skip_authorization = skip_authorization
     items = find_union(label_ids, Label) || Label.none
     items = with_title(items)
+    items = by_search(items)
     sort(items)
   end
 
@@ -63,6 +64,12 @@ class LabelsFinder < UnionFinder
     items.where(title: title)
   end
 
+  def by_search(labels)
+    return labels unless search?
+
+    labels.search(params[:search])
+  end
+
   # Gets redacted array of group ids
   # which can include the ancestors and descendants of the requested group.
   def group_ids_for(group)
@@ -106,6 +113,10 @@ class LabelsFinder < UnionFinder
     params[:only_group_labels]
   end
 
+  def search?
+    params[:search].present?
+  end
+
   def title
     params[:title] || params[:name]
   end

app/models/concerns/atomic_internal_id.rb

@@ -35,16 +35,21 @@ module AtomicInternalId
 
       define_method("ensure_#{scope}_#{column}!") do
         scope_value = association(scope).reader
+        value = read_attribute(column)
 
-        if read_attribute(column).blank? && scope_value
-          scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value }
-          usage = self.class.table_name.to_sym
+        return value unless scope_value
 
-          new_iid = InternalId.generate_next(self, scope_attrs, usage, init)
-          write_attribute(column, new_iid)
+        scope_attrs = { scope_value.class.table_name.singularize.to_sym => scope_value }
+        usage = self.class.table_name.to_sym
+
+        if value.present?
+          InternalId.track_greatest(self, scope_attrs, usage, value, init)
+        else
+          value = InternalId.generate_next(self, scope_attrs, usage, init)
+          write_attribute(column, value)
         end
 
-        read_attribute(column)
+        value
       end
     end
   end

app/models/internal_id.rb

 # An InternalId is a strictly monotone sequence of integers
 # generated for a given scope and usage.
 #
+# The monotone sequence may be broken if an ID is explicitly provided
+# to `.track_greatest_and_save!` or `#track_greatest`.
+#
 # For example, issues use their project to scope internal ids:
 # In that sense, scope is "project" and usage is "issues".
 # Generated internal ids for an issue are unique per project.
@@ -25,13 +28,34 @@ class InternalId < ActiveRecord::Base
   # The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL).
   # As such, the increment is atomic and safe to be called concurrently.
   def increment_and_save!
+    update_and_save { self.last_value = (last_value || 0) + 1 }
+  end
+
+  # Increments #last_value with new_value if it is greater than the current,
+  # and saves the record
+  #
+  # The operation locks the record and gathers a `ROW SHARE` lock (in PostgreSQL).
+  # As such, the increment is atomic and safe to be called concurrently.
+  def track_greatest_and_save!(new_value)
+    update_and_save { self.last_value = [last_value || 0, new_value].max }
+  end
+
+  private
+
+  def update_and_save(&block)
     lock!
-    self.last_value = (last_value || 0) + 1
+    yield
     save!
     last_value
   end
 
   class << self
+    def track_greatest(subject, scope, usage, new_value, init)
+      return new_value unless available?
+
+      InternalIdGenerator.new(subject, scope, usage, init).track_greatest(new_value)
+    end
+
     def generate_next(subject, scope, usage, init)
       # Shortcut if `internal_ids` table is not available (yet)
       # This can be the case in other (unrelated) migration specs
@@ -94,6 +118,16 @@ class InternalId < ActiveRecord::Base
       end
     end
 
+    # Create a record in internal_ids if one does not yet exist
+    # and set its new_value if it is higher than the current last_value
+    #
+    # Note this will acquire a ROW SHARE lock on the InternalId record
+    def track_greatest(new_value)
+      subject.transaction do
+        (lookup || create_record).track_greatest_and_save!(new_value)
+      end
+    end
+
     private
 
     # Retrieve InternalId record for (project, usage) combination, if it exists

app/models/label.rb

@@ -2,6 +2,7 @@ class Label < ActiveRecord::Base
   include CacheMarkdownField
   include Referable
   include Subscribable
+  include Gitlab::SQL::Pattern
 
   # Represents a "No Label" state used for filtering Issues and Merge
   # Requests that have no label assigned.
@@ -103,6 +104,17 @@ class Label < ActiveRecord::Base
     nil
   end
 
+  # Searches for labels with a matching title or description.
+  #
+  # This method uses ILIKE on PostgreSQL and LIKE on MySQL.
+  #
+  # query - The search query as a String.
+  #
+  # Returns an ActiveRecord::Relation.
+  def self.search(query)
+    fuzzy_search(query, [:title, :description])
+  end
+
   def open_issues_count(user = nil)
     issues_count(user, state: 'opened')
   end

app/views/projects/labels/index.html.haml

@@ -2,32 +2,45 @@
 - page_title "Labels"
 - can_admin_label = can?(current_user, :admin_label, @project)
 - hide_class = ''
+- search = params[:search]
 
 - if can_admin_label
   - content_for(:header_content) do
     .nav-controls
       = link_to _('New label'), new_project_label_path(@project), class: "btn btn-new"
 
-- if @labels.exists? || @prioritized_labels.exists?
+- if @labels.exists? || @prioritized_labels.exists? || search.present?
   #promote-label-modal
   %div{ class: container_class }
     .top-area.adjust
       .nav-text
         = _('Labels can be applied to issues and merge requests.')
-        - if can_admin_label
-          = _('Star a label to make it a priority label. Order the prioritized labels to change their relative priority, by dragging.')
 
-    .labels-container.prepend-top-5
+      .nav-controls
+        = form_tag project_labels_path(@project), method: :get do
+          .input-group
+            = search_field_tag :search, params[:search], { placeholder: _('Filter'), id: 'label-search', class: 'form-control search-text-input input-short', spellcheck: false }
+            %span.input-group-append
+              %button.btn.btn-default{ type: "submit", "aria-label" => _('Submit search') }
+                = icon("search")
+
+    .labels-container.prepend-top-10
       - if can_admin_label
+        - if search.blank?
+          %p.text-muted
+            = _('Star a label to make it a priority label. Order the prioritized labels to change their relative priority, by dragging.')
         -# Only show it in the first page
         - hide = @available_labels.empty? || (params[:page].present? && params[:page] != '1')
         .prioritized-labels{ class: ('hide' if hide) }
           %h5.prepend-top-10= _('Prioritized Labels')
           .content-list.manage-labels-list.js-prioritized-labels{ "data-url" => set_priorities_project_labels_path(@project) }
-            #js-priority-labels-empty-state.priority-labels-empty-state{ class: "#{'hidden' unless @prioritized_labels.empty?}" }
+            #js-priority-labels-empty-state.priority-labels-empty-state{ class: "#{'hidden' unless @prioritized_labels.empty? && search.blank?}" }
               = render 'shared/empty_states/priority_labels'
             - if @prioritized_labels.present?
               = render partial: 'shared/label', subject: @project, collection: @prioritized_labels, as: :label, locals: { force_priority: true }
+            - elsif search.present?
+              .nothing-here-block
+                = _('No prioritised labels with such name or description')
 
       - if @labels.present?
         .other-labels
@@ -36,6 +49,18 @@
           .content-list.manage-labels-list.js-other-labels
             = render partial: 'shared/label', subject: @project, collection: @labels, as: :label
             = paginate @labels, theme: 'gitlab'
+      - elsif search.present?
+        .other-labels
+          - if @available_labels.any?
+            %h5
+              = _('Other Labels')
+            .nothing-here-block
+              = _('No other labels with such name or description')
+          - else
+            .nothing-here-block
+              = _('No labels with such name or description')
+
+
 - else
   = render 'shared/empty_states/labels'
 

changelogs/unreleased/1756-set-iid-via-api.yml

+---
+title: Allow issues API to receive an internal ID (iid) on create
+merge_request: 20626
+author: Jamie Schembri
+type: fixed

changelogs/unreleased/dz-labels-search.yml

+---
+title: Search for labels by title or description on project labels page
+merge_request: 20749
+author:
+type: added

changelogs/unreleased/floating-avarage-commit-numbers.yml

+---
+title: Show one digit after dot in commit_per_day value in charts page.
+merge_request:
+author: msdundar
+type: changed

doc/api/issues.md

@@ -467,6 +467,7 @@ POST /projects/:id/issues
 | Attribute                                 | Type           | Required | Description  |
 |-------------------------------------------|----------------|----------|--------------|
 | `id`                                      | integer/string | yes      | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) owned by the authenticated user |
+| `iid`                                     | integer/string | no       | The internal ID of the project's issue (requires admin or project owner rights) |
 | `title`                                   | string         | yes      | The title of an issue |
 | `description`                             | string         | no       | The description of an issue  |
 | `confidential`                            | boolean        | no       | Set an issue to be confidential. Default is `false`.  |

lib/api/issues.rb

@@ -167,6 +167,9 @@ module API
                                                            desc: 'The IID of a merge request for which to resolve discussions'
         optional :discussion_to_resolve, type: String,
                                          desc: 'The ID of a discussion to resolve, also pass `merge_request_to_resolve_discussions_of`'
+        optional :iid, type: Integer,
+                       desc: 'The internal ID of a project issue. Available only for admins and project owners.'
+
         use :issue_params
       end
       post ':id/issues' do
@@ -174,9 +177,10 @@ module API
 
         authorize! :create_issue, user_project
 
-        # Setting created_at time only allowed for admins and project owners
+        # Setting created_at time or iid only allowed for admins and project owners
         unless current_user.admin? || user_project.owner == current_user
           params.delete(:created_at)
+          params.delete(:iid)
         end
 
         issue_params = declared_params(include_missing: false)

lib/gitlab/graphs/commits.rb

@@ -18,7 +18,7 @@ module Gitlab
       end
 
       def commit_per_day
-        @commit_per_day ||= @commits.size / (@duration + 1)
+        @commit_per_day ||= (@commits.size.to_f / (@duration + 1)).round(1)
       end
 
       def collect_data

locale/gitlab.pot

@@ -2916,6 +2916,9 @@ msgstr ""
 msgid "Fill in the fields below, turn on <strong>%{enable_label}</strong>, and press <strong>%{save_changes}</strong>"
 msgstr ""
 
+msgid "Filter"
+msgstr ""
+
 msgid "Filter by commit message"
 msgstr ""
 
@@ -4366,6 +4369,9 @@ msgstr ""
 msgid "No issues for the selected time period."
 msgstr ""
 
+msgid "No labels with such name or description"
+msgstr ""
+
 msgid "No merge requests for the selected time period."
 msgstr ""
 
@@ -4375,6 +4381,12 @@ msgstr ""
 msgid "No messages were logged"
 msgstr ""
 
+msgid "No other labels with such name or description"
+msgstr ""
+
+msgid "No prioritised labels with such name or description"
+msgstr ""
+
 msgid "No public groups"
 msgstr ""
 
@@ -5987,6 +5999,9 @@ msgstr ""
 msgid "Submit as spam"
 msgstr ""
 
+msgid "Submit search"
+msgstr ""
+
 msgid "Subscribe"
 msgstr ""
 

spec/features/projects/labels/search_labels_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Search for labels', :js do
+  let(:user) { create(:user) }
+  let(:project) { create(:project) }
+  let!(:label1) { create(:label, title: 'Foo', description: 'Lorem ipsum', project: project) }
+  let!(:label2) { create(:label, title: 'Bar', description: 'Fusce consequat', project: project) }
+
+  before do
+    project.add_maintainer(user)
+    sign_in(user)
+
+    visit project_labels_path(project)
+  end
+
+  it 'searches for label by title' do
+    fill_in 'label-search', with: 'Bar'
+    find('#label-search').native.send_keys(:enter)
+
+    expect(page).to have_content(label2.title)
+    expect(page).to have_content(label2.description)
+    expect(page).not_to have_content(label1.title)
+    expect(page).not_to have_content(label1.description)
+  end
+
+  it 'searches for label by title' do
+    fill_in 'label-search', with: 'Lorem'
+    find('#label-search').native.send_keys(:enter)
+
+    expect(page).to have_content(label1.title)
+    expect(page).to have_content(label1.description)
+    expect(page).not_to have_content(label2.title)
+    expect(page).not_to have_content(label2.description)
+  end
+
+  it 'shows nothing found message' do
+    fill_in 'label-search', with: 'nonexistent'
+    find('#label-search').native.send_keys(:enter)
+
+    expect(page).to have_content('No labels with such name or description')
+    expect(page).not_to have_content(label1.title)
+    expect(page).not_to have_content(label1.description)
+    expect(page).not_to have_content(label2.title)
+    expect(page).not_to have_content(label2.description)
+  end
+
+  context 'priority labels' do
+    let!(:label_priority) { create(:label_priority, label: label1, project: project) }
+
+    it 'searches for priority label' do
+      fill_in 'label-search', with: 'Foo'
+      find('#label-search').native.send_keys(:enter)
+
+      page.within('.prioritized-labels') do
+        expect(page).to have_content(label1.title)
+        expect(page).to have_content(label1.description)
+      end
+
+      page.within('.other-labels') do
+        expect(page).to have_content('No other labels with such name or description')
+      end
+    end
+
+    it 'searches for other label' do
+      fill_in 'label-search', with: 'Bar'
+      find('#label-search').native.send_keys(:enter)
+
+      page.within('.prioritized-labels') do
+        expect(page).to have_content('No prioritised labels with such name or description')
+      end
+
+      page.within('.other-labels') do
+        expect(page).to have_content(label2.title)
+        expect(page).to have_content(label2.description)
+      end
+    end
+  end
+end

spec/finders/labels_finder_spec.rb

@@ -14,7 +14,7 @@ describe LabelsFinder do
     let(:project_4) { create(:project, :public) }
     let(:project_5) { create(:project, namespace: group_1) }
 
-    let!(:project_label_1) { create(:label, project: project_1, title: 'Label 1') }
+    let!(:project_label_1) { create(:label, project: project_1, title: 'Label 1', description: 'awesome label') }
     let!(:project_label_2) { create(:label, project: project_2, title: 'Label 2') }
     let!(:project_label_4) { create(:label, project: project_4, title: 'Label 4') }
     let!(:project_label_5) { create(:label, project: project_5, title: 'Label 5') }
@@ -196,5 +196,19 @@ describe LabelsFinder do
         expect(finder.execute).to be_empty
       end
     end
+
+    context 'search by title and description' do
+      it 'returns labels with a partially matching title' do
+        finder = described_class.new(user, search: '(group)')
+
+        expect(finder.execute).to eq [group_label_1]
+      end
+
+      it 'returns labels with a partially matching description' do
+        finder = described_class.new(user, search: 'awesome')
+
+        expect(finder.execute).to eq [project_label_1]
+      end
+    end
   end
 end

spec/lib/gitlab/graphs/commits_spec.rb

@@ -29,7 +29,7 @@ describe Gitlab::Graphs::Commits do
   context 'with commits from yesterday and today' do
     subject { described_class.new([commit2, commit1_yesterday]) }
     describe '#commit_per_day' do
-      it { expect(subject.commit_per_day).to eq 1 }
+      it { expect(subject.commit_per_day).to eq 1.0 }
     end
 
     describe '#duration' do

spec/models/internal_id_spec.rb

@@ -79,6 +79,46 @@ describe InternalId do
     end
   end
 
+  describe '.track_greatest' do
+    let(:value) { 9001 }
+    subject { described_class.track_greatest(issue, scope, usage, value, init) }
+
+    context 'in the absence of a record' do
+      it 'creates a record if not yet present' do
+        expect { subject }.to change { described_class.count }.from(0).to(1)
+      end
+    end
+
+    it 'stores record attributes' do
+      subject
+
+      described_class.first.tap do |record|
+        expect(record.project).to eq(project)
+        expect(record.usage).to eq(usage.to_s)
+        expect(record.last_value).to eq(value)
+      end
+    end
+
+    context 'with existing issues' do
+      before do
+        create(:issue, project: project)
+        described_class.delete_all
+      end
+
+      it 'still returns the last value to that of the given value' do
+        expect(subject).to eq(value)
+      end
+    end
+
+    context 'when value is less than the current last_value' do
+      it 'returns the current last_value' do
+        described_class.create!(**scope, usage: usage, last_value: 10_001)
+
+        expect(subject).to eq 10_001
+      end
+    end
+  end
+
   describe '#increment_and_save!' do
     let(:id) { create(:internal_id) }
     subject { id.increment_and_save! }
@@ -103,4 +143,30 @@ describe InternalId do
       end
     end
   end
+
+  describe '#track_greatest_and_save!' do
+    let(:id) { create(:internal_id) }
+    let(:new_last_value) { 9001 }
+    subject { id.track_greatest_and_save!(new_last_value) }
+
+    it 'returns new last value' do
+      expect(subject).to eq new_last_value
+    end
+
+    it 'saves the record' do
+      subject
+
+      expect(id.changed?).to be_falsey
+    end
+
+    context 'when new last value is lower than the max' do
+      it 'does not update the last value' do
+        id.update!(last_value: 10_001)
+
+        subject
+
+        expect(id.reload.last_value).to eq 10_001
+      end
+    end
+  end
 end

spec/models/label_spec.rb

@@ -139,4 +139,20 @@ describe Label do
       end
     end
   end
+
+  describe '.search' do
+    let(:label) { create(:label, title: 'bug', description: 'incorrect behavior') }
+
+    it 'returns labels with a partially matching title' do
+      expect(described_class.search(label.title[0..2])).to eq([label])
+    end
+
+    it 'returns labels with a partially matching description' do
+      expect(described_class.search(label.description[0..5])).to eq([label])
+    end
+
+    it 'returns nothing' do
+      expect(described_class.search('feature')).to be_empty
+    end
+  end
 end

spec/requests/api/issues_spec.rb

@@ -1006,6 +1006,38 @@ describe API::Issues do
       end
     end
 
+    context 'an internal ID is provided' do
+      context 'by an admin' do
+        it 'sets the internal ID on the new issue' do
+          post api("/projects/#{project.id}/issues", admin),
+            title: 'new issue', iid: 9001
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(json_response['iid']).to eq 9001
+        end
+      end
+
+      context 'by an owner' do
+        it 'sets the internal ID on the new issue' do
+          post api("/projects/#{project.id}/issues", user),
+            title: 'new issue', iid: 9001
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(json_response['iid']).to eq 9001
+        end
+      end
+
+      context 'by another user' do
+        it 'ignores the given internal ID' do
+          post api("/projects/#{project.id}/issues", user2),
+            title: 'new issue', iid: 9001
+
+          expect(response).to have_gitlab_http_status(201)
+          expect(json_response['iid']).not_to eq 9001
+        end
+      end
+    end
+
     it 'creates a new project issue' do
       post api("/projects/#{project.id}/issues", user),
         title: 'new issue', labels: 'label, label2', weight: 3,

spec/support/shared_examples/gitlab_projects_import_service_shared_examples.rb

+shared_examples 'gitlab projects import validations' do
+  context 'with an invalid path' do
+    let(:path) { '/invalid-path/' }
+
+    it 'returns an invalid project' do
+      project = subject.execute
+
+      expect(project).not_to be_persisted
+      expect(project).not_to be_valid
+    end
+  end
+
+  context 'with a valid path' do
+    it 'creates a project' do
+      project = subject.execute
+
+      expect(project).to be_persisted
+      expect(project).to be_valid
+    end
+  end
+
+  context 'override params' do
+    it 'stores them as import data when passed' do
+      project = described_class
+                  .new(namespace.owner, import_params, description: 'Hello')
+                  .execute
+
+      expect(project.import_data.data['override_params']['description']).to eq('Hello')
+    end
+  end
+
+  context 'when there is a project with the same path' do
+    let(:existing_project) { create(:project, namespace: namespace) }
+    let(:path) { existing_project.path}
+
+    it 'does not create the project' do
+      project = subject.execute
+
+      expect(project).to be_invalid
+      expect(project).not_to be_persisted
+    end
+
+    context 'when overwrite param is set' do
+      let(:overwrite) { true }
+
+      it 'creates a project in a temporary full_path' do
+        project = subject.execute
+
+        expect(project).to be_valid
+        expect(project).to be_persisted
+      end
+    end
+  end
+end

spec/support/shared_examples/models/atomic_internal_id_spec.rb

@@ -60,6 +60,20 @@ shared_examples_for 'AtomicInternalId' do |validate_presence: true|
 
         expect { subject }.not_to change { instance.public_send(internal_id_attribute) }
       end
+
+      context 'when the instance has an internal ID set' do
+        let(:internal_id) { 9001 }
+
+        it 'calls InternalId.update_last_value and sets the `last_value` to that of the instance' do
+          instance.send("#{internal_id_attribute}=", internal_id)
+
+          expect(InternalId)
+            .to receive(:track_greatest)
+            .with(instance, scope_attrs, usage, internal_id, any_args)
+            .and_return(internal_id)
+          subject
+        end
+      end
     end
   end
 end