Dashboard all/mine filters #467
Conversation
…sk/424-dashboard-all-mine
There was a problem hiding this comment.
I'd remove the option "DashboardAllMine" from setup and always render the "All/Mine" filter. Note also the following error is rised when the cookie is set for the first time:
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.dashboard.dashboard, line 50, in __call__
Module json, line 338, in loads
Module json.decoder, line 366, in decode
TypeError: expected string or buffer
| self.request.response.redirect(self.portal_url + "/bika-frontpage") | ||
| else: | ||
| self._init_date_range() | ||
| self.dashboard_cookie = get_strings( | ||
| json.loads(self.check_dashboard_cookie())) |
There was a problem hiding this comment.
This json.loadswill fail if self.check_dashboard_cookie() returns None (if 'all/mine' filter is disabled) or if the value is not in json format (note this happens when the cookie is created for the first time in self.check_dashboard_cookie(), L74).
| @@ -130,6 +198,24 @@ def get_sections(self): | |||
| self.get_worksheets_section()] | |||
| return sections | |||
|
|
|||
| def is_filter_enable(self): | |||
There was a problem hiding this comment.
I think the creation of a function to just only retrieve the setting value from bika_setup is not worth.
There was a problem hiding this comment.
In any case, I would omit this is_filter_enable() and always display the "Mine/All" selection list. See comment https://github.com/senaite/bika.lims/pull/467/files#diff-0dde5125b635d6fb55f5372798dae571R550
| def get_filter_options(self): | ||
| """ | ||
| Returns whether the dashboard filter is enabled. | ||
| :return: Boolean |
There was a problem hiding this comment.
Comment does not fit with the value the function does/returns
| tofrontpage = 'Manager' not in roles and 'LabManager' not in roles | ||
|
|
||
| if tofrontpage == True: | ||
| if tofrontpage: |
There was a problem hiding this comment.
| cookie_criteria = self.dashboard_cookie.get(section_name) | ||
| if cookie_criteria == 'mine': | ||
| query['Creator'] = self.member.getId() | ||
| return query |
There was a problem hiding this comment.
This return is not needed. To be consistent with your previous conditional, you could do the other way round:
if cookie_criteria != 'mine':
return query
query['Creator'] = self.member.getId()
return query
bika/lims/content/bikasetup.py
Outdated
| "statuses will only take into account the worksheets to which" | ||
| " the user is assigned.") | ||
| ), | ||
| ), |
There was a problem hiding this comment.
I'd say there is no need to add an option in bika_setup, rather always render the 'All/Mine' selection list.
| :return: a dictionary or None | ||
| """ | ||
| # Check setup configuration | ||
| if not self.is_filter_enable(): |
There was a problem hiding this comment.
I would omit this is_filter_enable() and always display the "Mine/All" selection list. See comment https://github.com/senaite/bika.lims/pull/467/files#diff-0dde5125b635d6fb55f5372798dae571R550
| <h2 tal:content="section/title" class='dashboard-section-title'></h2> | ||
| <div> | ||
| <h2 tal:content="section/title" class='dashboard-section-title'></h2> | ||
| <tal:dashboard_filters condition="python: view.is_filter_enable()"> |
There was a problem hiding this comment.
I would omit this is_filter_enable() and always display the "Mine/All" selection list. See comment https://github.com/senaite/bika.lims/pull/467/files#diff-0dde5125b635d6fb55f5372798dae571R550
| response = request.RESPONSE | ||
|
|
||
| # Voiding our special cookie on logout | ||
| response.setCookie(DASHBOARD_FILTER_COOKIE, None, path='/', max_age=0) |
There was a problem hiding this comment.
Is this necessary?. I would expect to see the dashboard filtered as I left it some days ago.
There was a problem hiding this comment.
I was just following what we do with other cookies
| json.dumps(cookie_raw), | ||
| quoted=False, | ||
| path='/') | ||
| return cookie_raw |
Description of the issue/feature this PR addresses
The ability to change panel dashboards depending on assignation.
Current behavior before PR
There is no way to update dashboard panels accordingly to user property.
Desired behavior after PR is merged
A selection list with the values «All» and «Mine» will be added above each section of the Dashboard. The panels and information within each panel will change according to the option selected: for example, if the user selects «Mine», the panel with the summary of worksheets and worksheets statuses will only take into account the worksheets to which the user is assigned.
This feature can be activated/deactivated in bika setup.
--
I confirm I have tested this PR thoroughly and coded it according to PEP8
and Plone's Python styleguide standards.