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 sample reports retrieval #2282

Merged
merged 147 commits into from
Mar 30, 2023
Merged

Fix sample reports retrieval #2282

merged 147 commits into from
Mar 30, 2023

Conversation

ramonski
Copy link
Contributor

@ramonski ramonski commented Mar 27, 2023

Description of the issue/feature this PR addresses

Please merge #2279 first

This PR introduces the following new getters for samples:

  • getRawReports: Lookup sample reports from the report catalog
  • getReports: Returns sample report objects
  • getReportUIDs: Returns a list of sample report UIDs

Current behavior before PR

Sample reports were assumed to live inside the sample and were retrieved via objectValues

Desired behavior after PR is merged

Sample reports may live in other samples and contain only the current sample in a multi report. Therefore, a catalog search is required.

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

@ramonski ramonski added Bug 🐞 Improvement 🔧 Cleanup 🧹 Code cleanup and refactoring labels Mar 27, 2023
@ramonski ramonski requested a review from xispa March 27, 2023 15:33
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.

I suggest renaming getRawReports to be compliant with AT behavior that we adopted for all types (DX included)

@@ -1645,26 +1646,61 @@ def getLate(self):
return True
return False

def getRawReports(self, inlcude_provisional=True, **kw):
Copy link
Member

Choose a reason for hiding this comment

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

We usually use the getRaw* function to return UIDs. I suggest to add the full_objects=False in getReports (along with **kwargs) instead.

Also note that getRawReports could rely on get_backreferences:

def getRawReports(self):
    return get_backreferences(self, "ARReportAnalysisRequest")

def getReportUIDs(self):
    return self.getRawReports()

Also note there is a typo with inlcude_provisional (should be include_provisional)

def getReports(self, include_provisional=True, full_objects=False, **kw):
    ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I thought about this. Shall I rely better on the backreferences instead?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I thought about this. Shall I rely better on the backreferences instead?

I think that relying on backreferences is better, yes

src/bika/lims/content/analysisrequest.py Outdated Show resolved Hide resolved
@xispa xispa merged commit 47d21d3 into 2.x Mar 30, 2023
@xispa xispa deleted the sample-reports branch March 30, 2023 15:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants