Downloads Documentation Community
Contribute
Contribute Demo






Show Sidebar
Login | Register


Ticket #1582 (closed defect: fixed)

Opened 9 months ago

Last modified 2 months ago

Encounter table doesn't have dateChanged/changedBy

Reported by: djazayeri Assigned to: n.nehete
Priority: critical Milestone: OpenMRS 1.6
Component: Data Model Keywords:
Cc: bwolfe, bmamlin, pbiondich Introductory Ticket: 0
Code Review Status: Approved

Description

I'm filing this as a defect and tagging it as 1.5 because I feel like it's quite important.

Attachments

patch-1582 (2.9 kB) - added by madanmohan on 07/16/09 07:56:44.
added datechanged and changed by to the encounter table
trunk-1582.patch (2.9 kB) - added by n.nehete on 12/14/09 11:14:37.
Patch for this ticket
trunk-1582.2.patch (2.1 kB) - added by n.nehete on 12/15/09 05:16:56.

Change History

06/21/09 04:02:30 changed by djazayeri

  • owner deleted.

06/23/09 14:38:34 changed by bwolfe

Yeah, this isn't going to make it into 1.5.0, regardless of importance. :-)

07/16/09 07:56:44 changed by madanmohan

  • attachment patch-1582 added.

added datechanged and changed by to the encounter table

07/21/09 05:25:08 changed by madanmohan

please let me know if any changes for the submitted patch

07/21/09 12:37:35 changed by bwolfe

  • review_status set to Reviewed.
  • milestone changed from OpenMRS 1.5 to OpenMRS 1.6.

Can you use the standard liquibase element alternations instead of the <sql> tag? We want the liquibase file to be database independent and not depenedent on mysql. See http://www.liquibase.org/manual/home#available_database_refactorings or other examples of addColumn in our liquibase-update-to-latest.xml file. Thanks!

10/26/09 23:19:43 changed by djazayeri

Hi madanmohan,

Do you want to make these changes? I'd like to get this into OpenMRS 1.6.

In addition to Ben's comment, a few more of mine:

  • in Encounter.hbm.xml dateChanged needs to be allowed to be null
  • in Encounter.java, get rid of @Element(required = false), and the import of that annotation. (We are going to stop using simpleframework for xml serialization, so there's no need to add this to a new class.)

Thanks!

-Darius (the 1.6 release manager)

12/14/09 11:14:37 changed by n.nehete

  • attachment trunk-1582.patch added.

Patch for this ticket

12/14/09 14:32:43 changed by bwolfe

  • owner set to n.nehete.

Thanks Namrata! Because this is slated for 1.6, we'll get this reviewed very soon!

12/14/09 19:01:33 changed by djazayeri

Hi Namrata,

Thanks for the patch. Ben, Justin, and I reviewed it today, and we have a few comments:

  • In the Encounter.hbm.xml file, the new columns say not-null="true", but it's supposed to be not-null="false"
  • In liquibase-update-to-latest.xml, you also need to add a foreign key constraint on the changed_by column.
  • You don't need to add anything to Encounter.java, because the changedBy and dateChanged properties are already inherited from BaseOpenmrsData.

-Darius

12/15/09 05:16:56 changed by n.nehete

  • attachment trunk-1582.2.patch added.

12/15/09 05:18:12 changed by n.nehete

Hi,

Thanks for comment,

Can you please review again. I made changed according to your comment.

12/15/09 06:56:57 changed by jmiranda

  • review_status changed from Reviewed to Needs Review.

12/29/09 18:42:56 changed by bwolfe

Looks good. I suggest we split the addColumn into two changesets when we apply it to trunk.

01/07/10 06:28:09 changed by jmiranda

  • status changed from new to closed.
  • review_status changed from Needs Review to Approved.
  • resolution set to fixed.

Committed to trunk in changeset [11630].