Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix jsonapi.read omits include_methods when a single parameter is used #1493

Merged
merged 4 commits into from
Dec 23, 2019
Merged

Fix jsonapi.read omits include_methods when a single parameter is used #1493

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
bc4e62c
Fix get_include_methods function from jsonapi
xispa Dec 20, 2019
6508a99
Now we are here, make it a bit better
xispa Dec 20, 2019
5a40e59
Changelog
xispa Dec 20, 2019
4b93837
Merge branch 'master' into jsonapi-read-methods
ramonski Dec 23, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGES.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Changelog

**Fixed**

- #1493 jsonapi.read omits `include_methods` when a single parameter is used
- #1494 Fix KeyError in Sample Type Listing
- #1477 Sample edit form - some selection widgets empty
- #1478 Clients default CC E-Mails missing in Add Sample
Expand Down
14 changes: 10 additions & 4 deletions bika/lims/jsonapi/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@

import json
import Missing
import six
import sys, traceback


Expand Down Expand Up @@ -129,10 +130,15 @@ def load_field_values(instance, include_fields):
def get_include_methods(request):
"""Retrieve include_methods values from the request
"""
methods = request.get("include_methods", "")
include_methods = [
x.strip() for x in methods.split(",") if x.strip()]
return include_methods
include_methods = request.get("include_methods[]")
if not include_methods:
include_methods = request.get("include_methods", [])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When is the variable named include_methods[] in the request?

Copy link
Member Author

@xispa xispa Dec 22, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

First of all, include_methods is not used as a parameter anywhere in the code, but I was playing around and found this issue. Nevertheless, include_fields from here:

senaite.core/bika/lims/browser/js/coffee/bika_widgets/remarkswidget.coffee

Lines 121 to 129 in 699f02f

@ajax_submit
url: @get_portal_url() + "/@@API/read"
data:
catalog_name: "uid_catalog"
UID: widget.attr('data-uid')
include_fields: [fieldname]
.done (data) ->
return deferred.resolveWith this, [data.objects[0][fieldname]]
return deferred.promise()

is received as include_fields[]. Therefore, this trick here:

senaite.core/bika/lims/jsonapi/__init__.py

Lines 56 to 58 in 699f02f

if "include_fields[]" in request:
include_fields = request['include_fields[]']
return include_fields

Same happens if you use include_methods instead. Thus, I just followed same criteria and did not go any further. As a side note, the issue this PR addresses happens with include_fields too, but is a hidden "bug" because of:

senaite.core/bika/lims/jsonapi/__init__.py

Lines 88 to 89 in 699f02f

if include_fields and fieldname not in include_fields:
continue

and this code will work even if include_fields is a string instead of a list.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I think I got it. This means that either JQuery or the Zope Publisher is converting include_methods -> include_methods[]?

Anyhow, going to merge then.


if isinstance(include_methods, six.string_types):
include_methods = include_methods.split(",")
include_methods = map(lambda me: me.strip(), include_methods)

return filter(None, include_methods)


def load_method_values(instance, include_methods):
Expand Down