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
1.10.x #480
1.10.x #480
Conversation
Primary change is to add a new class called CareSetting as an attribute to Order. All test datasets have been modified to include this new attribute. OrderSaveHandler will populate the default value (OUTPATIENT) if not provided.
private String careSetting; | ||
|
||
public CareSetting() { | ||
this.careSetting = OUT_PATIENT; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is quite right, i would rather leave careSetting property null
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be an Enum rather than a regular class?
-Darius (by phone)
On Dec 19, 2013 3:34 PM, "wluyima" notifications@github.com wrote:
In api/src/main/java/org/openmrs/CareSetting.java:
- * Copyright (C) OpenMRS, LLC. All Rights Reserved.
- */
+package org.openmrs;
+
+public class CareSetting implements java.io.Serializable {
+- private static final String OUT_PATIENT = "OUTPATIENT";
- private static final String IN_PATIENT = "INPATIENT";
- public static CareSetting OUTPATIENT = new CareSetting(OUT_PATIENT);
- public static CareSetting INPATIENT = new CareSetting(IN_PATIENT);
- private String careSetting;
- public CareSetting() {
this.careSetting = OUT_PATIENT;
I don't think this is quite right, i would rather leave careSetting
property null—
Reply to this email directly or view it on GitHubhttps://github.com//pull/480/files#r8488765
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, Burke suggested that we should still let an implementation to be able to define its own kind of care setting and this is the only way to do it without introducing a new table
Hi, Sorry about the confusion, but this pull request is closed. @wluyima you had asked to not raise pull requests where there are merge commits, and I found it hard to pull --rebase from an upstream repo and keep pushing to my remote when I have commits. |
So you are saying you couldn't push t your fork(origin), if yes, what error Wyclif On Thu, Dec 19, 2013 at 8:44 PM, Vinay Venu notifications@github.comwrote:
Wyclif Luyima Confidentiality Notice: The contents of this message and any files If you have received this message in error, please notify the sender by |
Completes TRUNK-4156.
Adds new CareSetting class.
Adds this as an attribute to Order
Adds mandatory varchar(50) field care_setting in orders table with default value of 'OUTPATIENT'
Defaults careSetting of Order to CareSetting.OUTPATIENT in OrderSaveHandler
Fixes test data xml files