diff --git a/app/finders/issuable_finder.rb b/app/finders/issuable_finder.rb index 37151f8d1348358674aa0915bdeea56970c60b77..6f2adf47c3a94c94e2b901a85a6f6077ee3596a8 100644 --- a/app/finders/issuable_finder.rb +++ b/app/finders/issuable_finder.rb @@ -128,7 +128,8 @@ class IssuableFinder @labels = Label.where(title: label_names) if projects - @labels = @labels.where(project: projects) + label_ids = LabelsFinder.new(current_user, project_id: projects).execute.select(:id) + @labels = @labels.where(labels: { id: label_ids }) end else @labels = Label.none diff --git a/app/helpers/labels_helper.rb b/app/helpers/labels_helper.rb index c14caa5e387207a2ddc9d7dfb9570b0503969210..844bd3fd183ff983af2bc6e3c354527f1383be5a 100644 --- a/app/helpers/labels_helper.rb +++ b/app/helpers/labels_helper.rb @@ -57,21 +57,21 @@ module LabelsHelper def edit_label_path(label) case label when GroupLabel then edit_group_label_path(label.group, label) - else edit_namespace_project_label_path(label.project.namespace, label.project, label) + when ProjectLabel then edit_namespace_project_label_path(label.project.namespace, label.project, label) end end def destroy_label_path(label) case label when GroupLabel then group_label_path(label.group, label) - else namespace_project_label_path(label.project.namespace, label.project, label) + when ProjectLabel then namespace_project_label_path(label.project.namespace, label.project, label) end end def toggle_subscription_label_path(label) case label when GroupLabel then toggle_subscription_group_label_path(label.group, label) - else toggle_subscription_namespace_project_label_path(label.project.namespace, label.project, label) + when ProjectLabel then toggle_subscription_namespace_project_label_path(label.project.namespace, label.project, label) end end @@ -79,7 +79,7 @@ module LabelsHelper title, icon = case label when GroupLabel then ['Group', 'folder-open'] - else ['Project', 'bookmark'] + when ProjectLabel then ['Project', 'bookmark'] end options[:class] ||= '' diff --git a/app/models/label.rb b/app/models/label.rb index be0c20479d56a20c0ce9b9c0f4f2bb09630845ed..0a68be7a30f6369a8b489555f6e332543d4b6d00 100644 --- a/app/models/label.rb +++ b/app/models/label.rb @@ -15,15 +15,12 @@ class Label < ActiveRecord::Base default_value_for :color, DEFAULT_COLOR - belongs_to :project - has_many :lists, dependent: :destroy has_many :label_links, dependent: :destroy has_many :issues, through: :label_links, source: :target, source_type: 'Issue' has_many :merge_requests, through: :label_links, source: :target, source_type: 'MergeRequest' validates :color, color: true, allow_blank: false - validates :project, presence: true, if: :project_label? # Don't allow ',' for label titles validates :title, presence: true, format: { with: /\A[^,]+\z/ } @@ -137,10 +134,6 @@ class Label < ActiveRecord::Base .count end - def project_label? - type.blank? && !template? - end - def label_format_reference(format = :id) raise StandardError, 'Unknown format' unless [:id, :name].include?(format) diff --git a/app/models/project.rb b/app/models/project.rb index db7301219e5a25cde0aa4bca2025f24b36fc0b66..411252230443bdc2d7ee4e3a1b4bb444cad195e0 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -107,7 +107,7 @@ class Project < ActiveRecord::Base # Merge requests from source project should be kept when source project was removed has_many :fork_merge_requests, foreign_key: 'source_project_id', class_name: MergeRequest has_many :issues, dependent: :destroy - has_many :labels, dependent: :destroy + has_many :labels, dependent: :destroy, class_name: 'ProjectLabel' has_many :services, dependent: :destroy has_many :events, dependent: :destroy has_many :milestones, dependent: :destroy @@ -730,8 +730,10 @@ class Project < ActiveRecord::Base def create_labels Label.templates.each do |label| label = label.dup - label.template = nil + label.template = false label.project_id = self.id + label.type = 'ProjectLabel' + label.save end end diff --git a/app/models/project_label.rb b/app/models/project_label.rb new file mode 100644 index 0000000000000000000000000000000000000000..3e41113e3402152388c3dec44c7fd462e660c075 --- /dev/null +++ b/app/models/project_label.rb @@ -0,0 +1,5 @@ +class ProjectLabel < Label + belongs_to :project + + validates :project, presence: true +end diff --git a/app/policies/label_policy.rb b/app/policies/project_label_policy.rb similarity index 64% rename from app/policies/label_policy.rb rename to app/policies/project_label_policy.rb index 1677ad7f1bb0a9a9477f1ea70ac1785fc7f8b024..e7bd58372a65a933816394e856d0fc999f45017f 100644 --- a/app/policies/label_policy.rb +++ b/app/policies/project_label_policy.rb @@ -1,7 +1,5 @@ -class LabelPolicy < BasePolicy +class ProjectLabelPolicy < BasePolicy def rules - return unless @user - can! :admin_label if Ability.allowed?(@user, :admin_label, @subject.project) end end diff --git a/app/views/projects/labels/_form.html.haml b/app/views/projects/labels/_form.html.haml index 5f7be074f259bf3f7e5fc701b7ef3fe1b4e54c06..28a062c7eb5363ac217dfd40d569d39c4b203fbc 100644 --- a/app/views/projects/labels/_form.html.haml +++ b/app/views/projects/labels/_form.html.haml @@ -1,4 +1,4 @@ -= form_for [@project.namespace.becomes(Namespace), @project, @label], html: { class: 'form-horizontal label-form js-quick-submit js-requires-input' } do |f| += form_for @label, as: :label, url: url, html: { class: 'form-horizontal label-form js-quick-submit js-requires-input' } do |f| = form_errors(@label) .form-group diff --git a/app/views/projects/labels/edit.html.haml b/app/views/projects/labels/edit.html.haml index c9ec371c3e120488c67842cdae1a5ce8c9067b0d..372abcb877386e3d05b090690acc9e0c5792e7a7 100644 --- a/app/views/projects/labels/edit.html.haml +++ b/app/views/projects/labels/edit.html.haml @@ -7,4 +7,4 @@ = icon('bookmark') Edit Project Label %hr - = render 'form' + = render 'form', url: namespace_project_label_path(@project.namespace.becomes(Namespace), @project, @label) diff --git a/app/views/projects/labels/new.html.haml b/app/views/projects/labels/new.html.haml index a1e2df6c55df827e79e477fb2cb9335a2a8b6843..f170c41bfc4a68ed3588619d4b499986ca3feb41 100644 --- a/app/views/projects/labels/new.html.haml +++ b/app/views/projects/labels/new.html.haml @@ -7,4 +7,4 @@ = icon('bookmark') New Project Label %hr - = render 'form' + = render 'form', url: namespace_project_labels_path(@project.namespace.becomes(Namespace), @project) diff --git a/db/migrate/20160920191518_set_project_label_type_on_labels.rb b/db/migrate/20160920191518_set_project_label_type_on_labels.rb new file mode 100644 index 0000000000000000000000000000000000000000..af47d0320e231d677c5751d1ea9076a97d8cf85d --- /dev/null +++ b/db/migrate/20160920191518_set_project_label_type_on_labels.rb @@ -0,0 +1,17 @@ +class SetProjectLabelTypeOnLabels < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def up + update_column_in_batches(:labels, :type, 'ProjectLabel') do |table, query| + query.where(table[:project_id].not_eq(nil)) + end + end + + def down + update_column_in_batches(:labels, :type, nil) do |table, query| + query.where(table[:project_id].not_eq(nil)) + end + end +end diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb index 3a09912f1be5694a012080b7fbca035a627d57b0..4c4784b005274b6e49a32f54ed6c78683a83293a 100644 --- a/lib/banzai/filter/label_reference_filter.rb +++ b/lib/banzai/filter/label_reference_filter.rb @@ -70,7 +70,7 @@ module Banzai end def object_link_text(object, matches) - if object.project.nil? || object.project == context[:project] + if object.is_a?(GroupLabel) || object.project == context[:project] LabelsHelper.render_colored_label(object) else LabelsHelper.render_colored_cross_project_label(object) diff --git a/lib/gitlab/google_code_import/importer.rb b/lib/gitlab/google_code_import/importer.rb index 62da327931faff96264b4152325c321bc7f16281..ef8c3e35619c6646dd8d5cb912aacc8f13dc625a 100644 --- a/lib/gitlab/google_code_import/importer.rb +++ b/lib/gitlab/google_code_import/importer.rb @@ -237,7 +237,7 @@ module Gitlab def create_label(name) color = nice_label_color(name) - Label.create!(project_id: project.id, name: name, color: color) + project.labels.create!(name: name, color: color) end def format_content(raw_content) diff --git a/spec/factories/labels.rb b/spec/factories/labels.rb index eb489099854d719e3752cf6e47701e3fc66f3ce2..ec4c56457ea12ba0c52db9e79e84a75afe7541ad 100644 --- a/spec/factories/labels.rb +++ b/spec/factories/labels.rb @@ -1,5 +1,5 @@ FactoryGirl.define do - factory :label do + factory :label, class: ProjectLabel do sequence(:title) { |n| "label#{n}" } color "#990000" project diff --git a/spec/models/label_spec.rb b/spec/models/label_spec.rb index 894021dc8e6eeb7c04f587c84e6aaf46e6731243..1f1fe45d5a75d7fa96c2a41c6d9ef1f1c4a7ffd2 100644 --- a/spec/models/label_spec.rb +++ b/spec/models/label_spec.rb @@ -1,47 +1,41 @@ require 'spec_helper' describe Label, models: true do - let(:label) { create(:label) } + describe 'modules' do + it { is_expected.to include_module(Referable) } + it { is_expected.to include_module(Subscribable) } + end describe 'associations' do - it { is_expected.to belong_to(:project) } - - it { is_expected.to have_many(:label_links).dependent(:destroy) } it { is_expected.to have_many(:issues).through(:label_links).source(:target) } + it { is_expected.to have_many(:label_links).dependent(:destroy) } it { is_expected.to have_many(:lists).dependent(:destroy) } end - describe 'modules' do - subject { described_class } - - it { is_expected.to include_module(Referable) } - end - describe 'validation' do - it { is_expected.to validate_presence_of(:project) } it { is_expected.to validate_uniqueness_of(:title) } it 'validates color code' do - expect(label).not_to allow_value('G-ITLAB').for(:color) - expect(label).not_to allow_value('AABBCC').for(:color) - expect(label).not_to allow_value('#AABBCCEE').for(:color) - expect(label).not_to allow_value('GGHHII').for(:color) - expect(label).not_to allow_value('#').for(:color) - expect(label).not_to allow_value('').for(:color) - - expect(label).to allow_value('#AABBCC').for(:color) - expect(label).to allow_value('#abcdef').for(:color) + is_expected.not_to allow_value('G-ITLAB').for(:color) + is_expected.not_to allow_value('AABBCC').for(:color) + is_expected.not_to allow_value('#AABBCCEE').for(:color) + is_expected.not_to allow_value('GGHHII').for(:color) + is_expected.not_to allow_value('#').for(:color) + is_expected.not_to allow_value('').for(:color) + + is_expected.to allow_value('#AABBCC').for(:color) + is_expected.to allow_value('#abcdef').for(:color) end it 'validates title' do - expect(label).not_to allow_value('G,ITLAB').for(:title) - expect(label).not_to allow_value('').for(:title) - - expect(label).to allow_value('GITLAB').for(:title) - expect(label).to allow_value('gitlab').for(:title) - expect(label).to allow_value('G?ITLAB').for(:title) - expect(label).to allow_value('G&ITLAB').for(:title) - expect(label).to allow_value("customer's request").for(:title) + is_expected.not_to allow_value('G,ITLAB').for(:title) + is_expected.not_to allow_value('').for(:title) + + is_expected.to allow_value('GITLAB').for(:title) + is_expected.to allow_value('gitlab').for(:title) + is_expected.to allow_value('G?ITLAB').for(:title) + is_expected.to allow_value('G&ITLAB').for(:title) + is_expected.to allow_value("customer's request").for(:title) end end @@ -53,6 +47,8 @@ describe Label, models: true do end describe '#to_reference' do + let(:label) { create(:label) } + context 'using id' do it 'returns a String reference to the object' do expect(label.to_reference).to eq "~#{label.id}" diff --git a/spec/models/project_label_spec.rb b/spec/models/project_label_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..93062b9d4029468f4c6603d0ef5a774c803b7d7f --- /dev/null +++ b/spec/models/project_label_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +describe ProjectLabel, models: true do + describe 'relationships' do + it { is_expected.to belong_to(:project) } + end + + describe 'validations' do + it { is_expected.to validate_presence_of(:project) } + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index 67dbcc362f6115b780fa76e7aeab954462dbdea2..e6d98e25d0bce6b8960f7f68db832cdaa5229a93 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -56,7 +56,7 @@ describe Project, models: true do it { is_expected.to have_many(:runners) } it { is_expected.to have_many(:variables) } it { is_expected.to have_many(:triggers) } - it { is_expected.to have_many(:labels).dependent(:destroy) } + it { is_expected.to have_many(:labels).class_name('ProjectLabel').dependent(:destroy) } it { is_expected.to have_many(:users_star_projects).dependent(:destroy) } it { is_expected.to have_many(:environments).dependent(:destroy) } it { is_expected.to have_many(:deployments).dependent(:destroy) }