Commit 6f4b2afc authored by Simon Knox's avatar Simon Knox Committed by Shinya Maeda

Load correct iteration on iteration report

Also unquarantine test, as it will load correct item now
Remove sign_in as not needed to view
parent 6566448f
......@@ -13,6 +13,10 @@ class ApplicationRecord < ActiveRecord::Base
where(id: ids)
end
def self.iid_in(iids)
where(iid: iids)
end
def self.id_not_in(ids)
where.not(id: ids)
end
......
......@@ -602,6 +602,14 @@ class Project < ApplicationRecord
end
end
def self.projects_user_can(projects, user, action)
projects = where(id: projects)
DeclarativePolicy.user_scope do
projects.select { |project| Ability.allowed?(user, action, project) }
end
end
# This scope returns projects where user has access to both the project and the feature.
def self.filter_by_feature_visibility(feature, user)
with_feature_available_for_user(feature, user)
......
......@@ -5094,6 +5094,11 @@ type Group {
"""
id: ID
"""
The internal ID of the Iteration to look up
"""
iid: ID
"""
Returns the last _n_ elements from the list.
"""
......@@ -6301,6 +6306,11 @@ type Iteration {
"""
id: ID!
"""
Internal ID of the iteration
"""
iid: ID!
"""
Timestamp of the iteration start date
"""
......
......@@ -14100,6 +14100,16 @@
},
"defaultValue": null
},
{
"name": "iid",
"description": "The internal ID of the Iteration to look up",
"type": {
"kind": "SCALAR",
"name": "ID",
"ofType": null
},
"defaultValue": null
},
{
"name": "after",
"description": "Returns the elements in the list that come after the specified cursor.",
......@@ -17329,6 +17339,24 @@
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "iid",
"description": "Internal ID of the iteration",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "ID",
"ofType": null
}
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "startDate",
"description": "Timestamp of the iteration start date",
......@@ -946,6 +946,7 @@ Represents an iteration object.
| `description` | String | Description of the iteration |
| `dueDate` | Time | Timestamp of the iteration due date |
| `id` | ID! | ID of the iteration |
| `iid` | ID! | Internal ID of the iteration |
| `startDate` | Time | Timestamp of the iteration start date |
| `state` | IterationState! | State of the iteration |
| `title` | String! | Title of the iteration |
......
......@@ -10,7 +10,6 @@ import {
} from '@gitlab/ui';
import { formatDate } from '~/lib/utils/datetime_utility';
import { __ } from '~/locale';
import { getIdFromGraphQLId } from '~/graphql_shared/utils';
import IterationForm from './iteration_form.vue';
import query from '../queries/group_iteration.query.graphql';
......@@ -37,7 +36,7 @@ export default {
variables() {
return {
groupPath: this.groupPath,
id: getIdFromGraphQLId(this.iterationId),
iid: this.iterationIid,
};
},
update(data) {
......@@ -57,7 +56,7 @@ export default {
type: String,
required: true,
},
iterationId: {
iterationIid: {
type: String,
required: true,
},
......
......@@ -51,7 +51,7 @@ export function initIterationForm() {
export function initIterationReport() {
const el = document.querySelector('.js-iteration');
const { groupPath, iterationId, editIterationPath } = el.dataset;
const { groupPath, iterationIid, editIterationPath } = el.dataset;
const canEdit = parseBoolean(el.dataset.canEdit);
return new Vue({
......@@ -61,7 +61,7 @@ export function initIterationReport() {
return createElement(IterationReport, {
props: {
groupPath,
iterationId,
iterationIid,
canEdit,
editIterationPath,
},
......
query GroupIteration($groupPath: ID!, $id: ID!) {
query GroupIteration($groupPath: ID!, $iid: ID!) {
group(fullPath: $groupPath) {
iterations(id: $id, first: 1) {
iterations(iid: $iid, first: 1) {
nodes {
title
state
iid
id
description
webPath
......
......@@ -13,15 +13,19 @@ class IterationsFinder
include FinderMethods
include TimeFrameFilter
attr_reader :params
attr_reader :params, :current_user
def initialize(params = {})
def initialize(current_user, params = {})
@params = params
@current_user = current_user
end
def execute
filter_permissions
items = Iteration.all
items = by_id(items)
items = by_iid(items)
items = by_groups_and_projects(items)
items = by_title(items)
items = by_search_title(items)
......@@ -33,6 +37,31 @@ class IterationsFinder
private
def filter_permissions
filter_allowed_projects
filter_allowed_groups
# Only allow either one project_id or one group_id when filtering by `iid`
if params[:iid] && params.slice(:project_ids, :group_ids).keys.count > 1
raise ArgumentError, 'You can specify only one scope if you use iid filter'
end
end
def filter_allowed_projects
return unless params[:project_ids].present?
projects = Project.id_in(params[:project_ids])
params[:project_ids] = Project.projects_user_can(projects, current_user, :read_iteration)
end
def filter_allowed_groups
return unless params[:group_ids].present?
groups = Group.id_in(params[:group_ids])
params[:group_ids] = Group.groups_user_can(groups, current_user, :read_iteration)
end
def by_groups_and_projects(items)
items.for_projects_and_groups(params[:project_ids], params[:group_ids])
end
......@@ -45,6 +74,10 @@ class IterationsFinder
end
end
def by_iid(items)
params[:iid].present? ? items.iid_in(params[:iid]) : items
end
def by_title(items)
if params[:title]
items.with_title(params[:title])
......
......@@ -14,6 +14,9 @@ module Resolvers
argument :id, GraphQL::ID_TYPE,
required: false,
description: 'The ID of the Iteration to look up'
argument :iid, GraphQL::ID_TYPE,
required: false,
description: 'The internal ID of the Iteration to look up'
type Types::IterationType, null: true
......@@ -22,7 +25,7 @@ module Resolvers
authorize!
iterations = IterationsFinder.new(iterations_finder_params(args)).execute
iterations = IterationsFinder.new(context[:current_user], iterations_finder_params(args)).execute
Gitlab::Graphql::Pagination::OffsetActiveRecordRelationConnection.new(iterations)
end
......@@ -32,6 +35,7 @@ module Resolvers
def iterations_finder_params(args)
{
id: args[:id],
iid: args[:iid],
state: args[:state] || 'all',
start_date: args[:start_date],
end_date: args[:end_date],
......
......@@ -12,6 +12,9 @@ module Types
field :id, GraphQL::ID_TYPE, null: false,
description: 'ID of the iteration'
field :iid, GraphQL::ID_TYPE, null: false,
description: 'Internal ID of the iteration'
field :title, GraphQL::STRING_TYPE, null: false,
description: 'Title of the iteration'
......
......@@ -142,7 +142,7 @@ module EE
end
class_methods do
def groups_user_can_read_epics(groups, user, same_root: false)
def groups_user_can(groups, user, action, same_root: false)
groups = ::Gitlab::GroupPlansPreloader.new.preload(groups)
# if we are sure that all groups have the same root group, we can
......@@ -152,8 +152,12 @@ module EE
preset_root_ancestor_for(groups) if same_root
DeclarativePolicy.user_scope do
groups.select { |group| Ability.allowed?(user, :read_epic, group) }
groups.select { |group| Ability.allowed?(user, action, group) }
end
end
def groups_user_can_read_epics(groups, user, same_root: false)
groups_user_can(groups, user, :read_epic, same_root: same_root)
end
def preset_root_ancestor_for(groups)
......
......@@ -3,4 +3,4 @@
- page_title _("Iterations")
- if Feature.enabled?(:group_iterations, @group)
.js-iteration{ data: { group_path: @group.full_path, iteration_id: params[:id], can_edit: can?(current_user, :admin_iteration, @group).to_s, preview_markdown_path: preview_markdown_path(@group) } }
.js-iteration{ data: { group_path: @group.full_path, iteration_iid: params[:id], can_edit: can?(current_user, :admin_iteration, @group).to_s, preview_markdown_path: preview_markdown_path(@group) } }
......@@ -77,7 +77,7 @@ module EE
def find_iteration_with_finder(parent, params)
finder_params = iteration_finder_params(parent)
::IterationsFinder.new(finder_params).find_by(params)
::IterationsFinder.new(context[:current_user], finder_params).find_by(params)
end
# rubocop:enable CodeReuse/ActiveRecord
......
......@@ -130,9 +130,9 @@ module EE
end
def find_iterations(project, params = {})
group_ids = project.group.self_and_ancestors.select(:id) if project.group
group_ids = project.group.self_and_ancestors.map(&:id) if project.group
::IterationsFinder.new(params.merge(project_ids: [project.id], group_ids: group_ids)).execute
::IterationsFinder.new(current_user, params.merge(project_ids: [project.id], group_ids: group_ids)).execute
end
desc _('Publish to status page')
......
......@@ -16,7 +16,6 @@ module Gitlab
# rubocop: disable CodeReuse/ActiveRecord
def preload(groups)
groups_and_ancestors = groups_and_ancestors_for(groups)
# A Hash mapping group IDs to their corresponding Group instances.
groups_map = groups_and_ancestors.each_with_object({}) do |group, hash|
hash[group.id] = group
......
......@@ -5,18 +5,14 @@ require 'spec_helper'
RSpec.describe 'User views iteration' do
let_it_be(:now) { Time.now }
let_it_be(:group) { create(:group) }
let_it_be(:user) { create(:group_member, :maintainer, user: create(:user), group: group ).user }
let_it_be(:iteration) { create(:iteration, :skip_future_date_validation, group: group, start_date: now - 1.day, due_date: now) }
around do |example|
Timecop.freeze { example.run }
end
context 'with license' do
before do
sign_in(user)
stub_licensed_features(iterations: true)
end
context 'view an iteration', :js, quarantine: 'https://gitlab.com/gitlab-org/gitlab/-/issues/222915' do
context 'view an iteration', :js do
before do
visit group_iteration_path(iteration.group, iteration)
end
......@@ -24,6 +20,9 @@ RSpec.describe 'User views iteration' do
it 'shows iteration info and dates' do
expect(page).to have_content(iteration.title)
expect(page).to have_content(iteration.description)
expect(page).to have_content(iteration.start_date.strftime('%b %-d, %Y'))
expect(page).to have_content(iteration.due_date.strftime('%b %-d, %Y'))
end
end
end
end
......@@ -4,16 +4,34 @@ require 'spec_helper'
RSpec.describe IterationsFinder do
let(:now) { Time.now }
let_it_be(:group) { create(:group) }
let_it_be(:group) { create(:group, :private) }
let_it_be(:project_1) { create(:project, namespace: group) }
let_it_be(:project_2) { create(:project, namespace: group) }
let_it_be(:user) { create(:user) }
let!(:started_group_iteration) { create(:started_iteration, :skip_future_date_validation, group: group, title: 'one test', start_date: now - 1.day, due_date: now) }
let!(:upcoming_group_iteration) { create(:iteration, group: group, start_date: 1.day.from_now, due_date: 2.days.from_now) }
let!(:iteration_from_project_1) { create(:started_iteration, project: project_1, start_date: 2.days.from_now, due_date: 3.days.from_now) }
let!(:iteration_from_project_2) { create(:started_iteration, project: project_2, start_date: 4.days.from_now, due_date: 5.days.from_now) }
let(:project_ids) { [project_1.id, project_2.id] }
subject { described_class.new(params).execute }
subject { described_class.new(user, params).execute }
context 'without permissions' do
context 'groups and projects' do
let(:params) { { project_ids: project_ids, group_ids: group.id, state: 'all' } }
it 'returns iterations for groups and projects' do
expect(subject).to be_empty
end
end
end
context 'with permissions' do
before do
group.add_reporter(user)
project_1.add_reporter(user)
project_2.add_reporter(user)
end
context 'iterations for projects' do
let(:params) { { project_ids: project_ids, state: 'all' } }
......@@ -120,11 +138,26 @@ RSpec.describe IterationsFinder do
end
end
describe 'iid' do
let(:params) do
{
project_ids: project_ids,
group_ids: group.id,
iid: iteration_from_project_1.iid
}
end
it 'only accepts one of project_id or group_id' do
expect { subject }.to raise_error(ArgumentError, 'You can specify only one scope if you use iid filter')
end
end
describe '#find_by' do
it 'finds a single iteration' do
finder = described_class.new(project_ids: [project_1.id], state: 'all')
finder = described_class.new(user, project_ids: [project_1.id], state: 'all')
expect(finder.find_by(iid: iteration_from_project_1.iid)).to eq(iteration_from_project_1)
end
end
end
end
......@@ -6,7 +6,7 @@ describe('Iterations tabs', () => {
let wrapper;
const defaultProps = {
groupPath: 'gitlab-org',
iterationId: '3',
iterationIid: '3',
};
const findTopbar = () => wrapper.find({ ref: 'topbar' });
......
......@@ -30,9 +30,9 @@ RSpec.describe Resolvers::IterationsResolver do
context 'without parameters' do
it 'calls IterationsFinder to retrieve all iterations' do
params = { id: nil, group_ids: group.id, state: 'all', start_date: nil, end_date: nil, search_title: nil }
params = { id: nil, iid: nil, group_ids: group.id, state: 'all', start_date: nil, end_date: nil, search_title: nil }
expect(IterationsFinder).to receive(:new).with(params).and_call_original
expect(IterationsFinder).to receive(:new).with(current_user, params).and_call_original
resolve_group_iterations
end
......@@ -44,11 +44,12 @@ RSpec.describe Resolvers::IterationsResolver do
end_date = start_date + 1.hour
search = 'wow'
id = 1
params = { id: id, group_ids: group.id, state: 'closed', start_date: start_date, end_date: end_date, search_title: search }
iid = 2
params = { id: id, iid: iid, group_ids: group.id, state: 'closed', start_date: start_date, end_date: end_date, search_title: search }
expect(IterationsFinder).to receive(:new).with(params).and_call_original
expect(IterationsFinder).to receive(:new).with(current_user, params).and_call_original
resolve_group_iterations(start_date: start_date, end_date: end_date, state: 'closed', title: search, id: id)
resolve_group_iterations(start_date: start_date, end_date: end_date, state: 'closed', title: search, id: id, iid: iid)
end
end
......
......@@ -221,12 +221,18 @@ RSpec.describe QuickActions::InterpretService do
end
context 'when iteration exists' do
context 'with permissions' do
before do
group.add_developer(current_user)
end
it 'assigns an iteration to an issue' do
_, updates, message = service.execute(content, issue)
expect(updates).to eq(iteration: iteration)
expect(message).to eq("Set the iteration to #{iteration.to_reference}.")
end
end
context 'when the user does not have enough permissions' do
before do
......
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