Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
4bafba99
Commit
4bafba99
authored
Jul 29, 2021
by
Eulyeon Ko
Committed by
Jan Provaznik
Jul 29, 2021
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Allow filtering of issues by milestone title wildcards in GraphQL
parent
dd9e040d
Changes
9
Hide whitespace changes
Inline
Side-by-side
Showing
9 changed files
with
153 additions
and
18 deletions
+153
-18
app/finders/issuable_finder.rb
app/finders/issuable_finder.rb
+2
-1
app/finders/issuable_finder/params.rb
app/finders/issuable_finder/params.rb
+20
-8
app/finders/issues_finder.rb
app/finders/issues_finder.rb
+2
-1
app/graphql/resolvers/concerns/issue_resolver_arguments.rb
app/graphql/resolvers/concerns/issue_resolver_arguments.rb
+17
-4
app/graphql/types/issues/negated_issue_filter_input_type.rb
app/graphql/types/issues/negated_issue_filter_input_type.rb
+3
-0
app/graphql/types/milestone_wildcard_id_enum.rb
app/graphql/types/milestone_wildcard_id_enum.rb
+13
-0
app/graphql/types/negated_milestone_wildcard_id_enum.rb
app/graphql/types/negated_milestone_wildcard_id_enum.rb
+11
-0
doc/api/graphql/reference/index.md
doc/api/graphql/reference/index.md
+25
-0
spec/graphql/resolvers/issues_resolver_spec.rb
spec/graphql/resolvers/issues_resolver_spec.rb
+60
-4
No files found.
app/finders/issuable_finder.rb
View file @
4bafba99
...
...
@@ -15,7 +15,8 @@
# state: 'opened' or 'closed' or 'locked' or 'all'
# group_id: integer
# project_id: integer
# milestone_title: string
# milestone_title: string (cannot be simultaneously used with milestone_wildcard_id)
# milestone_wildcard_id: 'none', 'any', 'upcoming', 'started' (cannot be simultaneously used with milestone_title)
# release_tag: string
# author_id: integer
# author_username: string
...
...
app/finders/issuable_finder/params.rb
View file @
4bafba99
...
...
@@ -4,9 +4,11 @@ class IssuableFinder
class
Params
<
SimpleDelegator
include
Gitlab
::
Utils
::
StrongMemoize
# This is used as a common filter for None / Any
# This is used as a common filter for None / Any
/ Upcoming / Started
FILTER_NONE
=
'none'
FILTER_ANY
=
'any'
FILTER_STARTED
=
'started'
FILTER_UPCOMING
=
'upcoming'
# This is used in unassigning users
NONE
=
'0'
...
...
@@ -42,25 +44,35 @@ class IssuableFinder
end
def
milestones?
params
[
:milestone_title
].
present?
params
[
:milestone_title
].
present?
||
params
[
:milestone_wildcard_id
].
present?
end
def
filter_by_no_milestone?
# Accepts `No Milestone` for compatibility
params
[
:milestone_title
].
to_s
.
downcase
==
FILTER_NONE
||
params
[
:milestone_title
]
==
Milestone
::
None
.
title
# Usage of `No Milestone` and `none`/`None` in milestone_title to be deprecated
# https://gitlab.com/gitlab-org/gitlab/-/issues/336044
params
[
:milestone_title
].
to_s
.
downcase
==
FILTER_NONE
||
params
[
:milestone_title
]
==
Milestone
::
None
.
title
||
params
[
:milestone_wildcard_id
].
to_s
.
downcase
==
FILTER_NONE
end
def
filter_by_any_milestone?
# Accepts `Any Milestone` for compatibility
params
[
:milestone_title
].
to_s
.
downcase
==
FILTER_ANY
||
params
[
:milestone_title
]
==
Milestone
::
Any
.
title
# Usage of `Any Milestone` and `any`/`Any` in milestone_title to be deprecated
# https://gitlab.com/gitlab-org/gitlab/-/issues/336044
params
[
:milestone_title
].
to_s
.
downcase
==
FILTER_ANY
||
params
[
:milestone_title
]
==
Milestone
::
Any
.
title
||
params
[
:milestone_wildcard_id
].
to_s
.
downcase
==
FILTER_ANY
end
def
filter_by_upcoming_milestone?
params
[
:milestone_title
]
==
Milestone
::
Upcoming
.
name
# Usage of `#upcoming` in milestone_title to be deprecated
# https://gitlab.com/gitlab-org/gitlab/-/issues/336044
params
[
:milestone_title
]
==
Milestone
::
Upcoming
.
name
||
params
[
:milestone_wildcard_id
].
to_s
.
downcase
==
FILTER_UPCOMING
end
def
filter_by_started_milestone?
params
[
:milestone_title
]
==
Milestone
::
Started
.
name
# Usage of `#started` in milestone_title to be deprecated
# https://gitlab.com/gitlab-org/gitlab/-/issues/336044
params
[
:milestone_title
]
==
Milestone
::
Started
.
name
||
params
[
:milestone_wildcard_id
].
to_s
.
downcase
==
FILTER_STARTED
end
def
filter_by_no_release?
...
...
app/finders/issues_finder.rb
View file @
4bafba99
...
...
@@ -11,7 +11,8 @@
# state: 'opened' or 'closed' or 'all'
# group_id: integer
# project_id: integer
# milestone_title: string
# milestone_title: string (cannot be simultaneously used with milestone_wildcard_id)
# milestone_wildcard_id: 'none', 'any', 'upcoming', 'started' (cannot be simultaneously used with milestone_title)
# assignee_id: integer
# search: string
# in: 'title', 'description', or a string joining them with comma
...
...
app/graphql/resolvers/concerns/issue_resolver_arguments.rb
View file @
4bafba99
...
...
@@ -56,6 +56,9 @@ module IssueResolverArguments
as: :issue_types
,
description:
'Filter issues by the given issue types.'
,
required:
false
argument
:milestone_wildcard_id
,
::
Types
::
MilestoneWildcardIdEnum
,
required:
false
,
description:
'Filter issues by milestone ID wildcard.'
argument
:not
,
Types
::
Issues
::
NegatedIssueFilterInputType
,
description:
'Negated arguments.'
,
prepare:
->
(
negated_args
,
ctx
)
{
negated_args
.
to_h
},
...
...
@@ -82,10 +85,9 @@ module IssueResolverArguments
end
def
ready?
(
**
args
)
if
args
.
slice
(
*
mutually_exclusive_assignee_username_args
).
compact
.
size
>
1
arg_str
=
mutually_exclusive_assignee_username_args
.
map
{
|
x
|
x
.
to_s
.
camelize
(
:lower
)
}.
join
(
', '
)
raise
Gitlab
::
Graphql
::
Errors
::
ArgumentError
,
"only one of [
#{
arg_str
}
] arguments is allowed at the same time."
end
params_not_mutually_exclusive
(
args
,
mutually_exclusive_assignee_username_args
)
params_not_mutually_exclusive
(
args
,
mutually_exclusive_milestone_args
)
params_not_mutually_exclusive
(
args
.
fetch
(
:not
,
{}),
mutually_exclusive_milestone_args
)
super
end
...
...
@@ -106,6 +108,17 @@ module IssueResolverArguments
args
[
:not
][
:assignee_username
]
=
args
[
:not
].
delete
(
:assignee_usernames
)
if
args
.
dig
(
:not
,
:assignee_usernames
).
present?
end
def
params_not_mutually_exclusive
(
args
,
mutually_exclusive_args
)
if
args
.
slice
(
*
mutually_exclusive_args
).
compact
.
size
>
1
arg_str
=
mutually_exclusive_args
.
map
{
|
x
|
x
.
to_s
.
camelize
(
:lower
)
}.
join
(
', '
)
raise
::
Gitlab
::
Graphql
::
Errors
::
ArgumentError
,
"only one of [
#{
arg_str
}
] arguments is allowed at the same time."
end
end
def
mutually_exclusive_milestone_args
[
:milestone_title
,
:milestone_wildcard_id
]
end
def
mutually_exclusive_assignee_username_args
[
:assignee_usernames
,
:assignee_username
]
end
...
...
app/graphql/types/issues/negated_issue_filter_input_type.rb
View file @
4bafba99
...
...
@@ -20,6 +20,9 @@ module Types
argument
:assignee_id
,
GraphQL
::
Types
::
String
,
required:
false
,
description:
'ID of a user not assigned to the issues.'
argument
:milestone_wildcard_id
,
::
Types
::
NegatedMilestoneWildcardIdEnum
,
required:
false
,
description:
'Filter by negated milestone wildcard values.'
end
end
end
...
...
app/graphql/types/milestone_wildcard_id_enum.rb
0 → 100644
View file @
4bafba99
# frozen_string_literal: true
module
Types
class
MilestoneWildcardIdEnum
<
BaseEnum
graphql_name
'MilestoneWildcardId'
description
'Milestone ID wildcard values'
value
'NONE'
,
'No milestone is assigned.'
value
'ANY'
,
'A milestone is assigned.'
value
'STARTED'
,
'An open, started milestone (start date <= today).'
value
'UPCOMING'
,
'An open milestone due in the future (due date >= today).'
end
end
app/graphql/types/negated_milestone_wildcard_id_enum.rb
0 → 100644
View file @
4bafba99
# frozen_string_literal: true
module
Types
class
NegatedMilestoneWildcardIdEnum
<
BaseEnum
graphql_name
'NegatedMilestoneWildcardId'
description
'Negated Milestone ID wildcard values'
value
'STARTED'
,
'An open, started milestone (start date <= today).'
value
'UPCOMING'
,
'An open milestone due in the future (due date >= today).'
end
end
doc/api/graphql/reference/index.md
View file @
4bafba99
...
...
@@ -9662,6 +9662,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
|
<a
id=
"groupissuesiterationwildcardid"
></a>
`iterationWildcardId`
|
[
`IterationWildcardId`
](
#iterationwildcardid
)
| Filter by iteration ID wildcard. |
|
<a
id=
"groupissueslabelname"
></a>
`labelName`
|
[
`[String]`
](
#string
)
| Labels applied to this issue. |
|
<a
id=
"groupissuesmilestonetitle"
></a>
`milestoneTitle`
|
[
`[String]`
](
#string
)
| Milestone applied to this issue. |
|
<a
id=
"groupissuesmilestonewildcardid"
></a>
`milestoneWildcardId`
|
[
`MilestoneWildcardId`
](
#milestonewildcardid
)
| Filter issues by milestone ID wildcard. |
|
<a
id=
"groupissuesnot"
></a>
`not`
|
[
`NegatedIssueFilterInput`
](
#negatedissuefilterinput
)
| Negated arguments. |
|
<a
id=
"groupissuessearch"
></a>
`search`
|
[
`String`
](
#string
)
| Search query for issue title or description. |
|
<a
id=
"groupissuessort"
></a>
`sort`
|
[
`IssueSort`
](
#issuesort
)
| Sort issues by this criteria. |
...
...
@@ -11902,6 +11903,7 @@ Returns [`Issue`](#issue).
|
<a
id=
"projectissueiterationwildcardid"
></a>
`iterationWildcardId`
|
[
`IterationWildcardId`
](
#iterationwildcardid
)
| Filter by iteration ID wildcard. |
|
<a
id=
"projectissuelabelname"
></a>
`labelName`
|
[
`[String]`
](
#string
)
| Labels applied to this issue. |
|
<a
id=
"projectissuemilestonetitle"
></a>
`milestoneTitle`
|
[
`[String]`
](
#string
)
| Milestone applied to this issue. |
|
<a
id=
"projectissuemilestonewildcardid"
></a>
`milestoneWildcardId`
|
[
`MilestoneWildcardId`
](
#milestonewildcardid
)
| Filter issues by milestone ID wildcard. |
|
<a
id=
"projectissuenot"
></a>
`not`
|
[
`NegatedIssueFilterInput`
](
#negatedissuefilterinput
)
| Negated arguments. |
|
<a
id=
"projectissuesearch"
></a>
`search`
|
[
`String`
](
#string
)
| Search query for issue title or description. |
|
<a
id=
"projectissuesort"
></a>
`sort`
|
[
`IssueSort`
](
#issuesort
)
| Sort issues by this criteria. |
...
...
@@ -11933,6 +11935,7 @@ Returns [`IssueStatusCountsType`](#issuestatuscountstype).
|
<a
id=
"projectissuestatuscountsiids"
></a>
`iids`
|
[
`[String!]`
](
#string
)
| List of IIDs of issues. For example,
`["1", "2"]`
. |
|
<a
id=
"projectissuestatuscountslabelname"
></a>
`labelName`
|
[
`[String]`
](
#string
)
| Labels applied to this issue. |
|
<a
id=
"projectissuestatuscountsmilestonetitle"
></a>
`milestoneTitle`
|
[
`[String]`
](
#string
)
| Milestone applied to this issue. |
|
<a
id=
"projectissuestatuscountsmilestonewildcardid"
></a>
`milestoneWildcardId`
|
[
`MilestoneWildcardId`
](
#milestonewildcardid
)
| Filter issues by milestone ID wildcard. |
|
<a
id=
"projectissuestatuscountsnot"
></a>
`not`
|
[
`NegatedIssueFilterInput`
](
#negatedissuefilterinput
)
| Negated arguments. |
|
<a
id=
"projectissuestatuscountssearch"
></a>
`search`
|
[
`String`
](
#string
)
| Search query for issue title or description. |
|
<a
id=
"projectissuestatuscountstypes"
></a>
`types`
|
[
`[IssueType!]`
](
#issuetype
)
| Filter issues by the given issue types. |
...
...
@@ -11968,6 +11971,7 @@ four standard [pagination arguments](#connection-pagination-arguments):
|
<a
id=
"projectissuesiterationwildcardid"
></a>
`iterationWildcardId`
|
[
`IterationWildcardId`
](
#iterationwildcardid
)
| Filter by iteration ID wildcard. |
|
<a
id=
"projectissueslabelname"
></a>
`labelName`
|
[
`[String]`
](
#string
)
| Labels applied to this issue. |
|
<a
id=
"projectissuesmilestonetitle"
></a>
`milestoneTitle`
|
[
`[String]`
](
#string
)
| Milestone applied to this issue. |
|
<a
id=
"projectissuesmilestonewildcardid"
></a>
`milestoneWildcardId`
|
[
`MilestoneWildcardId`
](
#milestonewildcardid
)
| Filter issues by milestone ID wildcard. |
|
<a
id=
"projectissuesnot"
></a>
`not`
|
[
`NegatedIssueFilterInput`
](
#negatedissuefilterinput
)
| Negated arguments. |
|
<a
id=
"projectissuessearch"
></a>
`search`
|
[
`String`
](
#string
)
| Search query for issue title or description. |
|
<a
id=
"projectissuessort"
></a>
`sort`
|
[
`IssueSort`
](
#issuesort
)
| Sort issues by this criteria. |
...
...
@@ -15044,6 +15048,17 @@ Current state of milestone.
|
<a
id=
"milestonestateenumactive"
></a>
`active`
| Milestone is currently active. |
|
<a
id=
"milestonestateenumclosed"
></a>
`closed`
| Milestone is closed. |
### `MilestoneWildcardId`
Milestone ID wildcard values.
| Value | Description |
| ----- | ----------- |
|
<a
id=
"milestonewildcardidany"
></a>
`ANY`
| A milestone is assigned. |
|
<a
id=
"milestonewildcardidnone"
></a>
`NONE`
| No milestone is assigned. |
|
<a
id=
"milestonewildcardidstarted"
></a>
`STARTED`
| An open, started milestone (start date <= today). |
|
<a
id=
"milestonewildcardidupcoming"
></a>
`UPCOMING`
| An open milestone due in the future (due date >= today). |
### `MoveType`
The position to which the adjacent object should be moved.
...
...
@@ -15080,6 +15095,15 @@ Negated Iteration ID wildcard values.
| ----- | ----------- |
|
<a
id=
"negatediterationwildcardidcurrent"
></a>
`CURRENT`
| Current iteration. |
### `NegatedMilestoneWildcardId`
Negated Milestone ID wildcard values.
| Value | Description |
| ----- | ----------- |
|
<a
id=
"negatedmilestonewildcardidstarted"
></a>
`STARTED`
| An open, started milestone (start date <= today). |
|
<a
id=
"negatedmilestonewildcardidupcoming"
></a>
`UPCOMING`
| An open milestone due in the future (due date >= today). |
### `NetworkPolicyKind`
Kind of the network policy.
...
...
@@ -16774,6 +16798,7 @@ Represents an escalation rule.
|
<a
id=
"negatedissuefilterinputiterationwildcardid"
></a>
`iterationWildcardId`
|
[
`IterationWildcardId`
](
#iterationwildcardid
)
| Filter by negated iteration ID wildcard. |
|
<a
id=
"negatedissuefilterinputlabelname"
></a>
`labelName`
|
[
`[String!]`
](
#string
)
| Labels not applied to this issue. |
|
<a
id=
"negatedissuefilterinputmilestonetitle"
></a>
`milestoneTitle`
|
[
`[String!]`
](
#string
)
| Milestone not applied to this issue. |
|
<a
id=
"negatedissuefilterinputmilestonewildcardid"
></a>
`milestoneWildcardId`
|
[
`NegatedMilestoneWildcardId`
](
#negatedmilestonewildcardid
)
| Filter by negated milestone wildcard values. |
|
<a
id=
"negatedissuefilterinputweight"
></a>
`weight`
|
[
`String`
](
#string
)
| Weight not applied to the issue. |
### `OncallRotationActivePeriodInputType`
...
...
spec/graphql/resolvers/issues_resolver_spec.rb
View file @
4bafba99
...
...
@@ -11,9 +11,9 @@ RSpec.describe Resolvers::IssuesResolver do
let_it_be
(
:project
)
{
create
(
:project
,
group:
group
)
}
let_it_be
(
:other_project
)
{
create
(
:project
,
group:
group
)
}
let_it_be
(
:
milestone
)
{
create
(
:milestone
,
project:
project
)
}
let_it_be
(
:
started_milestone
)
{
create
(
:milestone
,
project:
project
,
title:
"started milestone"
,
start_date:
1
.
day
.
ago
)
}
let_it_be
(
:assignee
)
{
create
(
:user
)
}
let_it_be
(
:issue1
)
{
create
(
:incident
,
project:
project
,
state: :opened
,
created_at:
3
.
hours
.
ago
,
updated_at:
3
.
hours
.
ago
,
milestone:
milestone
)
}
let_it_be
(
:issue1
)
{
create
(
:incident
,
project:
project
,
state: :opened
,
created_at:
3
.
hours
.
ago
,
updated_at:
3
.
hours
.
ago
,
milestone:
started_
milestone
)
}
let_it_be
(
:issue2
)
{
create
(
:issue
,
project:
project
,
state: :closed
,
title:
'foo'
,
created_at:
1
.
hour
.
ago
,
updated_at:
1
.
hour
.
ago
,
closed_at:
1
.
hour
.
ago
,
assignees:
[
assignee
])
}
let_it_be
(
:issue3
)
{
create
(
:issue
,
project:
other_project
,
state: :closed
,
title:
'foo'
,
created_at:
1
.
hour
.
ago
,
updated_at:
1
.
hour
.
ago
,
closed_at:
1
.
hour
.
ago
,
assignees:
[
assignee
])
}
let_it_be
(
:issue4
)
{
create
(
:issue
)
}
...
...
@@ -43,7 +43,63 @@ RSpec.describe Resolvers::IssuesResolver do
end
it
'filters by milestone'
do
expect
(
resolve_issues
(
milestone_title:
[
milestone
.
title
])).
to
contain_exactly
(
issue1
)
expect
(
resolve_issues
(
milestone_title:
[
started_milestone
.
title
])).
to
contain_exactly
(
issue1
)
end
describe
'filtering by milestone wildcard id'
do
let_it_be
(
:upcoming_milestone
)
{
create
(
:milestone
,
project:
project
,
title:
"upcoming milestone"
,
start_date:
1
.
day
.
ago
,
due_date:
1
.
day
.
from_now
)
}
let_it_be
(
:past_milestone
)
{
create
(
:milestone
,
project:
project
,
title:
"past milestone"
,
due_date:
1
.
day
.
ago
)
}
let_it_be
(
:future_milestone
)
{
create
(
:milestone
,
project:
project
,
title:
"future milestone"
,
start_date:
1
.
day
.
from_now
)
}
let_it_be
(
:issue5
)
{
create
(
:issue
,
project:
project
,
state: :opened
,
milestone:
upcoming_milestone
)
}
let_it_be
(
:issue6
)
{
create
(
:issue
,
project:
project
,
state: :opened
,
milestone:
past_milestone
)
}
let_it_be
(
:issue7
)
{
create
(
:issue
,
project:
project
,
state: :opened
,
milestone:
future_milestone
)
}
let
(
:wildcard_started
)
{
'STARTED'
}
let
(
:wildcard_upcoming
)
{
'UPCOMING'
}
let
(
:wildcard_any
)
{
'ANY'
}
let
(
:wildcard_none
)
{
'NONE'
}
it
'returns issues with started milestone'
do
expect
(
resolve_issues
(
milestone_wildcard_id:
wildcard_started
)).
to
contain_exactly
(
issue1
,
issue5
)
end
it
'returns issues with upcoming milestone'
do
expect
(
resolve_issues
(
milestone_wildcard_id:
wildcard_upcoming
)).
to
contain_exactly
(
issue5
)
end
it
'returns issues with any milestone'
do
expect
(
resolve_issues
(
milestone_wildcard_id:
wildcard_any
)).
to
contain_exactly
(
issue1
,
issue5
,
issue6
,
issue7
)
end
it
'returns issues with no milestone'
do
expect
(
resolve_issues
(
milestone_wildcard_id:
wildcard_none
)).
to
contain_exactly
(
issue2
)
end
it
'raises a mutually exclusive filter error when wildcard and title are provided'
do
expect
do
resolve_issues
(
milestone_title:
[
"started milestone"
],
milestone_wildcard_id:
wildcard_started
)
end
.
to
raise_error
(
Gitlab
::
Graphql
::
Errors
::
ArgumentError
,
'only one of [milestoneTitle, milestoneWildcardId] arguments is allowed at the same time.'
)
end
context
'negated filtering'
do
it
'returns issues matching the searched title after applying a negated filter'
do
expect
(
resolve_issues
(
milestone_title:
[
'past milestone'
],
not:
{
milestone_wildcard_id:
wildcard_upcoming
})).
to
contain_exactly
(
issue6
)
end
it
'returns issues excluding the ones with started milestone'
do
expect
(
resolve_issues
(
not:
{
milestone_wildcard_id:
wildcard_started
})).
to
contain_exactly
(
issue7
)
end
it
'returns issues excluding the ones with upcoming milestone'
do
expect
(
resolve_issues
(
not:
{
milestone_wildcard_id:
wildcard_upcoming
})).
to
contain_exactly
(
issue6
)
end
it
'raises a mutually exclusive filter error when wildcard and title are provided as negated filters'
do
expect
do
resolve_issues
(
not:
{
milestone_title:
[
"started milestone"
],
milestone_wildcard_id:
wildcard_started
})
end
.
to
raise_error
(
Gitlab
::
Graphql
::
Errors
::
ArgumentError
,
'only one of [milestoneTitle, milestoneWildcardId] arguments is allowed at the same time.'
)
end
end
end
it
'filters by two assignees'
do
...
...
@@ -169,7 +225,7 @@ RSpec.describe Resolvers::IssuesResolver do
end
it
'returns issues without the specified milestone'
do
expect
(
resolve_issues
(
not:
{
milestone_title:
[
milestone
.
title
]
})).
to
contain_exactly
(
issue2
)
expect
(
resolve_issues
(
not:
{
milestone_title:
[
started_
milestone
.
title
]
})).
to
contain_exactly
(
issue2
)
end
it
'returns issues without the specified assignee_usernames'
do
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment