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

Change unassigned filter #627

Merged
merged 11 commits into from
Feb 15, 2018
Merged

Conversation

Lunga001
Copy link
Contributor

@Lunga001 Lunga001 commented Feb 2, 2018

Description of the issue/feature this PR addresses

On the AR Listing change the Unassigned filter because it currently displays a cluttered list

Current behavior before PR

Filters ARs that are not assigned to a Worksheet by SampleReceived, ToBeVerified, AttachmentDue, Verified and Published states

Desired behavior after PR is merged

Should filter ARs that are not assigned to a Worksheet by SampleRegistered, SamplePrep, ToBeSampled, SampleDue, SampleReceived, and AttachmentDue states

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

@xispa
Copy link
Member

xispa commented Feb 5, 2018

With this Pull Request, when the Unassigned filter from Analysis Requests lists is used, only Analysis Requests that meet the following criteria are displayed:

  • The Analysis Request hasn't been yet transitioned to to_be_verified state (this is, the Analysis Request has at least one result that hasn't been yet submitted).
  • At least one of the Analysis associated to the Analysis Request has not been assigned to a Worksheet.

Note that the assigned/unassigned status is only meaningful during the planning of the tasks to be done by analysts, when there are no results set yet. In fact, assigned status means that all the analyses from a given AR has been assigned to a Worksheet and, therefore to an analyst. Thus, ARs that are in a state beyond received (to_be_verified, verified, published, ...) are not displayed.

'to_be_sampled',
'sample_due',
'sample_received',
'attachment_due',),
Copy link
Member

@xispa xispa Feb 5, 2018

Choose a reason for hiding this comment

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

sample_registered is a temporary state the Analysis Request reaches when is created, but the object is immediately transitioned to either sampled (if sampling workflow disabled) or to_be_sampled (if sampling workflow is enabled). In turn, if no preservation is required, an Analysis Request with sampled state is automatically transitioned to sample_due. In summary, querying for sample_registered should never return anything.

From my point of view, these are the states to be filtered by when unassigned filter is used: sample_received, sample_due, sample_prep, to_be_preserved, to_be_sampled, attachment_due.

Also filter by cancellation_state == 'active' (we don't want inactive ARs to be listed)

Copy link
Member

Choose a reason for hiding this comment

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

My wrong. The assignment of analyses to Worksheets can only be done for those analyses that belong to Analysis Requests that are associated to a Sample Partitions that have been received. Therefore, there is no sense to add in this filter those states that are from the preservation or sampling workflow. Following this reasoning, I suggest this filter to consider the following states:

{'worksheetanalysis_review_state': 'unassigned',
 'cancellation_state': 'active',
 'review_state': ('sample_received', 'attachment_due')

Related, but for analyses instead of Analysis Requests: Analyses filter not kept after selecting one in dashboard #613 . Note the comments:

@@ -743,35 +751,35 @@ def __init__(self, context, request):
'title': _('Print stickers'),
'url': 'workflow_action?action=print_stickers'}],
'columns': ['Priority',
Copy link
Member

Choose a reason for hiding this comment

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

Transitions publish, republish, verify and reinstate will never be available for ARs displayed when using this unassigned filter: only active (so reinstate will never be used) and those ARs that at least have one Analysis with result pending will be displayed (so verify, publish and republish will never be used).

@Lunga001
Copy link
Contributor Author

Lunga001 commented Feb 6, 2018

Thank you for review @xispa, I've made the requested changes using these filters
{'worksheetanalysis_review_state': 'unassigned',
'cancellation_state': 'active',
'review_state': ('sample_received', 'attachment_due') }

@xispa
Copy link
Member

xispa commented Feb 6, 2018

I realized now that this filter will never work because of these two reasons:

I've fixed the first, would be nice @Lunga001 if you fix the second before I accept this PR. Remember to add the necessary logic into the upgradestep.

EDIT: both points mentioned above addressed in #637

@xispa xispa merged commit fe45266 into senaite:master Feb 15, 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