Reminder: Create & find trunk-related issues here. Legacy Trac tickets are still available. Problems? Workflow feedback? Need a module project? Open a support ticket.

OpenMRS Trunk

Encounter table doesn't have dateChanged/changedBy

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Must Must
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: OpenMRS 1.6.0
  • Component/s: Data Model
  • Severity:
    3
  • Keywords:
  • Description:

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

  1. patch-1582
    (3 kB)
    Burke Mamlin
    2009-07-16 07:56:44 EDT
  2. trunk-1582.2.patch
    (2 kB)
    Burke Mamlin
    2009-12-15 05:16:56 EST
  3. trunk-1582.patch
    (3 kB)
    Burke Mamlin
    2009-12-14 11:14:37 EST

Activity

Hide
Ben Wolfe added a comment - 2009-06-23 14:38:34 EDT

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

Show
Ben Wolfe added a comment - 2009-06-23 14:38:34 EDT Yeah, this isn't going to make it into 1.5.0, regardless of importance.
Hide
Madanmohan Rao added a comment - 2009-07-16 07:56:44 EDT

added datechanged and changed by to the encounter table

Show
Madanmohan Rao added a comment - 2009-07-16 07:56:44 EDT added datechanged and changed by to the encounter table
Hide
Madanmohan Rao added a comment - 2009-07-21 05:25:08 EDT

please let me know if any changes for the submitted patch

Show
Madanmohan Rao added a comment - 2009-07-21 05:25:08 EDT please let me know if any changes for the submitted patch
Hide
Ben Wolfe added a comment - 2009-07-21 12:37:35 EDT

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!

Show
Ben Wolfe added a comment - 2009-07-21 12:37:35 EDT 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!
Hide
Darius Jazayeri added a comment - 2009-10-26 23:19:43 EDT

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)

Show
Darius Jazayeri added a comment - 2009-10-26 23:19:43 EDT 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)
Hide
Namrata Nehete added a comment - 2009-12-14 11:14:37 EST

Patch for this ticket

Show
Namrata Nehete added a comment - 2009-12-14 11:14:37 EST Patch for this ticket
Hide
Ben Wolfe added a comment - 2009-12-14 14:32:43 EST

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

Show
Ben Wolfe added a comment - 2009-12-14 14:32:43 EST Thanks Namrata! Because this is slated for 1.6, we'll get this reviewed very soon!
Hide
Darius Jazayeri added a comment - 2009-12-14 19:01:33 EST

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

Show
Darius Jazayeri added a comment - 2009-12-14 19:01:33 EST 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
Hide
Namrata Nehete added a comment - 2009-12-15 05:18:12 EST

Hi,

Thanks for comment,

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

Show
Namrata Nehete added a comment - 2009-12-15 05:18:12 EST Hi, Thanks for comment, Can you please review again. I made changed according to your comment.
Hide
Ben Wolfe added a comment - 2009-12-29 18:42:56 EST

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

Show
Ben Wolfe added a comment - 2009-12-29 18:42:56 EST Looks good. I suggest we split the addColumn into two changesets when we apply it to trunk.
Hide
Justin Miranda added a comment - 2010-01-07 06:28:09 EST

Committed to trunk in changeset rev:11630.

Show
Justin Miranda added a comment - 2010-01-07 06:28:09 EST Committed to trunk in changeset rev:11630.

People

Dates

  • Created:
    2009-06-21 04:01:54 EDT
    Updated:
    2010-07-08 16:42:32 EDT
    Resolved:
    2010-07-01 22:44:14 EDT