Better handling of RemarksField and friendly display #1495
Conversation
When include_methods param is used in a read call with a single
parameter (instead of a list of more than one), the jsonapi does
not handle the value parameter properly and transforms it to
a list of characters instead.
A call like follows:
```
this.ajax_submit({
url: this.get_portal_url() + "/@@API/read",
data: {
catalog_name: "uid_catalog",
UID: widget.attr('data-uid'),
include_methods: ["getSomething"],
}
}).done(function(data) {
return deferred.resolveWith(this, [data.objects[0]["getSomething"]]);
});
```
fails. The function `get_include_methods` returns `getSomething`
instead of a list `[getSomething]`. Therefore, the function
`load_method_values` tries the following calls to the instance:
`g`, `e`, `t`, `S`, `o`, ...
If the call is like follows:
```
this.ajax_submit({
url: this.get_portal_url() + "/@@API/read",
data: {
catalog_name: "uid_catalog",
UID: widget.attr('data-uid'),
include_methods: ["getSomething", "getId"],
}
}).done(function(data) {
return deferred.resolveWith(this, [data.objects[0]["getSomething"]]);
});
```
the list is resolved properly.
There was a problem hiding this comment.
Fails for Unicode Remarks with the following Traceback:
[...]
Module bika.lims.browser.fields.remarksfield, line 102, in markdown_content
Module markdown, line 411, in markdown
Module markdown, line 279, in convert
UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 91: ordinal not in range(128). -- Note: Markdown only accepts unicode input!
|
It also seems that the timestamp for new remarks is missing:
Is there any chance to migrate existing remarks to the new format? Also just an idea: How about storing that whole thing in JSON format? |
|
Sorry, the timestamp is now displayed after a full reload, but old comments get the same timestamp as the new ones:
Also I would rather display the full date instead of the friendly date and do the conversion in JavaScript with momentjs |
Done
Can be done, but since the remarks is already handling legacy values without having to walk-through all objects in an endless upgrade step, I thought that was not worth.
I did store the value jsonified at the beginning: 0718a9b#diff-ff77314d577c3d0d3aca8ee30f1ff8a9L107 but I thought it was making the thing more complex without providing any benefits.
Umm, this comes from the markdown library. I tested the RemarksField using |
…ode or ascii input
|
I think is working fine now, except that the styling of remarks fields in places where the form is automatically rendered by AT (e.g. for setup content types such as Without the previous commit and adding the following property in the remarks styling in e.g. Analysis Profile (both in view and edit), would look like this:
Now it looks like follows:
|
|
Also, I feel the sort order from newest to oldest is quite confusing. I know it was like this before, but still... |
|
Hmm, I've seen that in view-mode the class Or maybe we can use a simple text widget for the remarks in setup items? I've seen that this is the case for Analysis Services. |
Agree, I don't think such remarks widget makes sense in setup items. A simple text widget would work, indeed. |
Description of the issue/feature this PR addresses
Remarks field is a bit wonky, especially because of how handles and displays the text. This Pull Request improves the RemarksField as well as it's look&feel (see screenshot below).
Remarks are now stored in a list (a
RemarksHistoryobject) made of dicts (RemarksHistoryRecordobjects), that allows easy access to remark entries and metadata as well (user id, user name, etc). It opens the possibility to add more keys in theRemarksHistoryRecorddict to handle future behaviors (e.g. pin remarks, choose those to be printed in results report, etc.)Current behavior before PR
The way RemarksField works is wonky
Desired behavior after PR is merged
RemarksField becomes more flexible
--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.