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.

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

ReferenceWidget does not handle searches with null/None #1014

Merged
merged 1 commit into from
Aug 31, 2018

Conversation

xispa
Copy link
Member

@xispa xispa commented Aug 30, 2018

Description of the issue/feature this PR addresses

ast.literal_eval does not convert javascript's null to python's None and the following Traceback is thrown:

Traceback (innermost last):
  Module ZPublisher.Publish, line 138, in publish
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 48, in call_object
  Module bika.lims.browser.widgets.referencewidget, line 204, in __call__
  Module bika.lims.browser.client.ajax, line 31, in __call__
  Module bika.lims.adapters.referencewidgetvocabulary, line 44, in __call__
  Module ast, line 80, in literal_eval
  Module ast, line 63, in _convert
  Module ast, line 62, in <genexpr>
  Module ast, line 60, in _convert
  Module ast, line 79, in _convert
ValueError: malformed string

ast.literal_eval has been replaced by built-in json.loads, and to overcome the issue about unicodes, a function that converts unicodes to strings recursively has been added, as suggested in the same issue above.

Sometimes, searching catalog for a content type with a field that has either a value or None is highly desirable. E.g (in health):

client_uid = "01f004a407da40bebd9f63c2669abf9f"
query = dict(portal_type="Doctor", getPrimaryReferrerUID=[client_uid, None])
doctors = api.search(query, "portal_catalog")
results = map(lambda doctor: api.get_object(res).getPrimaryReferrerUID(), doctors)
list(set(results))
# This will return ["01f004a407da40bebd9f63c2669abf9f", None]

This search will include all Doctors that belong to the client client_uid and those that are not assigned to any client.

Current behavior before PR

ReferenceWidget does not supports searches with "null" values.

Desired behavior after PR is merged

ReferenceWidget does not supports searches with "null" values.

--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.

ast does not convert "null" to python's None and the following
Traceback is thrown:

Traceback (innermost last):
  Module ZPublisher.Publish, line 138, in publish
  Module ZPublisher.mapply, line 77, in mapply
  Module ZPublisher.Publish, line 48, in call_object
  Module bika.lims.browser.widgets.referencewidget, line 204, in __call__
  Module bika.lims.browser.client.ajax, line 31, in __call__
  Module bika.lims.adapters.referencewidgetvocabulary, line 44, in __call__
  Module ast, line 80, in literal_eval
  Module ast, line 63, in _convert
  Module ast, line 62, in <genexpr>
  Module ast, line 60, in _convert
  Module ast, line 79, in _convert
ValueError: malformed string

ast.literal_eval has been replaced by built-in json.loads, and to
overcome the issue about unicodes:
#443
a function that converts unicodes to strings recursively has been
added, as suggested in the same issue above
@xispa xispa requested a review from ramonski August 30, 2018 21:33
xispa added a commit to senaite/senaite.health that referenced this pull request Aug 30, 2018
Dependencies: senaite/senaite.core#1014

In AR Add form, do not display doctors from other client than the
one selected, but include those that do not have any client assigned
xispa added a commit to senaite/senaite.health that referenced this pull request Aug 30, 2018
Dependencies: senaite/senaite.core#1014

In AR Add form, do not display doctors from other client than the
one selected, but include those that do not have any client assigned
@ramonski
Copy link
Contributor

ramonski commented Aug 31, 2018

Ok, just tested it here once again:

In [26]: data = {"name": 'S\xc3\x9cDRAD', "uids": ["1", "2", "3"], "foo": None}

In [27]: j = json.dumps(data)

In [28]: j
Out[28]: '{"foo": null, "name": "S\\u00dcDRAD", "uids": ["1", "2", "3"]}'

In [29]: ast.literal_eval(j)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-29-0491f871e43d> in <module>()
----> 1 ast.literal_eval(j)

/Users/rbartl/miniconda2/lib/python2.7/ast.pyc in literal_eval(node_or_string)
     78                 return left - right
     79         raise ValueError('malformed string')
---> 80     return _convert(node_or_string)
     81
     82

/Users/rbartl/miniconda2/lib/python2.7/ast.pyc in _convert(node)
     61         elif isinstance(node, Dict):
     62             return dict((_convert(k), _convert(v)) for k, v
---> 63                         in zip(node.keys, node.values))
     64         elif isinstance(node, Name):
     65             if node.id in _safe_names:

/Users/rbartl/miniconda2/lib/python2.7/ast.pyc in <genexpr>((k, v))
     60             return list(map(_convert, node.elts))
     61         elif isinstance(node, Dict):
---> 62             return dict((_convert(k), _convert(v)) for k, v
     63                         in zip(node.keys, node.values))
     64         elif isinstance(node, Name):

/Users/rbartl/miniconda2/lib/python2.7/ast.pyc in _convert(node)
     77             else:
     78                 return left - right
---> 79         raise ValueError('malformed string')
     80     return _convert(node_or_string)
     81

ValueError: malformed string

In [30]: json.loads(j)
Out[30]: {u'foo': None, u'name': u'S\xdcDRAD', u'uids': [u'1', u'2', u'3']}

So json.loads always returns unicode for strings and with the recursive encoding it works also for non-string values.

Copy link
Contributor

@ramonski ramonski left a comment

Choose a reason for hiding this comment

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

Thanks!

@ramonski ramonski merged commit 5c20622 into master Aug 31, 2018
@ramonski ramonski deleted the refwidget-null-searches branch August 31, 2018 06:36
xispa added a commit to senaite/senaite.health that referenced this pull request Aug 31, 2018
)

* Filter doctors by current's user client

If the current user is a Client contact, only the doctors assigned
to the same client and those that do not have any client assigned are
displayed. If user is from the lab, no filtering is applied.

If the current context of the listing is Client, only doctors assigned
to same client are displayed.

* Cleanup folderitems function from Doctors view

* Cleanup Doctors listing view

Dependencies: senaite/senaite.core#1012

* Hide doctors from other clients when logged as client

* Don't display doctors from other clients in AR Add

Dependencies: senaite/senaite.core#1014

In AR Add form, do not display doctors from other client than the
one selected, but include those that do not have any client assigned

* Don't display doctors from other clients in AR Add

Dependencies: senaite/senaite.core#1014

In AR Add form, do not display doctors from other client than the
one selected, but include those that do not have any client assigned

* Do not display doctors from other clients in Case Add view

* Add Primary Referrer column in Doctor listings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants