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

Sample panel in dashboard #480

Merged
merged 10 commits into from
Feb 1, 2018

Conversation

Espurna
Copy link

@Espurna Espurna commented Dec 20, 2017

Description of the issue/feature this PR addresses

There is no panel for Samples

Current behavior before PR

There is no panel for Samples

Desired behavior after PR is merged

The dashboard will display a panel for Samples

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

Copy link
Member

@xispa xispa left a comment

Choose a reason for hiding this comment

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

Minor changes and a question

@@ -661,7 +661,7 @@ def _process_request(self):
continue
request_key = "%s_%s" % (form_id, index)
value = self.request.get(request_key, '')
if len(value) > 1:
if len(value) > 0:
Copy link
Member

Choose a reason for hiding this comment

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

Not sure about this change, but this condition is also used in https://github.com/senaite/bika.lims/pull/480/files#diff-05737f388606e64fa9543e7eea467c36R680 . So, if this changes here, I guess this change must be applied in Line 680 too. Maybe there are other places where this condition is used?

If I understand correctly, with len(value) > 1 we prevent the system to do searches if the search term does not have at least 2 characters? Can you confirm @Espurna

Copy link
Author

Choose a reason for hiding this comment

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

Indeed, think about a case where we have '1' and '0' as True/False indicators. In my opinion this conditional should check whether there is data.

Copy link
Member

Choose a reason for hiding this comment

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

Good point

purl = 'analysisrequests?analysisrequests_getPrinted=0'
query['getPrinted'] = '0'
if 'review_state' in query:
del query['review_state']
Copy link
Member

Choose a reason for hiding this comment

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

I'd add the review_state in which the printed option makes sense (published) instead of removing it:

query['getPrinted'] = 0
query['review_state'] = ['published',]

filtering_allowed = \
self.context.bika_setup.getAllowDepartmentFiltering()
if filtering_allowed:
cookie_dep_uid = self.request.get('filter_by_department_info',
Copy link
Member

Choose a reason for hiding this comment

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

for cookie name, better use a constant outside the class like:

FILTER_BY_DEPT_COOKIE_ID = 'filter_by_department_info' 

Also, encapsulate this block of code like:

def _get_filter_by_department_uids(self):
    if not self.context.bika_setup.getAllowedDepartmentFiltering():
        return []
    cookie_dep_uid = self.request.get(FILTER_BY_DEPT_COOKIE_ID, None)
    if not cookie_dep_uid:
        return []
    return cookie_dep_uid.split(",")

def _fill_department_uids(self, query=None):
    if not query or not isinstance(query, dict):
        return
    uids = self._get_filter_by_department_uids()
    if len(uids) == 0:
        return
    query['getDepartmentUIDs'] = {"query": uids, "operator": "or"}

Then, you can simply do the following:

self._fill_department_uids(query)

@xispa xispa removed the to master label Dec 21, 2017
@xispa xispa merged commit a5f5ece into senaite:master Feb 1, 2018
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