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

Create an Analysis Request from Sample View #594

Merged
merged 18 commits into from
Feb 8, 2018

Conversation

Espurna
Copy link

@Espurna Espurna commented Jan 22, 2018

Description of the issue/feature this PR addresses

This PR adds a button in Sample Views to create analysis requests from them.

An “Add Analysis Request” button will be added in Sample's view. Once the button is pressed, the user will be redirected to the Analysis Request Add form, and the data already assigned to the Sample will be filled automatically

Since some AnalysisRequest object fields can be overridden or extended in addons, a marker interface and an adapters query has been added to get_default_value. The process is as follows:

Default data for AR Add form fields are filed through get_default_value method that give them a value using some logic. In some cases those fields can be defined in addons. Now get_default_value queries for hooks adapting IGetDefaultFieldValueARAddHook, those hooks should return a default value for the extended fields.

Current behavior before PR

The only way to create an analysis request is from an analysis request view. If a user is checking a sample and wants to create an analysis request for it, he/she has to move to analysis requests view, open a new AR Add form and introduce the sample ID in the AR Add form field.

Desired behavior after PR is merged

An analysis request for a sample can be created from the same Sample View, easing the process.

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

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 Pau, I like the idea with the adapter to fetch the default value – nicely decoupled.
I just have some change request according to formatting and PEP8-ing, sorry for being nerving with that, but these small things are important for my little coders heart ;)

def get_sample(self):
"""
Returns the Sample
"""
Copy link
Contributor

Choose a reason for hiding this comment

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

I know, it might sound pedantic, but stick to the same docstring style as the other methods, e.g.

def get_sample(self):
    """Returns the Sample
    """

from . import SampleAnalysesView
from . import SamplePartitionsView
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to put PEP8 changes into a separate commit

Copy link
Author

Choose a reason for hiding this comment

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

Done!

<span tal:replace="python:add_item"/>
</a>
</tal:add_actions>
</h1>
</metal:title>
Copy link
Contributor

Choose a reason for hiding this comment

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

Try to format also HTML files nicely, e.g. with an indendation of 2 spaces to safe some space. Also avoid whenever possible inline styles

Copy link
Author

Choose a reason for hiding this comment

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

Done, pycharm was using tabs as default

from bika.lims import bikaMessageFactory as _
from bika.lims import logger
from bika.lims.interfaces import IGetDefaultFieldValueARAddHook
from bika.lims.utils import tmpID
from bika.lims.utils.analysisrequest import create_analysisrequest as crar
Copy link
Contributor

Choose a reason for hiding this comment

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

Here also a separate commit please

Copy link
Author

Choose a reason for hiding this comment

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

Ok

name=hook_name,
interface=IGetDefaultFieldValueARAddHook)
if adapter is not None:
default = adapter(self.context)
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice!

field = $("#DateSampled-#{arnum}")
value = sample.date_sampled
field.val value

Copy link
Contributor

Choose a reason for hiding this comment

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

Try to keep your commits consistent. E.g. this change is independent of the hook you introduced above

Copy link
Author

Choose a reason for hiding this comment

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

Got it

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.

Added entry to CHANGES.rst, removed duplicate code and merged master into

@xispa xispa merged commit c4ed7c1 into senaite:master Feb 8, 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.

3 participants