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

Fixed Require MRN patient setting - see issue #70 #71

Merged
merged 3 commits into from
May 16, 2023

Conversation

mikejmets
Copy link
Contributor

Description of the issue/feature this PR addresses

Linked issue: #70

Current behavior before PR

Desired behavior after PR is merged

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

@mikejmets mikejmets marked this pull request as draft May 12, 2023 16:14
@mikejmets
Copy link
Contributor Author

I set this to Draft because the tests I created fail. Namely, when the Require MRN setting is True and I create a Patient with no MRN value, the patient is created. My test expects it to fail. Am I wrong?

@mikejmets mikejmets changed the title Fixed Requrie MRN patient setting - see issue #70 Fixed Require MRN patient setting - see issue #70 May 14, 2023
src/senaite/patient/adapters/listing.py Outdated Show resolved Hide resolved
src/senaite/patient/adapters/listing.py Outdated Show resolved Hide resolved
src/senaite/patient/api.py Outdated Show resolved Hide resolved
src/senaite/patient/browser/patientfolder.py Outdated Show resolved Hide resolved
src/senaite/patient/content/patient.py Show resolved Hide resolved
src/senaite/patient/tests/doctests/Patient.rst Outdated Show resolved Hide resolved
src/senaite/patient/tests/doctests/Patient.rst Outdated Show resolved Hide resolved
@mikejmets mikejmets requested a review from ramonski May 15, 2023 13:39
@mikejmets mikejmets marked this pull request as ready for review May 15, 2023 18:47
@ramonski
Copy link
Contributor

Thanks for the fix Mike. Would you mind to add a Changelog entry, then we can merge it.

@mikejmets
Copy link
Contributor Author

Changelog updated

@ramonski ramonski merged commit 6e41d9c into senaite:master May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants