diff options
29 files changed, 245 insertions, 20 deletions
diff --git a/app/assets/stylesheets/framework/filters.scss b/app/assets/stylesheets/framework/filters.scss index a52e6c4f6a7..e9b074236cc 100644 --- a/app/assets/stylesheets/framework/filters.scss +++ b/app/assets/stylesheets/framework/filters.scss @@ -92,8 +92,8 @@ display: -webkit-flex; display: flex; flex-shrink: 0; - margin-top: 5px; - margin-bottom: 5px; + margin-top: 4px; + margin-bottom: 4px; .selectable { display: -webkit-flex; diff --git a/app/controllers/groups/labels_controller.rb b/app/controllers/groups/labels_controller.rb index e95123c0933..059cf160fa2 100644 --- a/app/controllers/groups/labels_controller.rb +++ b/app/controllers/groups/labels_controller.rb @@ -12,6 +12,7 @@ class Groups::LabelsController < Groups::ApplicationController format.html do @labels = @group.labels .optionally_search(params[:search]) + .order_by(sort) .page(params[:page]) end format.json do @@ -117,4 +118,8 @@ class Groups::LabelsController < Groups::ApplicationController include_descendant_groups: params[:include_descendant_groups], search: params[:search]).execute end + + def sort + @sort ||= params[:sort] || 'name_asc' + end end diff --git a/app/controllers/projects/labels_controller.rb b/app/controllers/projects/labels_controller.rb index 8a2bce6e7b5..69332ee2a0e 100644 --- a/app/controllers/projects/labels_controller.rb +++ b/app/controllers/projects/labels_controller.rb @@ -163,7 +163,12 @@ class Projects::LabelsController < Projects::ApplicationController LabelsFinder.new(current_user, project_id: @project.id, include_ancestor_groups: params[:include_ancestor_groups], - search: params[:search]).execute + search: params[:search], + sort: sort).execute + end + + def sort + @sort ||= params[:sort] || 'name_asc' end def authorize_admin_labels! diff --git a/app/finders/labels_finder.rb b/app/finders/labels_finder.rb index 1d05bf28438..8418577dab2 100644 --- a/app/finders/labels_finder.rb +++ b/app/finders/labels_finder.rb @@ -54,7 +54,11 @@ class LabelsFinder < UnionFinder end def sort(items) - items.reorder(title: :asc) + if params[:sort] + items.order_by(params[:sort]) + else + items.reorder(title: :asc) + end end def with_title(items) diff --git a/app/helpers/sorting_helper.rb b/app/helpers/sorting_helper.rb index 731b6806b5f..a6e65d30eda 100644 --- a/app/helpers/sorting_helper.rb +++ b/app/helpers/sorting_helper.rb @@ -101,6 +101,17 @@ module SortingHelper } end + def label_sort_options_hash + { + sort_value_name => sort_title_name, + sort_value_name_desc => sort_title_name_desc, + sort_value_recently_created => sort_title_recently_created, + sort_value_oldest_created => sort_title_oldest_created, + sort_value_recently_updated => sort_title_recently_updated, + sort_value_oldest_updated => sort_title_oldest_updated + } + end + def sortable_item(item, path, sorted_by) link_to item, path, class: sorted_by == item ? 'is-active' : '' end diff --git a/app/models/label.rb b/app/models/label.rb index 8db7c3abd10..8dc7ded53ad 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -6,6 +6,7 @@ class Label < ActiveRecord::Base include Subscribable include Gitlab::SQL::Pattern include OptionallySearch + include Sortable # Represents a "No Label" state used for filtering Issues and Merge # Requests that have no label assigned. @@ -41,6 +42,8 @@ class Label < ActiveRecord::Base scope :with_lists_and_board, -> { joins(lists: :board).merge(List.movable) } scope :on_group_boards, ->(group_id) { with_lists_and_board.where(boards: { group_id: group_id }) } scope :on_project_boards, ->(project_id) { with_lists_and_board.where(boards: { project_id: project_id }) } + scope :order_name_asc, -> { reorder(title: :asc) } + scope :order_name_desc, -> { reorder(title: :desc) } def self.prioritized(project) joins(:priorities) diff --git a/app/services/emails/base_service.rb b/app/services/emails/base_service.rb index ba7b689a9af..988215ffc78 100644 --- a/app/services/emails/base_service.rb +++ b/app/services/emails/base_service.rb @@ -2,6 +2,8 @@ module Emails class BaseService + attr_reader :current_user + def initialize(current_user, params = {}) @current_user, @params = current_user, params.dup @user = params.delete(:user) diff --git a/app/services/emails/create_service.rb b/app/services/emails/create_service.rb index acf575e24e5..56925a724fe 100644 --- a/app/services/emails/create_service.rb +++ b/app/services/emails/create_service.rb @@ -3,7 +3,12 @@ module Emails class CreateService < ::Emails::BaseService def execute(extra_params = {}) - @user.emails.create(@params.merge(extra_params)) + skip_confirmation = @params.delete(:skip_confirmation) + + email = @user.emails.create(@params.merge(extra_params)) + + email&.confirm if skip_confirmation && current_user.admin? + email end end end diff --git a/app/uploaders/avatar_uploader.rb b/app/uploaders/avatar_uploader.rb index 8526bc16390..c0165759203 100644 --- a/app/uploaders/avatar_uploader.rb +++ b/app/uploaders/avatar_uploader.rb @@ -19,7 +19,7 @@ class AvatarUploader < GitlabUploader end def absolute_path - self.class.absolute_path(model.avatar) + self.class.absolute_path(model.avatar.upload) end private diff --git a/app/uploaders/namespace_file_uploader.rb b/app/uploaders/namespace_file_uploader.rb index b0154f85a5c..4965bd7f057 100644 --- a/app/uploaders/namespace_file_uploader.rb +++ b/app/uploaders/namespace_file_uploader.rb @@ -21,6 +21,10 @@ class NamespaceFileUploader < FileUploader File.join(model.id.to_s) end + def self.workhorse_local_upload_path + File.join(options.storage_path, 'uploads', TMP_UPLOAD_PATH) + end + # Re-Override def store_dir store_dirs[object_store] diff --git a/app/views/groups/labels/index.html.haml b/app/views/groups/labels/index.html.haml index e6821009d03..86178eb2ffd 100644 --- a/app/views/groups/labels/index.html.haml +++ b/app/views/groups/labels/index.html.haml @@ -22,6 +22,7 @@ %span.input-group-append %button.btn.btn-default{ type: "submit", "aria-label" => _('Submit search') } = icon("search") + = render 'shared/labels/sort_dropdown' .labels-container.prepend-top-5 - if @labels.any? diff --git a/app/views/projects/labels/index.html.haml b/app/views/projects/labels/index.html.haml index dfac62e7985..1bfd8a85f0f 100644 --- a/app/views/projects/labels/index.html.haml +++ b/app/views/projects/labels/index.html.haml @@ -22,6 +22,7 @@ %span.input-group-append %button.btn.btn-default{ type: "submit", "aria-label" => _('Submit search') } = icon("search") + = render 'shared/labels/sort_dropdown' .labels-container.prepend-top-10 - if can_admin_label diff --git a/app/views/shared/labels/_sort_dropdown.html.haml b/app/views/shared/labels/_sort_dropdown.html.haml new file mode 100644 index 00000000000..ff6e2947ffd --- /dev/null +++ b/app/views/shared/labels/_sort_dropdown.html.haml @@ -0,0 +1,9 @@ +- sort_title = label_sort_options_hash[@sort] || sort_title_name_desc +.dropdown.inline + %button.dropdown-toggle{ type: 'button', data: { toggle: 'dropdown' } } + = sort_title + = icon('chevron-down') + %ul.dropdown-menu.dropdown-menu-right.dropdown-menu-sort + %li + - label_sort_options_hash.each do |value, title| + = sortable_item(title, page_filter_path(sort: value, label: true), sort_title) diff --git a/changelogs/unreleased/49943-resolve-filter-bar-height-changes.yml b/changelogs/unreleased/49943-resolve-filter-bar-height-changes.yml new file mode 100644 index 00000000000..aa19b816b0b --- /dev/null +++ b/changelogs/unreleased/49943-resolve-filter-bar-height-changes.yml @@ -0,0 +1,5 @@ +--- +title: Fix filter bar height bug when a tag is added +merge_request: +author: +type: fixed diff --git a/changelogs/unreleased/50835-add-filtering-sorting-for-labels-on-labels-page.yml b/changelogs/unreleased/50835-add-filtering-sorting-for-labels-on-labels-page.yml new file mode 100644 index 00000000000..24e231ed88a --- /dev/null +++ b/changelogs/unreleased/50835-add-filtering-sorting-for-labels-on-labels-page.yml @@ -0,0 +1,5 @@ +--- +title: Add sorting for labels on labels page +merge_request: 21642 +author: +type: added diff --git a/changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml b/changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml new file mode 100644 index 00000000000..c0f7e88f2b7 --- /dev/null +++ b/changelogs/unreleased/51318-project-export-broken-when-avatar-is-set.yml @@ -0,0 +1,5 @@ +--- +title: Fix broken exports when they include a projet avatar +merge_request: 21649 +author: +type: fixed diff --git a/changelogs/unreleased/fix-namespace-upload.yml b/changelogs/unreleased/fix-namespace-upload.yml new file mode 100644 index 00000000000..383d79a998f --- /dev/null +++ b/changelogs/unreleased/fix-namespace-upload.yml @@ -0,0 +1,5 @@ +--- +title: Fix workhorse temp path for namespace uploads +merge_request: 21650 +author: +type: fixed diff --git a/changelogs/unreleased/sh-support-adding-confirmed-emails.yml b/changelogs/unreleased/sh-support-adding-confirmed-emails.yml new file mode 100644 index 00000000000..1b64a1c62dc --- /dev/null +++ b/changelogs/unreleased/sh-support-adding-confirmed-emails.yml @@ -0,0 +1,5 @@ +--- +title: Add ability to skip user email confirmation with API +merge_request: 21630 +author: +type: added diff --git a/db/importers/common_metrics_importer.rb b/db/importers/common_metrics_importer.rb index 01fbbd6866b..6302394d7a6 100644 --- a/db/importers/common_metrics_importer.rb +++ b/db/importers/common_metrics_importer.rb @@ -35,8 +35,8 @@ module Importers attr_reader :content - def initialize(file = 'config/prometheus/common_metrics.yml') - @content = YAML.load_file(file) + def initialize(filename = 'common_metrics.yml') + @content = YAML.load_file(Rails.root.join('config', 'prometheus', filename)) end def execute diff --git a/doc/api/users.md b/doc/api/users.md index a8858468cab..51935280401 100644 --- a/doc/api/users.md +++ b/doc/api/users.md @@ -972,6 +972,7 @@ Parameters: - `id` (required) - id of specified user - `email` (required) - email address +- `skip_confirmation` (optional) - Skip confirmation and assume e-mail is verified - true or false (default) ## Delete email for current user diff --git a/lib/api/users.rb b/lib/api/users.rb index b0811bb4aad..a4ae597e252 100644 --- a/lib/api/users.rb +++ b/lib/api/users.rb @@ -361,6 +361,7 @@ module API params do requires :id, type: Integer, desc: 'The ID of the user' requires :email, type: String, desc: 'The email of the user' + optional :skip_confirmation, type: Boolean, desc: 'Skip confirmation of email and assume it is verified' end post ":id/emails" do authenticated_as_admin! diff --git a/lib/gitlab/import_export/avatar_restorer.rb b/lib/gitlab/import_export/avatar_restorer.rb index ded05f73cf8..17796430811 100644 --- a/lib/gitlab/import_export/avatar_restorer.rb +++ b/lib/gitlab/import_export/avatar_restorer.rb @@ -19,7 +19,7 @@ module Gitlab private def avatar_export_file - @avatar_export_file ||= Dir["#{avatar_export_path}/**/*"].first + @avatar_export_file ||= Dir["#{avatar_export_path}/**/*"].find { |f| File.file?(f) } end def avatar_export_path diff --git a/spec/db/importers/common_metrics_importer_spec.rb b/spec/db/importers/common_metrics_importer_spec.rb index 16b59e1dfe8..68260820958 100644 --- a/spec/db/importers/common_metrics_importer_spec.rb +++ b/spec/db/importers/common_metrics_importer_spec.rb @@ -47,6 +47,16 @@ describe Importers::CommonMetricsImporter do end end + context "does import common_metrics.yml" do + it "when executed from outside of the Rails.root" do + Dir.chdir(Dir.tmpdir) do + expect { subject.execute }.not_to raise_error + end + + expect(PrometheusMetric.common).not_to be_empty + end + end + context 'does import properly all fields' do let(:query_identifier) { 'response-metric' } let(:group) do diff --git a/spec/features/groups/labels/sort_labels_spec.rb b/spec/features/groups/labels/sort_labels_spec.rb new file mode 100644 index 00000000000..2aea4d77675 --- /dev/null +++ b/spec/features/groups/labels/sort_labels_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Sort labels', :js do + let(:user) { create(:user) } + let(:group) { create(:group) } + let!(:label1) { create(:group_label, title: 'Foo', description: 'Lorem ipsum', group: group) } + let!(:label2) { create(:group_label, title: 'Bar', description: 'Fusce consequat', group: group) } + + before do + group.add_maintainer(user) + sign_in(user) + + visit group_labels_path(group) + end + + it 'sorts by title by default' do + expect(page).to have_button('Name') + + # assert default sorting + within '.other-labels' do + expect(page.all('.label-list-item').first.text).to include('Bar') + expect(page.all('.label-list-item').last.text).to include('Foo') + end + end + + it 'sorts by date' do + click_button 'Name' + + sort_options = find('ul.dropdown-menu-sort li').all('a').collect(&:text) + + expect(sort_options[0]).to eq('Name') + expect(sort_options[1]).to eq('Name, descending') + expect(sort_options[2]).to eq('Last created') + expect(sort_options[3]).to eq('Oldest created') + expect(sort_options[4]).to eq('Last updated') + expect(sort_options[5]).to eq('Oldest updated') + + click_link 'Name, descending' + + # assert default sorting + within '.other-labels' do + expect(page.all('.label-list-item').first.text).to include('Foo') + expect(page.all('.label-list-item').last.text).to include('Bar') + end + end +end diff --git a/spec/features/projects/labels/sort_labels_spec.rb b/spec/features/projects/labels/sort_labels_spec.rb new file mode 100644 index 00000000000..01c3f251173 --- /dev/null +++ b/spec/features/projects/labels/sort_labels_spec.rb @@ -0,0 +1,48 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe 'Sort 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 'sorts by title by default' do + expect(page).to have_button('Name') + + # assert default sorting + within '.other-labels' do + expect(page.all('.label-list-item').first.text).to include('Bar') + expect(page.all('.label-list-item').last.text).to include('Foo') + end + end + + it 'sorts by date' do + click_button 'Name' + + sort_options = find('ul.dropdown-menu-sort li').all('a').collect(&:text) + + expect(sort_options[0]).to eq('Name') + expect(sort_options[1]).to eq('Name, descending') + expect(sort_options[2]).to eq('Last created') + expect(sort_options[3]).to eq('Oldest created') + expect(sort_options[4]).to eq('Last updated') + expect(sort_options[5]).to eq('Oldest updated') + + click_link 'Name, descending' + + # assert default sorting + within '.other-labels' do + expect(page.all('.label-list-item').first.text).to include('Foo') + expect(page.all('.label-list-item').last.text).to include('Bar') + end + end +end diff --git a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb index 4897d604bc1..e44ff6bbcbd 100644 --- a/spec/lib/gitlab/import_export/avatar_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/avatar_restorer_spec.rb @@ -6,22 +6,35 @@ describe Gitlab::ImportExport::AvatarRestorer do let(:shared) { project.import_export_shared } let(:project) { create(:project) } - before do - allow_any_instance_of(described_class).to receive(:avatar_export_file) - .and_return(uploaded_image_temp_path) - end - after do project.remove_avatar! end - it 'restores a project avatar' do - expect(described_class.new(project: project, shared: shared).restore).to be true + context 'with avatar' do + before do + allow_any_instance_of(described_class).to receive(:avatar_export_file) + .and_return(uploaded_image_temp_path) + end + + it 'restores a project avatar' do + expect(described_class.new(project: project, shared: shared).restore).to be true + end + + it 'saves the avatar into the project' do + described_class.new(project: project, shared: shared).restore + + expect(project.reload.avatar.file.exists?).to be true + end end - it 'saves the avatar into the project' do - described_class.new(project: project, shared: shared).restore + it 'does not break if there is just a directory' do + Dir.mktmpdir do |tmpdir| + FileUtils.mkdir_p("#{tmpdir}/a/b") + + allow_any_instance_of(described_class).to receive(:avatar_export_path) + .and_return("#{tmpdir}/a") - expect(project.reload.avatar.file.exists?).to be true + expect(described_class.new(project: project, shared: shared).restore).to be true + end end end diff --git a/spec/requests/api/users_spec.rb b/spec/requests/api/users_spec.rb index d48d577afa1..b7d62df0663 100644 --- a/spec/requests/api/users_spec.rb +++ b/spec/requests/api/users_spec.rb @@ -1031,11 +1031,14 @@ describe API::Users do expect(json_response['error']).to eq('email is missing') end - it "creates email" do + it "creates unverified email" do email_attrs = attributes_for :email expect do post api("/users/#{user.id}/emails", admin), email_attrs end.to change { user.emails.count }.by(1) + + email = Email.find_by(user_id: user.id, email: email_attrs[:email]) + expect(email).not_to be_confirmed end it "returns a 400 for invalid ID" do @@ -1043,6 +1046,18 @@ describe API::Users do expect(response).to have_gitlab_http_status(400) end + + it "creates verified email" do + email_attrs = attributes_for :email + email_attrs[:skip_confirmation] = true + + post api("/users/#{user.id}/emails", admin), email_attrs + + expect(response).to have_gitlab_http_status(201) + + email = Email.find_by(user_id: user.id, email: email_attrs[:email]) + expect(email).to be_confirmed + end end describe 'GET /user/:id/emails' do diff --git a/spec/uploaders/avatar_uploader_spec.rb b/spec/uploaders/avatar_uploader_spec.rb index b0468bc35ff..6aaec7a4fef 100644 --- a/spec/uploaders/avatar_uploader_spec.rb +++ b/spec/uploaders/avatar_uploader_spec.rb @@ -35,5 +35,13 @@ describe AvatarUploader do it_behaves_like "migrates", to_store: described_class::Store::REMOTE it_behaves_like "migrates", from_store: described_class::Store::REMOTE, to_store: described_class::Store::LOCAL + + it 'sets the right absolute path' do + storage_path = Gitlab.config.uploads.storage_path + absolute_path = File.join(storage_path, upload.path) + + expect(uploader.absolute_path.scan(storage_path).size).to eq(1) + expect(uploader.absolute_path).to eq(absolute_path) + end end end diff --git a/spec/uploaders/namespace_file_uploader_spec.rb b/spec/uploaders/namespace_file_uploader_spec.rb index eafbea07e10..799c6db57fa 100644 --- a/spec/uploaders/namespace_file_uploader_spec.rb +++ b/spec/uploaders/namespace_file_uploader_spec.rb @@ -40,6 +40,12 @@ describe NamespaceFileUploader do end end + describe '#workhorse_local_upload_path' do + it 'returns the correct path in uploads directory' do + expect(described_class.workhorse_local_upload_path).to end_with('/uploads/tmp/uploads') + end + end + describe "#migrate!" do before do uploader.store!(fixture_file_upload(File.join('spec/fixtures/doc_sample.txt'))) |