Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
erp5
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
0
Merge Requests
0
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
Eteri
erp5
Commits
2f41d248
Commit
2f41d248
authored
Apr 20, 2018
by
Tomáš Peterka
Committed by
Tomáš Peterka
Apr 25, 2018
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
[hal_json_style] Add and document `extra_param_json` usage
parent
4c273cae
Changes
4
Show whitespace changes
Inline
Side-by-side
Showing
4 changed files
with
96 additions
and
29 deletions
+96
-29
bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_callDialogMethod.py
...portal_skins/erp5_hal_json_style/Base_callDialogMethod.py
+23
-12
bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_callDialogMethod.xml
...ortal_skins/erp5_hal_json_style/Base_callDialogMethod.xml
+1
-1
bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py
...rtal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py
+67
-12
bt5/erp5_hal_json_style/TestTemplateItem/portal_components/test.erp5.testHalJsonStyle.py
...plateItem/portal_components/test.erp5.testHalJsonStyle.py
+5
-4
No files found.
bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_callDialogMethod.py
View file @
2f41d248
"""
Generic method called when submitting a form in dialog mode.
Responsible for validating form data and redirecting to the form action.
Please note that the new UI has deprecated use of Selections. Your scripts
will no longer receive `selection_name` nor `selection` in their arguments.
There are runtime values hidden in every form (injected by getHateoas Script):
form_id - previous form ID (backward compatibility reasons)
dialog_id - current form dialog ID
dialog_method - method to be called - can be either update_method or dialog_method of the Dialog Form
"""
from
Products.ERP5Type.Log
import
log
,
DEBUG
,
INFO
,
WARNING
from
Products.ERP5Type.Log
import
log
,
DEBUG
,
INFO
,
WARNING
,
ERROR
from
Products.Formulator.Errors
import
FormValidationError
,
ValidationError
from
ZTUtils
import
make_query
import
json
# XXX We should not use meta_type properly,
# XXX We need to discuss this problem.(yusei)
def
isFieldType
(
field
,
type_name
):
if
field
.
meta_type
==
'ProxyField'
:
field
=
field
.
getRecursiveTemplateField
()
return
field
.
meta_type
==
type_name
from
Products.Formulator.Errors
import
FormValidationError
,
ValidationError
from
ZTUtils
import
make_query
# Kato: I do not understand why we throw away REQUEST from parameters (hidden in **kw)
# and use container.REQUEST just to introduce yet another global state. Maybe because
# container.REQUEST is used in other places.
...
...
@@ -27,30 +32,31 @@ request = kw.get('REQUEST', None) or container.REQUEST
request_form
=
request
.
form
error_message
=
''
translate
=
context
.
Base_translateString
portal
=
context
.
getPortalObject
()
# Make this script work alike no matter if called by a script or a request
kw
.
update
(
request_form
)
# Exceptions for UI
if
dialog_method
==
'Base_configureUI'
:
return
context
.
Base_configureUI
(
form_id
=
kw
[
'form_id'
]
,
return
context
.
Base_configureUI
(
form_id
=
form_id
,
selection_name
=
kw
[
'selection_name'
],
field_columns
=
kw
[
'field_columns'
],
stat_columns
=
kw
[
'stat_columns'
])
# Exceptions for Sort
if
dialog_method
==
'Base_configureSortOn'
:
return
context
.
Base_configureSortOn
(
form_id
=
kw
[
'form_id'
]
,
return
context
.
Base_configureSortOn
(
form_id
=
form_id
,
selection_name
=
kw
[
'selection_name'
],
field_sort_on
=
kw
[
'field_sort_on'
],
field_sort_order
=
kw
[
'field_sort_order'
])
# Exceptions for Workflow
if
dialog_method
==
'Workflow_statusModify'
:
return
context
.
Workflow_statusModify
(
form_id
=
kw
[
'form_id'
]
,
return
context
.
Workflow_statusModify
(
form_id
=
form_id
,
dialog_id
=
dialog_id
)
# Exception for edit relation
if
dialog_method
==
'Base_editRelation'
:
return
context
.
Base_editRelation
(
form_id
=
kw
[
'form_id'
]
,
return
context
.
Base_editRelation
(
form_id
=
form_id
,
field_id
=
kw
[
'field_id'
],
selection_name
=
kw
[
'list_selection_name'
],
selection_index
=
kw
[
'selection_index'
],
...
...
@@ -60,7 +66,7 @@ if dialog_method == 'Base_editRelation':
# Exception for create relation
# Not used in new UI - relation field implemented using JIO calls from JS
if
dialog_method
==
'Base_createRelation'
:
return
context
.
Base_createRelation
(
form_id
=
kw
[
'form_id'
]
,
return
context
.
Base_createRelation
(
form_id
=
form_id
,
selection_name
=
kw
[
'list_selection_name'
],
selection_index
=
kw
[
'selection_index'
],
base_category
=
kw
[
'base_category'
],
...
...
@@ -77,6 +83,7 @@ if dialog_method == 'Folder_delete':
md5_object_uid_list
=
kw
[
'md5_object_uid_list'
])
form
=
getattr
(
context
,
dialog_id
)
extra_param
=
json
.
loads
(
extra_param_json
or
"{}"
)
# form can be a python script that returns the form
if
not
hasattr
(
form
,
'validate_all_to_request'
):
...
...
@@ -188,6 +195,10 @@ if listbox_uid is not None and kw.has_key('list_selection_name'):
# Remove empty values for make_query.
clean_kw
=
dict
((
k
,
v
)
for
k
,
v
in
kw
.
items
()
if
v
not
in
(
None
,
[],
()))
# Add rest of extra param into arguments of the target method
kw
.
update
(
extra_param
)
# Finally we will call the Dialog Method
# Handle deferred style, unless we are executing the update action
if
dialog_method
!=
update_method
and
clean_kw
.
get
(
'deferred_style'
,
0
):
clean_kw
[
'deferred_portal_skin'
]
=
clean_kw
.
get
(
'portal_skin'
,
None
)
...
...
bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/Base_callDialogMethod.xml
View file @
2f41d248
...
...
@@ -50,7 +50,7 @@
</item>
<item>
<key>
<string>
_params
</string>
</key>
<value>
<string>
dialog_method, dialog_id,
dialog_category=\'\', update_method=None
, **kw
</string>
</value>
<value>
<string>
dialog_method, dialog_id,
form_id, dialog_category=\'\', update_method=None, extra_param_json="{}"
, **kw
</string>
</value>
</item>
<item>
<key>
<string>
id
</string>
</key>
...
...
bt5/erp5_hal_json_style/SkinTemplateItem/portal_skins/erp5_hal_json_style/ERP5Document_getHateoas.py
View file @
2f41d248
...
...
@@ -21,15 +21,21 @@ Parameters for mode == 'search'
:param .form_id: In case of page_template = "form" it will be similar to form_relative_url with the exception that it contains
only the form name (e.g. FooModule_viewFooList). In case of dialogs it points to the previous form which is
often more important than the dialog form.
:param extra_param_json: {str} BASE64 encoded JSON with parameters for getHateoas script. Content will be put to the REQUEST so
it is accessible to all Scripts and TALES expressions.
Parameters for mode == 'form'
:param form: {instace} of a form - obviously this call can be only internal (Script-to-Script)
:param extra_param_json: {dict} extra fields to be added to the rendered form
Parameters for mode == 'traverse'
Traverse renders arbitrary View. It can be a Form or a Script.
:param relative_url: string, MANDATORY for obtaining the traversed_document. Calling this script directly on an object should be
forbidden in code (but it is not now).
:param view: {str} mandatory. the view reference as defined on a Portal Type (e.g. "view" or "publish_view")
:param extra_param_json: {str} BASE64 encoded JSON with parameters for getHateoas script. Content will be put to the REQUEST so
it is accessible to all Scripts and TALES expressions. If view contains embedded **dialog** form then
fields will be added to that form to preserve the values for the next step.
# Form
When handling form, we can expect field values to be stored in REQUEST.form in two forms
...
...
@@ -549,7 +555,7 @@ def getFieldDefault(form, field, key, value=None):
return
value
def
renderField
(
traversed_document
,
field
,
form
,
value
=
None
,
meta_type
=
None
,
key
=
None
,
key_prefix
=
None
,
selection_params
=
None
,
request_field
=
True
,
extra_param_json
=
None
):
def
renderField
(
traversed_document
,
field
,
form
,
value
=
None
,
meta_type
=
None
,
key
=
None
,
key_prefix
=
None
,
selection_params
=
None
,
request_field
=
True
):
"""Extract important field's attributes into `result` dictionary."""
if
selection_params
is
None
:
selection_params
=
{}
...
...
@@ -827,17 +833,12 @@ def renderField(traversed_document, field, form, value=None, meta_type=None, key
# listbox's default parameters
default_params
.
update
(
selection_params
)
if
extra_param_json
is
not
None
:
default_params
.
update
(
ensureDeserialized
(
byteify
(
json
.
loads
(
urlsafe_b64decode
(
extra_param_json
)))))
# ListBoxes in report view has portal_type defined already in default_params
# in that case we prefer non_empty version
list_method_query_dict
=
default_params
.
copy
()
if
not
list_method_query_dict
.
get
(
"portal_type"
,
[])
and
portal_type_list
:
list_method_query_dict
[
"portal_type"
]
=
[
x
[
0
]
for
x
in
portal_type_list
]
list_method_custom
=
None
# Search for non-editable documents - all reports goes here
# Reports have custom search scripts which wants parameters from the form
# thus we introspect such parameters and try to find them in REQUEST
...
...
@@ -1002,6 +1003,9 @@ def renderForm(traversed_document, form, response_dict, key_prefix=None, selecti
previous_request_other
=
{}
REQUEST
.
set
(
'here'
,
traversed_document
)
if
extra_param_json
is
None
:
extra_param_json
=
{}
# Following pop/push of form_id resp. dialog_id is here because of FormBox - an embedded form in a form
# Fields of forms use form_id in their TALES expressions and obviously FormBox's form_id is different
# from its parent's form. It is very important that we do not remove form_id in case of a Dialog Form.
...
...
@@ -1077,7 +1081,7 @@ def renderForm(traversed_document, form, response_dict, key_prefix=None, selecti
if
not
field
.
get_value
(
"enabled"
):
continue
try
:
response_dict
[
field
.
id
]
=
renderField
(
traversed_document
,
field
,
form
,
key_prefix
=
key_prefix
,
selection_params
=
selection_params
,
extra_param_json
=
extra_param_json
)
response_dict
[
field
.
id
]
=
renderField
(
traversed_document
,
field
,
form
,
key_prefix
=
key_prefix
,
selection_params
=
selection_params
)
if
field_errors
.
has_key
(
field
.
id
):
response_dict
[
field
.
id
][
"error_text"
]
=
field_errors
[
field
.
id
].
error_text
except
AttributeError
as
error
:
...
...
@@ -1232,6 +1236,7 @@ def renderFormDefinition(form, response_dict):
# every form dialog has its dialog_id and meta (control) attributes in extra_param_json
group_list
[
-
1
][
1
].
extend
([
(
'dialog_id'
,
{
'meta_type'
:
'StringField'
}),
(
'extra_param_json'
,
{
'meta_type'
:
'StringField'
})
])
response_dict
[
"group_list"
]
=
group_list
...
...
@@ -1261,7 +1266,7 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
response
=
None
,
view
=
None
,
mode
=
None
,
query
=
None
,
select_list
=
None
,
limit
=
None
,
form
=
None
,
relative_url
=
None
,
restricted
=
None
,
list_method
=
None
,
default_param_json
=
None
,
form_relative_url
=
None
):
default_param_json
=
None
,
form_relative_url
=
None
,
extra_param_json
=
None
):
if
relative_url
:
try
:
...
...
@@ -1270,6 +1275,12 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
is_site_root
=
False
except
:
raise
NotImplementedError
(
relative_url
)
# extra_param_json holds parameters for search interpreted by getHateoas itself
# not by the list_method neither url_columns - only getHateoas
if
extra_param_json
is
None
:
extra_param_json
=
{}
result_dict
=
{
'_debug'
:
mode
,
'_links'
:
{
...
...
@@ -1345,6 +1356,17 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
result_dict
[
'title'
]
=
traversed_document
.
getTitle
()
# extra_param_json should be base64 encoded JSON at this point
# only for mode == 'form' it is already a dictionary
if
not
extra_param_json
:
extra_param_json
=
{}
if
isinstance
(
extra_param_json
,
str
):
extra_param_json
=
ensureDeserialized
(
json
.
loads
(
urlsafe_b64decode
(
extra_param_json
)))
for
key
,
value
in
extra_param_json
.
items
():
REQUEST
.
set
(
key
,
value
)
# Add a link to the portal type if possible
if
not
is_portal
:
# traversed_document should always have its Portal Type in ERP5 Portal Types
...
...
@@ -1396,6 +1418,11 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
}
}
if
view_instance
.
pt
==
"form_dialog"
:
# If there is a "form_id" in the REQUEST then it means that last view was actually a form
# and we are most likely in a dialog. We save previous form into `last_form_id` ...
last_form_id
=
REQUEST
.
get
(
'form_id'
,
""
)
if
REQUEST
is
not
None
else
""
# Put all query parameters (?reset:int=1&workflow_action=start_action) in request to mimic usual form display
# Request is later used for method's arguments discovery so set URL params into REQUEST (just like it was sent by form)
for
query_key
,
query_value
in
current_action
[
'params'
].
items
():
...
...
@@ -1422,8 +1449,13 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
return
traversed_document
.
Base_redirect
(
keep_items
=
{
'portal_status_message'
:
status_message
})
# Sometimes callables (form dialog's method, listbox's list method...) do not touch
# REQUEST but expect all (formerly) URL query parameters to appear in their **kw
# thus we send extra_param_json (=rjs way of passing parameters to REQUEST) as
# selection_params so they get into callable's **kw.
renderForm
(
traversed_document
,
view_instance
,
embedded_dict
,
selection_params
=
extra_param_json
,
extra_param_json
=
extra_param_json
)
renderForm
(
traversed_document
,
renderer_form
,
embedded_dict
,
extra_param_json
=
extra_param_json
)
result_dict
[
'_embedded'
]
=
{
'_view'
:
embedded_dict
}
...
...
@@ -1453,12 +1485,18 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
# but when we do not have the last form id we do not pass is of course
if
not
(
current_action
.
get
(
'view_id'
,
''
)
or
last_form_id
):
url_template_key
=
"traverse_generator"
# some dialogs need previous form_id when rendering to pass UID to embedded Listbox
extra_param_json
[
'form_id'
]
=
current_action
[
'view_id'
]
\
if
current_action
.
get
(
'view_id'
,
''
)
and
view_instance
.
pt
in
(
"form_view"
,
"form_list"
)
\
else
last_form_id
erp5_action_list
[
-
1
][
'href'
]
=
url_template_dict
[
url_template_key
]
%
{
"root_url"
:
site_root
.
absolute_url
(),
"script_id"
:
script
.
id
,
# this script (ERP5Document_getHateoas)
"relative_url"
:
traversed_document
.
getRelativeUrl
().
replace
(
"/"
,
"%2F"
),
"view"
:
erp5_action_list
[
-
1
][
'name'
],
"
form_id"
:
form_id
if
form_id
and
renderer_form
.
pt
==
"form_view"
else
last_form_id
"
extra_param_json"
:
urlsafe_b64encode
(
json
.
dumps
(
ensureSerializable
(
extra_param_json
)))
}
if
erp5_action_key
==
'object_jump'
:
...
...
@@ -1601,6 +1639,7 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
# limit: [15, 16] (begin_index, num_records)
# local_roles: TODO
# selection_domain: JSON string: {region: 'foo/bar'}
# extra_param_json: <base64 encoded JSON> (paramters for getHateoas itself)
#
# Default Param JSON contains
# portal_type: list of Portal Types to include (singular form matches the
...
...
@@ -1619,6 +1658,21 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
# set 'here' for field rendering which contain TALES expressions
REQUEST
.
set
(
'here'
,
traversed_document
)
# Put all items from extra_param_json into the REQUEST. It is the only
# way how we can keep state, which is required by some actions for example
# search issued from dialog needs previous form_id because often it just
# copies the previous search thus we need to pass it and we do not want
# to introduce another parameter to getHateos so we reuse `form`
# This is needed for example for erp5_core/Folder_viewDeleteDialog/listbox
# (see TALES expression for form_id and field_id there)
if
not
extra_param_json
:
extra_param_json
=
{}
if
isinstance
(
extra_param_json
,
str
):
extra_param_json
=
ensureDeserialized
(
json
.
loads
(
urlsafe_b64decode
(
extra_param_json
)))
for
key
,
value
in
extra_param_json
.
items
():
REQUEST
.
set
(
key
,
value
)
# in case we have custom list method
catalog_kw
=
{}
...
...
@@ -1995,7 +2049,7 @@ def calculateHateoas(is_portal=None, is_site_root=None, traversed_document=None,
response
.
setStatus
(
405
)
return
""
renderForm
(
traversed_document
,
form
,
result_dict
)
renderForm
(
traversed_document
,
form
,
result_dict
,
extra_param_json
=
extra_param_json
)
elif
mode
==
'newContent'
:
#################################################
...
...
@@ -2122,7 +2176,8 @@ hateoas = calculateHateoas(is_portal=temp_is_portal, is_site_root=temp_is_site_r
query=query, select_list=select_list, limit=limit, form=form,
restricted=restricted, list_method=list_method,
default_param_json=default_param_json,
form_relative_url=form_relative_url)
form_relative_url=form_relative_url,
extra_param_json=extra_param_json)
if hateoas == "":
return hateoas
else:
...
...
bt5/erp5_hal_json_style/TestTemplateItem/portal_components/test.erp5.testHalJsonStyle.py
View file @
2f41d248
...
...
@@ -567,7 +567,7 @@ class TestERP5Document_getHateoas_mode_traverse(ERP5HALJSONStyleSkinsMixin):
self
.
assertEqual
(
result_dict
[
'_links'
][
'action_object_view'
][
0
][
'name'
],
"view"
)
self
.
assertEqual
(
result_dict
[
'_links'
][
'action_workflow'
][
0
][
'href'
],
"%s/web_site_module/hateoas/ERP5Document_getHateoas?mode=traverse&relative_url=%s&view=custom_action_no_dialog&
form_id=Foo_view
"
%
(
"%s/web_site_module/hateoas/ERP5Document_getHateoas?mode=traverse&relative_url=%s&view=custom_action_no_dialog&
extra_param_json=eyJmb3JtX2lkIjogIkZvb192aWV3In0=
"
%
(
self
.
portal
.
absolute_url
(),
urllib
.
quote_plus
(
document
.
getRelativeUrl
())))
self
.
assertEqual
(
result_dict
[
'_links'
][
'action_workflow'
][
0
][
'title'
],
"Custom Action No Dialog"
)
...
...
@@ -586,7 +586,7 @@ class TestERP5Document_getHateoas_mode_traverse(ERP5HALJSONStyleSkinsMixin):
self
.
assertEqual
(
result_dict
[
'_links'
][
'site_root'
][
'name'
],
self
.
portal
.
web_site_module
.
hateoas
.
getTitle
())
self
.
assertEqual
(
result_dict
[
'_links'
][
'action_object_new_content_action'
][
'href'
],
"%s/web_site_module/hateoas/ERP5Document_getHateoas?mode=traverse&relative_url=%s&view=create_a_document&
form_id=Foo_view
"
%
(
"%s/web_site_module/hateoas/ERP5Document_getHateoas?mode=traverse&relative_url=%s&view=create_a_document&
extra_param_json=eyJmb3JtX2lkIjogIkZvb192aWV3In0=
"
%
(
self
.
portal
.
absolute_url
(),
urllib
.
quote_plus
(
document
.
getRelativeUrl
())))
self
.
assertEqual
(
result_dict
[
'_links'
][
'action_object_new_content_action'
][
'title'
],
"Create a Document"
)
...
...
@@ -1721,7 +1721,7 @@ class TestERP5Document_getHateoas_mode_bulk(ERP5HALJSONStyleSkinsMixin):
self
.
assertEqual
(
result_dict
[
'result_list'
][
0
][
'_links'
][
'action_object_view'
][
0
][
'name'
],
"view"
)
self
.
assertEqual
(
result_dict
[
'result_list'
][
0
][
'_links'
][
'action_workflow'
][
0
][
'href'
],
"%s/web_site_module/hateoas/ERP5Document_getHateoas?mode=traverse&relative_url=%s&view=custom_action_no_dialog&
form_id=Foo_view
"
%
(
"%s/web_site_module/hateoas/ERP5Document_getHateoas?mode=traverse&relative_url=%s&view=custom_action_no_dialog&
extra_param_json=eyJmb3JtX2lkIjogIkZvb192aWV3In0=
"
%
(
self
.
portal
.
absolute_url
(),
urllib
.
quote_plus
(
document
.
getRelativeUrl
())))
self
.
assertEqual
(
result_dict
[
'result_list'
][
0
][
'_links'
][
'action_workflow'
][
0
][
'title'
],
"Custom Action No Dialog"
)
...
...
@@ -1734,7 +1734,7 @@ class TestERP5Document_getHateoas_mode_bulk(ERP5HALJSONStyleSkinsMixin):
self
.
assertEqual
(
result_dict
[
'result_list'
][
0
][
'_links'
][
'site_root'
][
'name'
],
self
.
portal
.
web_site_module
.
hateoas
.
getTitle
())
self
.
assertEqual
(
result_dict
[
'result_list'
][
0
][
'_links'
][
'action_object_new_content_action'
][
'href'
],
"%s/web_site_module/hateoas/ERP5Document_getHateoas?mode=traverse&relative_url=%s&view=create_a_document&
form_id=Foo_view
"
%
(
"%s/web_site_module/hateoas/ERP5Document_getHateoas?mode=traverse&relative_url=%s&view=create_a_document&
extra_param_json=eyJmb3JtX2lkIjogIkZvb192aWV3In0=
"
%
(
self
.
portal
.
absolute_url
(),
urllib
.
quote_plus
(
document
.
getRelativeUrl
())))
self
.
assertEqual
(
result_dict
[
'result_list'
][
0
][
'_links'
][
'action_object_new_content_action'
][
'title'
],
"Create a Document"
)
...
...
@@ -1965,6 +1965,7 @@ class TestERP5Action_getHateoas(ERP5HALJSONStyleSkinsMixin):
REQUEST
=
fake_request
,
dialog_method
=
'Foo_doNothing'
,
# 'Workflow_statusModify' would lead us by a different path in the code
dialog_id
=
'Foo_viewCustomWorkflowRequiredActionDialog'
,
form_id
=
'Foo_view'
)
self
.
assertEqual
(
fake_request
.
RESPONSE
.
status
,
400
)
...
...
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