Commit c423a41b authored by Gilang Gumilar's avatar Gilang Gumilar Committed by Shinya Maeda

Fix filter by releases at group issues and merge requests search bar

Closes https://gitlab.com/gitlab-org/gitlab/-/issues/208805

See merge request https://gitlab.com/gitlab-org/gitlab/-/merge_requests/26740
parent caa77d48
# frozen_string_literal: true
module Groups
class ReleasesController < Groups::ApplicationController
def index
respond_to do |format|
format.json do
render json: ReleaseSerializer.new.represent(releases)
end
end
end
private
def releases
ReleasesFinder
.new(@group, current_user, { include_subgroups: true })
.execute(preload: false)
.page(params[:page])
.per(30)
end
end
end
# frozen_string_literal: true
class ReleasesFinder
attr_reader :project, :current_user, :params
include Gitlab::Utils::StrongMemoize
def initialize(project, current_user = nil, params = {})
@project = project
attr_reader :parent, :current_user, :params
def initialize(parent, current_user = nil, params = {})
@parent = parent
@current_user = current_user
@params = params
end
def execute(preload: true)
return Release.none unless Ability.allowed?(current_user, :read_release, project)
return Release.none if projects.empty?
# See https://gitlab.com/gitlab-org/gitlab/-/issues/211988
releases = project.releases.where.not(tag: nil) # rubocop:disable CodeReuse/ActiveRecord
releases = get_releases
releases = by_tag(releases)
releases = releases.preloaded if preload
releases.sorted
......@@ -21,6 +22,34 @@ class ReleasesFinder
private
def get_releases
Release.where(project_id: projects).where.not(tag: nil) # rubocop: disable CodeReuse/ActiveRecord
end
def include_subgroups?
params.fetch(:include_subgroups, false)
end
def projects
strong_memoize(:projects) do
if parent.is_a?(Project)
Ability.allowed?(current_user, :read_release, parent) ? [parent] : []
elsif parent.is_a?(Group)
accessible_projects
end
end
end
def accessible_projects
projects = if include_subgroups?
Project.for_group_and_its_subgroups(parent)
else
parent.projects
end
projects.select { |project| Ability.allowed?(current_user, :read_release, project) }
end
# rubocop: disable CodeReuse/ActiveRecord
def by_tag(releases)
return releases unless params[:tag].present?
......
......@@ -229,6 +229,7 @@ module SearchHelper
opts[:data]['group-id'] = @group.id
opts[:data]['labels-endpoint'] = group_labels_path(@group)
opts[:data]['milestones-endpoint'] = group_milestones_path(@group)
opts[:data]['releases-endpoint'] = group_releases_path(@group)
else
opts[:data]['labels-endpoint'] = dashboard_labels_path
opts[:data]['milestones-endpoint'] = dashboard_milestones_path
......
# frozen_string_literal: true
class ReleaseEntity < Grape::Entity
expose :id
expose :tag # see https://gitlab.com/gitlab-org/gitlab/-/issues/36338
end
# frozen_string_literal: true
class ReleaseSerializer < BaseSerializer
entity ReleaseEntity
end
---
title: Fix filter by releases at group issues and merge requests search bar.
merge_request: 26740
author: Gilang Gumilar
type: fixed
......@@ -69,6 +69,8 @@ constraints(::Constraints::GroupUrlConstrainer.new) do
end
end
resources :releases, only: [:index]
resources :deploy_tokens, constraints: { id: /\d+/ }, only: [] do
member do
put :revoke
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Groups::ReleasesController do
let(:group) { create(:group) }
let!(:project) { create(:project, :repository, :public, namespace: group) }
let!(:private_project) { create(:project, :repository, :private, namespace: group) }
let(:developer) { create(:user) }
let!(:release_1) { create(:release, project: project, tag: 'v1', released_at: Time.zone.parse('2020-02-15')) }
let!(:release_2) { create(:release, project: project, tag: 'v2', released_at: Time.zone.parse('2020-02-20')) }
let!(:private_release_1) { create(:release, project: private_project, tag: 'p1', released_at: Time.zone.parse('2020-03-01')) }
let!(:private_release_2) { create(:release, project: private_project, tag: 'p2', released_at: Time.zone.parse('2020-03-05')) }
before do
private_project.add_developer(developer)
end
describe 'GET #index' do
context 'as json' do
let(:format) { :json }
subject { get :index, params: { group_id: group }, format: format }
context 'json_response' do
before do
subject
end
it 'returns an application/json content_type' do
expect(response.content_type).to eq 'application/json'
end
it 'returns OK' do
expect(response).to have_gitlab_http_status(:ok)
end
end
context 'the user is not authorized' do
before do
subject
end
it 'does not return any releases' do
expect(json_response.map {|r| r['tag'] } ).to match_array(%w(v2 v1))
end
it 'returns OK' do
expect(response).to have_gitlab_http_status(:ok)
end
end
context 'the user is authorized' do
it "returns all group's public and private project's releases as JSON, ordered by released_at" do
sign_in(developer)
subject
expect(json_response.map {|r| r['tag'] } ).to match_array(%w(p2 p1 v2 v1))
end
end
context 'N+1 queries' do
it 'avoids N+1 database queries' do
control_count = ActiveRecord::QueryRecorder.new { subject }.count
create_list(:release, 5, project: project)
create_list(:release, 5, project: private_project)
expect { subject }.not_to exceed_query_limit(control_count)
end
end
end
end
end
......@@ -4,40 +4,46 @@ require 'spec_helper'
RSpec.describe ReleasesFinder do
let(:user) { create(:user) }
let(:project) { create(:project, :repository) }
let(:group) { create :group }
let(:project) { create(:project, :repository, group: group) }
let(:params) { {} }
let(:args) { {} }
let(:repository) { project.repository }
let(:v1_0_0) { create(:release, project: project, tag: 'v1.0.0') }
let(:v1_1_0) { create(:release, project: project, tag: 'v1.1.0') }
let(:finder) { described_class.new(project, user, params) }
before do
v1_0_0.update_attribute(:released_at, 2.days.ago)
v1_1_0.update_attribute(:released_at, 1.day.ago)
end
describe '#execute' do
subject { finder.execute(**args) }
let(:args) { {} }
context 'when the user is not part of the project' do
shared_examples_for 'when the user is not part of the project' do
it 'returns no releases' do
is_expected.to be_empty
end
end
context 'when the user is a project developer' do
# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/27716
shared_examples_for 'when tag is nil' do
before do
project.add_developer(user)
v1_0_0.update_column(:tag, nil)
end
it 'sorts by release date' do
is_expected.to be_present
expect(subject.size).to eq(2)
expect(subject).to eq([v1_1_0, v1_0_0])
it 'ignores rows with a nil tag' do
expect(subject.size).to eq(1)
expect(subject).to eq([v1_1_0])
end
end
shared_examples_for 'when a tag parameter is passed' do
let(:params) { { tag: 'v1.0.0' } }
it 'only returns the release with the matching tag' do
expect(subject).to eq([v1_0_0])
end
end
shared_examples_for 'preload' do
it 'preloads associations' do
expect(Release).to receive(:preloaded).once.and_call_original
......@@ -53,24 +59,155 @@ RSpec.describe ReleasesFinder do
subject
end
end
end
# See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/27716
context 'when tag is nil' do
describe 'when parent is a project' do
subject { described_class.new(project, user, params).execute(**args) }
it_behaves_like 'when the user is not part of the project'
context 'when the user is a project developer' do
before do
v1_0_0.update_column(:tag, nil)
project.add_developer(user)
end
it 'ignores rows with a nil tag' do
expect(subject.size).to eq(1)
expect(subject).to eq([v1_1_0])
it 'sorts by release date' do
is_expected.to be_present
expect(subject.size).to eq(2)
expect(subject).to eq([v1_1_0, v1_0_0])
end
it_behaves_like 'preload'
it_behaves_like 'when tag is nil'
it_behaves_like 'when a tag parameter is passed'
end
end
context 'when a tag parameter is passed' do
let(:params) { { tag: 'v1.0.0' } }
describe 'when parent is a group' do
context 'without subgroups' do
let(:project2) { create(:project, :repository, namespace: group) }
let!(:v6) { create(:release, project: project2, tag: 'v6') }
it 'only returns the release with the matching tag' do
expect(subject).to eq([v1_0_0])
subject { described_class.new(group, user, params).execute(**args) }
it_behaves_like 'when the user is not part of the project'
context 'when the user is a project developer on one sibling project' do
before do
project.add_developer(user)
v1_0_0.update_attribute(:released_at, 3.days.ago)
v1_1_0.update_attribute(:released_at, 1.day.ago)
end
it 'sorts by release date' do
expect(subject.size).to eq(2)
expect(subject).to eq([v1_1_0, v1_0_0])
end
end
context 'when the user is a project developer on all projects' do
before do
project.add_developer(user)
project2.add_developer(user)
v1_0_0.update_attribute(:released_at, 3.days.ago)
v6.update_attribute(:released_at, 2.days.ago)
v1_1_0.update_attribute(:released_at, 1.day.ago)
end
it 'sorts by release date' do
expect(subject.size).to eq(3)
expect(subject).to eq([v1_1_0, v6, v1_0_0])
end
it_behaves_like 'when a tag parameter is passed'
end
end
describe 'with subgroups' do
let(:params) { { include_subgroups: true } }
subject { described_class.new(group, user, params).execute(**args) }
context 'with a single-level subgroup' do
let(:subgroup) { create :group, parent: group }
let(:project2) { create(:project, :repository, namespace: subgroup) }
let!(:v6) { create(:release, project: project2, tag: 'v6') }
it_behaves_like 'when the user is not part of the project'
context 'when the user a project developer in the subgroup project' do
before do
project2.add_developer(user)
end
it 'returns only the subgroup releases' do
expect(subject).to match_array([v6])
end
end
context 'when the user a project developer in both projects' do
before do
project.add_developer(user)
project2.add_developer(user)
v6.update_attribute(:released_at, 2.days.ago)
end
it 'returns all releases' do
expect(subject).to match_array([v1_1_0, v1_0_0, v6])
end
it_behaves_like 'when a tag parameter is passed'
end
end
context 'with a multi-level subgroup' do
let(:subgroup) { create :group, parent: group }
let(:subsubgroup) { create :group, parent: subgroup }
let(:project2) { create(:project, :repository, namespace: subgroup) }
let(:project3) { create(:project, :repository, namespace: subsubgroup) }
let!(:v6) { create(:release, project: project2, tag: 'v6') }
let!(:p3) { create(:release, project: project3, tag: 'p3') }
before do
v6.update_attribute(:released_at, 2.days.ago)
p3.update_attribute(:released_at, 3.days.ago)
end
it_behaves_like 'when the user is not part of the project'
context 'when the user a project developer in the subgroup and subsubgroup project' do
before do
project2.add_developer(user)
project3.add_developer(user)
end
it 'returns only the subgroup and subsubgroup releases' do
expect(subject).to match_array([v6, p3])
end
end
context 'when the user a project developer in the subsubgroup project' do
before do
project3.add_developer(user)
end
it 'returns only the subsubgroup releases' do
expect(subject).to match_array([p3])
end
end
context 'when the user a project developer in all projects' do
before do
project.add_developer(user)
project2.add_developer(user)
project3.add_developer(user)
end
it 'returns all releases' do
expect(subject).to match_array([v1_1_0, v6, v1_0_0, p3])
end
it_behaves_like 'when a tag parameter is passed'
end
end
end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe ReleaseSerializer do
let(:user) { create(:user) }
let(:project) { create :project }
subject { described_class.new.represent(resource, current_user: user) }
before do
project.add_developer(user)
end
describe '#represent' do
context 'when a single object is being serialized' do
let(:resource) { create(:release, project: project) }
it 'serializes the label object' do
expect(subject[:tag]).to eq resource.tag
end
end
context 'when multiple objects are being serialized' do
let(:resource) { create_list(:release, 3) }
it 'serializes the array of releases' do
expect(subject.size).to eq(3)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment