Commit 27277bfc authored by Jan Provaznik's avatar Jan Provaznik Committed by Bob Van Landuyt

Allow setting of issue assignees in GraphQL

Adds GraphQL mutation to set issue assignees same as for merge requests.
parent f7800a90
# frozen_string_literal: true
module Mutations
module Assignable
extend ActiveSupport::Concern
included do
argument :assignee_usernames,
[GraphQL::STRING_TYPE],
required: true,
description: 'The usernames to assign to the resource. Replaces existing assignees by default.'
argument :operation_mode,
Types::MutationOperationModeEnum,
required: false,
description: 'The operation to perform. Defaults to REPLACE.'
end
def resolve(project_path:, iid:, assignee_usernames:, operation_mode: Types::MutationOperationModeEnum.enum[:replace])
resource = authorized_find!(project_path: project_path, iid: iid)
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab/issues/36098') if resource.is_a?(MergeRequest)
update_service_class.new(
resource.project,
current_user,
assignee_ids: assignee_ids(resource, assignee_usernames, operation_mode)
).execute(resource)
{
resource.class.name.underscore.to_sym => resource,
errors: errors_on_object(resource)
}
end
private
def assignee_ids(resource, usernames, operation_mode)
assignee_ids = []
assignee_ids += resource.assignees.map(&:id) if Types::MutationOperationModeEnum.enum.values_at(:remove, :append).include?(operation_mode)
user_ids = UsersFinder.new(current_user, username: usernames).execute.map(&:id)
if operation_mode == Types::MutationOperationModeEnum.enum[:remove]
assignee_ids -= user_ids
else
assignee_ids |= user_ids
end
assignee_ids
end
end
end
# frozen_string_literal: true
module Mutations
module Issues
class SetAssignees < Base
graphql_name 'IssueSetAssignees'
include Assignable
def update_service_class
::Issues::UpdateService
end
end
end
end
...@@ -5,43 +5,10 @@ module Mutations ...@@ -5,43 +5,10 @@ module Mutations
class SetAssignees < Base class SetAssignees < Base
graphql_name 'MergeRequestSetAssignees' graphql_name 'MergeRequestSetAssignees'
argument :assignee_usernames, include Assignable
[GraphQL::STRING_TYPE],
required: true,
description: <<~DESC
The usernames to assign to the merge request. Replaces existing assignees by default.
DESC
argument :operation_mode, def update_service_class
Types::MutationOperationModeEnum, ::MergeRequests::UpdateService
required: false,
description: <<~DESC
The operation to perform. Defaults to REPLACE.
DESC
def resolve(project_path:, iid:, assignee_usernames:, operation_mode: Types::MutationOperationModeEnum.enum[:replace])
Gitlab::QueryLimiting.whitelist('https://gitlab.com/gitlab-org/gitlab/issues/36098')
merge_request = authorized_find!(project_path: project_path, iid: iid)
project = merge_request.project
assignee_ids = []
assignee_ids += merge_request.assignees.map(&:id) if Types::MutationOperationModeEnum.enum.values_at(:remove, :append).include?(operation_mode)
user_ids = UsersFinder.new(current_user, username: assignee_usernames).execute.map(&:id)
if operation_mode == Types::MutationOperationModeEnum.enum[:remove]
assignee_ids -= user_ids
else
assignee_ids |= user_ids
end
::MergeRequests::UpdateService.new(project, current_user, assignee_ids: assignee_ids)
.execute(merge_request)
{
merge_request: merge_request,
errors: errors_on_object(merge_request)
}
end end
end end
end end
......
...@@ -17,6 +17,7 @@ module Types ...@@ -17,6 +17,7 @@ module Types
mount_mutation Mutations::Branches::Create, calls_gitaly: true mount_mutation Mutations::Branches::Create, calls_gitaly: true
mount_mutation Mutations::Commits::Create, calls_gitaly: true mount_mutation Mutations::Commits::Create, calls_gitaly: true
mount_mutation Mutations::Discussions::ToggleResolve mount_mutation Mutations::Discussions::ToggleResolve
mount_mutation Mutations::Issues::SetAssignees
mount_mutation Mutations::Issues::SetConfidential mount_mutation Mutations::Issues::SetConfidential
mount_mutation Mutations::Issues::SetLocked mount_mutation Mutations::Issues::SetLocked
mount_mutation Mutations::Issues::SetDueDate mount_mutation Mutations::Issues::SetDueDate
......
---
title: Allow assign/unassign users to issues in GraphQL API.
merge_request: 38081
author:
type: added
...@@ -6269,6 +6269,56 @@ type IssuePermissions { ...@@ -6269,6 +6269,56 @@ type IssuePermissions {
updateIssue: Boolean! updateIssue: Boolean!
} }
"""
Autogenerated input type of IssueSetAssignees
"""
input IssueSetAssigneesInput {
"""
The usernames to assign to the resource. Replaces existing assignees by default.
"""
assigneeUsernames: [String!]!
"""
A unique identifier for the client performing the mutation.
"""
clientMutationId: String
"""
The IID of the issue to mutate
"""
iid: String!
"""
The operation to perform. Defaults to REPLACE.
"""
operationMode: MutationOperationMode
"""
The project the issue to mutate is in
"""
projectPath: ID!
}
"""
Autogenerated return type of IssueSetAssignees
"""
type IssueSetAssigneesPayload {
"""
A unique identifier for the client performing the mutation.
"""
clientMutationId: String
"""
Errors encountered during execution of the mutation.
"""
errors: [String!]!
"""
The issue after mutation
"""
issue: Issue
}
""" """
Autogenerated input type of IssueSetConfidential Autogenerated input type of IssueSetConfidential
""" """
...@@ -7770,7 +7820,7 @@ Autogenerated input type of MergeRequestSetAssignees ...@@ -7770,7 +7820,7 @@ Autogenerated input type of MergeRequestSetAssignees
""" """
input MergeRequestSetAssigneesInput { input MergeRequestSetAssigneesInput {
""" """
The usernames to assign to the merge request. Replaces existing assignees by default. The usernames to assign to the resource. Replaces existing assignees by default.
""" """
assigneeUsernames: [String!]! assigneeUsernames: [String!]!
...@@ -8410,6 +8460,7 @@ type Mutation { ...@@ -8410,6 +8460,7 @@ type Mutation {
epicAddIssue(input: EpicAddIssueInput!): EpicAddIssuePayload epicAddIssue(input: EpicAddIssueInput!): EpicAddIssuePayload
epicSetSubscription(input: EpicSetSubscriptionInput!): EpicSetSubscriptionPayload epicSetSubscription(input: EpicSetSubscriptionInput!): EpicSetSubscriptionPayload
epicTreeReorder(input: EpicTreeReorderInput!): EpicTreeReorderPayload epicTreeReorder(input: EpicTreeReorderInput!): EpicTreeReorderPayload
issueSetAssignees(input: IssueSetAssigneesInput!): IssueSetAssigneesPayload
issueSetConfidential(input: IssueSetConfidentialInput!): IssueSetConfidentialPayload issueSetConfidential(input: IssueSetConfidentialInput!): IssueSetConfidentialPayload
issueSetDueDate(input: IssueSetDueDateInput!): IssueSetDueDatePayload issueSetDueDate(input: IssueSetDueDateInput!): IssueSetDueDatePayload
issueSetIteration(input: IssueSetIterationInput!): IssueSetIterationPayload issueSetIteration(input: IssueSetIterationInput!): IssueSetIterationPayload
......
...@@ -17380,6 +17380,154 @@ ...@@ -17380,6 +17380,154 @@
"enumValues": null, "enumValues": null,
"possibleTypes": null "possibleTypes": null
}, },
{
"kind": "INPUT_OBJECT",
"name": "IssueSetAssigneesInput",
"description": "Autogenerated input type of IssueSetAssignees",
"fields": null,
"inputFields": [
{
"name": "projectPath",
"description": "The project the issue to mutate is in",
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "ID",
"ofType": null
}
},
"defaultValue": null
},
{
"name": "iid",
"description": "The IID of the issue to mutate",
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "String",
"ofType": null
}
},
"defaultValue": null
},
{
"name": "assigneeUsernames",
"description": "The usernames to assign to the resource. Replaces existing assignees by default.",
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "String",
"ofType": null
}
}
}
},
"defaultValue": null
},
{
"name": "operationMode",
"description": "The operation to perform. Defaults to REPLACE.",
"type": {
"kind": "ENUM",
"name": "MutationOperationMode",
"ofType": null
},
"defaultValue": null
},
{
"name": "clientMutationId",
"description": "A unique identifier for the client performing the mutation.",
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"defaultValue": null
}
],
"interfaces": null,
"enumValues": null,
"possibleTypes": null
},
{
"kind": "OBJECT",
"name": "IssueSetAssigneesPayload",
"description": "Autogenerated return type of IssueSetAssignees",
"fields": [
{
"name": "clientMutationId",
"description": "A unique identifier for the client performing the mutation.",
"args": [
],
"type": {
"kind": "SCALAR",
"name": "String",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "errors",
"description": "Errors encountered during execution of the mutation.",
"args": [
],
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "LIST",
"name": null,
"ofType": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "SCALAR",
"name": "String",
"ofType": null
}
}
}
},
"isDeprecated": false,
"deprecationReason": null
},
{
"name": "issue",
"description": "The issue after mutation",
"args": [
],
"type": {
"kind": "OBJECT",
"name": "Issue",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
}
],
"inputFields": null,
"interfaces": [
],
"enumValues": null,
"possibleTypes": null
},
{ {
"kind": "INPUT_OBJECT", "kind": "INPUT_OBJECT",
"name": "IssueSetConfidentialInput", "name": "IssueSetConfidentialInput",
...@@ -21772,7 +21920,7 @@ ...@@ -21772,7 +21920,7 @@
}, },
{ {
"name": "assigneeUsernames", "name": "assigneeUsernames",
"description": "The usernames to assign to the merge request. Replaces existing assignees by default.\n", "description": "The usernames to assign to the resource. Replaces existing assignees by default.",
"type": { "type": {
"kind": "NON_NULL", "kind": "NON_NULL",
"name": null, "name": null,
...@@ -21794,7 +21942,7 @@ ...@@ -21794,7 +21942,7 @@
}, },
{ {
"name": "operationMode", "name": "operationMode",
"description": "The operation to perform. Defaults to REPLACE.\n", "description": "The operation to perform. Defaults to REPLACE.",
"type": { "type": {
"kind": "ENUM", "kind": "ENUM",
"name": "MutationOperationMode", "name": "MutationOperationMode",
...@@ -24435,6 +24583,33 @@ ...@@ -24435,6 +24583,33 @@
"isDeprecated": false, "isDeprecated": false,
"deprecationReason": null "deprecationReason": null
}, },
{
"name": "issueSetAssignees",
"description": null,
"args": [
{
"name": "input",
"description": null,
"type": {
"kind": "NON_NULL",
"name": null,
"ofType": {
"kind": "INPUT_OBJECT",
"name": "IssueSetAssigneesInput",
"ofType": null
}
},
"defaultValue": null
}
],
"type": {
"kind": "OBJECT",
"name": "IssueSetAssigneesPayload",
"ofType": null
},
"isDeprecated": false,
"deprecationReason": null
},
{ {
"name": "issueSetConfidential", "name": "issueSetConfidential",
"description": null, "description": null,
...@@ -947,6 +947,16 @@ Check permissions for the current user on a issue ...@@ -947,6 +947,16 @@ Check permissions for the current user on a issue
| `reopenIssue` | Boolean! | Indicates the user can perform `reopen_issue` on this resource | | `reopenIssue` | Boolean! | Indicates the user can perform `reopen_issue` on this resource |
| `updateIssue` | Boolean! | Indicates the user can perform `update_issue` on this resource | | `updateIssue` | Boolean! | Indicates the user can perform `update_issue` on this resource |
## IssueSetAssigneesPayload
Autogenerated return type of IssueSetAssignees
| Name | Type | Description |
| --- | ---- | ---------- |
| `clientMutationId` | String | A unique identifier for the client performing the mutation. |
| `errors` | String! => Array | Errors encountered during execution of the mutation. |
| `issue` | Issue | The issue after mutation |
## IssueSetConfidentialPayload ## IssueSetConfidentialPayload
Autogenerated return type of IssueSetConfidential Autogenerated return type of IssueSetConfidential
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Mutations::Issues::SetAssignees do
it_behaves_like 'a multi-assignable resource' do
let_it_be(:resource, reload: true) { create(:issue) }
end
end
...@@ -3,67 +3,7 @@ ...@@ -3,67 +3,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Mutations::MergeRequests::SetAssignees do RSpec.describe Mutations::MergeRequests::SetAssignees do
let(:merge_request) { create(:merge_request) } it_behaves_like 'a multi-assignable resource' do
let(:user) { create(:user) } let_it_be(:resource, reload: true) { create(:merge_request) }
subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
describe '#resolve' do
let(:assignees) { create_list(:user, 3) }
let(:assignee_usernames) { assignees.map(&:username) }
let(:mutated_merge_request) { subject[:merge_request] }
subject { mutation.resolve(project_path: merge_request.project.full_path, iid: merge_request.iid, assignee_usernames: assignee_usernames) }
before do
assignees.each do |user|
merge_request.project.add_developer(user)
end
end
context 'when the user can update the merge request' do
before do
merge_request.project.add_developer(user)
end
it 'sets merge request assignees' do
expect(mutated_merge_request).to eq(merge_request)
expect(mutated_merge_request.assignees).to match_array(assignees)
expect(subject[:errors]).to be_empty
end
it 'removes assignees not in the list' do
users = create_list(:user, 2)
users.each do |user|
merge_request.project.add_developer(user)
end
merge_request.assignees = users
merge_request.save!
expect(mutated_merge_request).to eq(merge_request)
expect(mutated_merge_request.assignees).to match_array(assignees)
expect(subject[:errors]).to be_empty
end
context 'when passing "append" as true' do
subject { mutation.resolve(project_path: merge_request.project.full_path, iid: merge_request.iid, assignee_usernames: assignee_usernames, operation_mode: Types::MutationOperationModeEnum.enum[:append]) }
let(:existing_assignees) { create_list(:user, 2) }
before do
existing_assignees.each do |user|
merge_request.project.add_developer(user)
end
merge_request.assignees = existing_assignees
merge_request.save!
end
it 'does not remove assignees not in the list' do
expect(mutated_merge_request).to eq(merge_request)
expect(mutated_merge_request.assignees).to match_array(assignees + existing_assignees)
expect(subject[:errors]).to be_empty
end
end
end
end end
end end
# frozen_string_literal: true
require 'spec_helper'
RSpec.shared_examples 'a multi-assignable resource' do
let_it_be(:user) { create(:user) }
subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
describe '#resolve' do
let_it_be(:assignees) { create_list(:user, 3) }
let(:assignee_usernames) { assignees.map(&:username) }
let(:mutated_resource) { subject[resource.class.name.underscore.to_sym] }
subject { mutation.resolve(project_path: resource.project.full_path, iid: resource.iid, assignee_usernames: assignee_usernames) }
before do
assignees.each do |user|
resource.project.add_developer(user)
end
end
context 'when the user can update the resource' do
before do
resource.project.add_developer(user)
end
it 'sets the assignees' do
expect(mutated_resource).to eq(resource)
expect(mutated_resource.assignees).to match_array(assignees)
expect(subject[:errors]).to be_empty
end
it 'removes assignees not in the list' do
users = create_list(:user, 2)
users.each do |user|
resource.project.add_developer(user)
end
resource.assignees = users
resource.save!
expect(mutated_resource).to eq(resource)
expect(mutated_resource.assignees).to match_array(assignees)
expect(subject[:errors]).to be_empty
end
context 'when passing "append" as true' do
subject { mutation.resolve(project_path: resource.project.full_path, iid: resource.iid, assignee_usernames: assignee_usernames, operation_mode: Types::MutationOperationModeEnum.enum[:append]) }
let(:existing_assignees) { create_list(:user, 2) }
before do
existing_assignees.each do |user|
resource.project.add_developer(user)
end
resource.assignees = existing_assignees
resource.save!
end
it 'does not remove assignees not in the list' do
expect(mutated_resource).to eq(resource)
expect(mutated_resource.assignees).to match_array(assignees + existing_assignees)
expect(subject[:errors]).to be_empty
end
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Mutations::Issues::SetAssignees do
it_behaves_like 'an assignable resource' do
let_it_be(:resource, reload: true) { create(:issue) }
end
end
...@@ -3,106 +3,7 @@ ...@@ -3,106 +3,7 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Mutations::MergeRequests::SetAssignees do RSpec.describe Mutations::MergeRequests::SetAssignees do
let(:merge_request) { create(:merge_request) } it_behaves_like 'an assignable resource' do
let(:user) { create(:user) } let_it_be(:resource, reload: true) { create(:merge_request) }
subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
describe '#resolve' do
let(:assignee) { create(:user) }
let(:assignee2) { create(:user) }
let(:assignee_usernames) { [assignee.username] }
let(:mutated_merge_request) { subject[:merge_request] }
subject { mutation.resolve(project_path: merge_request.project.full_path, iid: merge_request.iid, assignee_usernames: assignee_usernames) }
before do
merge_request.project.add_developer(assignee)
merge_request.project.add_developer(assignee2)
end
it 'raises an error if the resource is not accessible to the user' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
context 'when the user can update the merge request' do
before do
merge_request.project.add_developer(user)
end
it 'replaces the assignee' do
merge_request.assignees = [assignee2]
merge_request.save!
expect(mutated_merge_request).to eq(merge_request)
expect(mutated_merge_request.assignees).to contain_exactly(assignee)
expect(subject[:errors]).to be_empty
end
it 'returns errors merge request could not be updated' do
# Make the merge request invalid
merge_request.allow_broken = true
merge_request.update!(source_project: nil)
expect(subject[:errors]).not_to be_empty
end
context 'when passing an empty assignee list' do
let(:assignee_usernames) { [] }
before do
merge_request.assignees = [assignee]
merge_request.save!
end
it 'removes all assignees' do
expect(mutated_merge_request).to eq(merge_request)
expect(mutated_merge_request.assignees).to eq([])
expect(subject[:errors]).to be_empty
end
end
context 'when passing "append" as true' do
subject { mutation.resolve(project_path: merge_request.project.full_path, iid: merge_request.iid, assignee_usernames: assignee_usernames, operation_mode: Types::MutationOperationModeEnum.enum[:append]) }
before do
merge_request.assignees = [assignee2]
merge_request.save!
# In CE, APPEND is a NOOP as you can't have multiple assignees
# We test multiple assignment in EE specs
stub_licensed_features(multiple_merge_request_assignees: false)
end
it 'is a NO-OP in FOSS' do
expect(mutated_merge_request).to eq(merge_request)
expect(mutated_merge_request.assignees).to contain_exactly(assignee2)
expect(subject[:errors]).to be_empty
end
end
context 'when passing "remove" as true' do
before do
merge_request.assignees = [assignee]
merge_request.save!
end
it 'removes named assignee' do
mutated_merge_request = mutation.resolve(project_path: merge_request.project.full_path, iid: merge_request.iid, assignee_usernames: assignee_usernames, operation_mode: Types::MutationOperationModeEnum.enum[:remove])[:merge_request]
expect(mutated_merge_request).to eq(merge_request)
expect(mutated_merge_request.assignees).to eq([])
expect(subject[:errors]).to be_empty
end
it 'does not remove unnamed assignee' do
mutated_merge_request = mutation.resolve(project_path: merge_request.project.full_path, iid: merge_request.iid, assignee_usernames: [assignee2.username], operation_mode: Types::MutationOperationModeEnum.enum[:remove])[:merge_request]
expect(mutated_merge_request).to eq(merge_request)
expect(mutated_merge_request.assignees).to contain_exactly(assignee)
expect(subject[:errors]).to be_empty
end
end
end
end end
end end
# frozen_string_literal: true
require 'spec_helper'
RSpec.shared_examples 'an assignable resource' do
let_it_be(:user) { create(:user) }
subject(:mutation) { described_class.new(object: nil, context: { current_user: user }, field: nil) }
describe '#resolve' do
let_it_be(:assignee) { create(:user) }
let_it_be(:assignee2) { create(:user) }
let(:assignee_usernames) { [assignee.username] }
let(:mutated_resource) { subject[resource.class.name.underscore.to_sym] }
subject { mutation.resolve(project_path: resource.project.full_path, iid: resource.iid, assignee_usernames: assignee_usernames) }
before do
resource.project.add_developer(assignee)
resource.project.add_developer(assignee2)
end
it 'raises an error if the resource is not accessible to the user' do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
end
context 'when the user can update the resource' do
before do
resource.project.add_developer(user)
end
it 'replaces the assignee' do
resource.assignees = [assignee2]
resource.save!
expect(mutated_resource).to eq(resource)
expect(mutated_resource.assignees).to contain_exactly(assignee)
expect(subject[:errors]).to be_empty
end
it 'returns errors when resource could not be updated' do
allow(resource).to receive(:errors_on_object).and_return(['foo'])
expect(subject[:errors]).not_to match_array(['foo'])
end
context 'when passing an empty assignee list' do
let(:assignee_usernames) { [] }
before do
resource.assignees = [assignee]
resource.save!
end
it 'removes all assignees' do
expect(mutated_resource).to eq(resource)
expect(mutated_resource.assignees).to eq([])
expect(subject[:errors]).to be_empty
end
end
context 'when passing "append" as true' do
subject do
mutation.resolve(
project_path: resource.project.full_path,
iid: resource.iid,
assignee_usernames: assignee_usernames,
operation_mode: Types::MutationOperationModeEnum.enum[:append]
)
end
before do
resource.assignees = [assignee2]
resource.save!
# In CE, APPEND is a NOOP as you can't have multiple assignees
# We test multiple assignment in EE specs
if resource.is_a?(MergeRequest)
stub_licensed_features(multiple_merge_request_assignees: false)
else
stub_licensed_features(multiple_issue_assignees: false)
end
end
it 'is a NO-OP in FOSS' do
expect(mutated_resource).to eq(resource)
expect(mutated_resource.assignees).to contain_exactly(assignee2)
expect(subject[:errors]).to be_empty
end
end
context 'when passing "remove" as true' do
before do
resource.assignees = [assignee]
resource.save!
end
it 'removes named assignee' do
mutated_resource = mutation.resolve(
project_path: resource.project.full_path,
iid: resource.iid,
assignee_usernames: assignee_usernames,
operation_mode: Types::MutationOperationModeEnum.enum[:remove]
)[resource.class.name.underscore.to_sym]
expect(mutated_resource).to eq(resource)
expect(mutated_resource.assignees).to eq([])
expect(subject[:errors]).to be_empty
end
it 'does not remove unnamed assignee' do
mutated_resource = mutation.resolve(
project_path: resource.project.full_path,
iid: resource.iid,
assignee_usernames: [assignee2.username],
operation_mode: Types::MutationOperationModeEnum.enum[:remove]
)[resource.class.name.underscore.to_sym]
expect(mutated_resource).to eq(resource)
expect(mutated_resource.assignees).to contain_exactly(assignee)
expect(subject[:errors]).to be_empty
end
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