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

Fix analysis_workflow permissions for Field Analysis Results #293

Conversation

rockfruit
Copy link
Contributor

Reported at https://jira.bikalabs.com/browse/AN-147, these incorrect permissions
allow the analysis to remain editable after submission or retraction.

(cherry picked from commit 15779a8ffe44680d5e00b9a391ce4ac8c4452d53)

I believe that acquiring permissions in workflows is evil, but I have only made modifications related to fixing the roles of the "Edit Field Results" permission of analysis workflow.

I'm requesting this against senaite-integration because the other branches don't yet use the xml format for workflow configuration.

Reported at https://jira.bikalabs.com/browse/AN-147, these incorrect permissions
allow the analysis to remain editable after submission or retraction.

(cherry picked from commit 15779a8ffe44680d5e00b9a391ce4ac8c4452d53)

# Conflicts:
#	bika/lims/profiles/default/workflows/bika_analysis_workflow/definition.xml
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.

Some considerations about permissions and roles regarding analyses. Maybe we can accept this PR for now, but I strongly suggest to establish a strict schema of roles-permissions in workflows. Otherwise, we will lost control of all this easily: state-permissions-roles schema is one of the things that will usually depend on the lab idiosyncrasy.

I suggest that once we have all permissions in place, to apply all modifications on this regard in a lab-specific add-on.

@@ -187,7 +192,7 @@
<permission-role>Manager</permission-role>
</permission-map>
<permission-map name="BIKA: Edit Field Results" acquired="False">
<permission-role>LabClerk</permission-role>
<permission-role>Analyst</permission-role>
Copy link
Member

Choose a reason for hiding this comment

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

In my opinion, an Analyst shouldn't never be able to modify Field results, I think this is a task that must concern to the Sampler only. If we were strict, the scope of work of an Analyst regarding to results introduction should be limited to those analyses performed inside the lab and ideally, only for those to which has been assigned through a Worksheet. Is the responsability of the labman to plan the work and assign the tasks to the available resources.

Note to_be_preserved state is reached before the sample is received (not yet available by personnel from inside the lab). See diagram here: https://github.com/senaite/bika.lims/blob/senaite-integration/docs/resources/bika_sample_workflow.svg

Anyhow, these are the things that will definitely depend on the lab idiosyncrasy, so I suggest that once we have all permissions in place, the changes in permissions should be done in a lab-specific add-on.

<permission-role>Analyst</permission-role>
<permission-role>LabManager</permission-role>
<permission-role>Manager</permission-role>
<permission-role>Sampler</permission-role>
Copy link
Member

Choose a reason for hiding this comment

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

Assuming that sample is received after the sample collection has taken place and the field results must be entered during the collecting process, no one should be able to edit those "field results" (except Sampler and Labman) once a Sample has been received.

@@ -116,12 +117,12 @@
<permission-role>Owner</permission-role>
</permission-map>
<permission-map name="BIKA: Edit Field Results" acquired="False">
<permission-role>Analyst</permission-role>
Copy link
Member

Choose a reason for hiding this comment

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

IMO, no one except Sampler should be allowed to edit field results

@@ -42,6 +42,7 @@
<permission-role>LabManager</permission-role>
<permission-role>Manager</permission-role>
<permission-role>RegulatoryInspector</permission-role>
<permission-role>Sampler</permission-role>
Copy link
Member

Choose a reason for hiding this comment

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

Sampler mustn't be allowed to see the results set in the lab. His/her work is only about the results in the field. If there are personnel in the lab that works both as samplers and analysts, then both roles should be applied to those users

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.

@xispa I agree with all the points you mentioned and we probably need to rework all the workflows once we have it separated in a senaite.lims.workflow package. For now I'm ok if the people within the lab have slightly more privileges than expected. Otherwise everyone will get lab manager permissions by the admin...

@ramonski
Copy link
Contributor

@xispa: Please decide if we merge it in the current state and merge then directly.

@xispa xispa merged commit fda6383 into senaite:senaite-integration Oct 25, 2017
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.

3 participants