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

1.10.x #480

Closed
wants to merge 2 commits into from
Closed

1.10.x #480

wants to merge 2 commits into from

Conversation

vinayvenu
Copy link
Member

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

Vinay Venu added 2 commits December 18, 2013 16:40
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.
@vinayvenu vinayvenu mentioned this pull request Dec 19, 2013
@vinayvenu vinayvenu deleted the 1.10.x branch December 19, 2013 15:24
private String careSetting;

public CareSetting() {
this.careSetting = OUT_PATIENT;
Copy link
Member

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

Copy link
Member

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
.

Copy link
Member

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

@vinayvenu
Copy link
Member Author

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 I sent a fresh pull request from a different branch. (#484). This has been merged already, but I will make sure the comments made here are incorporated.

@wluyima
Copy link
Member

wluyima commented Dec 20, 2013

So you are saying you couldn't push t your fork(origin), if yes, what error
message were you getting?

Wyclif

On Thu, Dec 19, 2013 at 8:44 PM, Vinay Venu notifications@github.comwrote:

Hi,

Sorry about the confusion, but this pull request is closed. @wluyimahttps://github.com/wluyimayou 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 I sent a fresh pull request from a different branch. (#484#484).
This has been merged already, but I will make sure the comments made here
are incorporated.


Reply to this email directly or view it on GitHubhttps://github.com//pull/480#issuecomment-30983283
.

Wyclif Luyima
Regenstrief Institute Inc.

Confidentiality Notice: The contents of this message and any files
transmitted with it may contain confidential and/or privileged information
and are intended solely for the use of the named addressee(s).
Additionally, the information contained herein may have been disclosed to
you from medical records with confidentiality protected by federal and
state laws. Federal regulations and State laws prohibit you from making
further disclosure of such information without the specific written consent
of the person to whom the information pertains or as otherwise permitted by
such regulations. A general authorization for the release of medical or
other information is not sufficient for this purpose.

If you have received this message in error, please notify the sender by
return e-mail and delete the original message. Any retention, disclosure,
copying, distribution or use of this information by anyone other than the
intended recipient is strictly prohibited.

RandilaP pushed a commit to RandilaP/openmrs-core that referenced this pull request Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants