TRUNK-4730 - complete list of domain objects missing changed_by or date_changed

Issue: https://issues.openmrs.org/browse/TRUNK-4730

The following classes do not have changed_by or date_changed fields. Would love if people could confirm whether any of these classes do not need those fields otherwise I will move on to the next step of the issue and add the fields to the database for the relevant classes.


  • ConceptClass
  • ConceptName
  • ConceptNameTag
  • ConceptReferenceSource
  • EncounterType
  • GlobalProperty
  • Patient_identifierType
  • RelationshipType
  • UserProperty
1 Like

@cathychen Thanks! This is great :smiley: @dkayiwa @wyclif could you run through these classes and confirm which need changed_by or date_changed fields in the database as @darius is unavailable.

Some of them are weak entities (all the concept collections for instance). I don’t think they need audit attributes per se, because its main entity already has.

I have turned the main post into a Wiki one, e.g. I think anyone can edit it in place.

@lluismf, do you have moment to make these comments inline?

1 Like

Sure but where’s the link to the wiki? Do you mean JIRA https://issues.openmrs.org/browse/TRUNK-4730 ?

I mean that I turned the initial post in this OpenMRS Talk thread into a wiki post, so I believe that anyone can edit it.

I.e. here:

Hey @lluismf, do you think we are good to go with the above list?

Hi Mayank

I don’t have enough business knowledge to say it, I just expressed an opinion about weak entities. Technically I’m not even sure that it can be possible to know if a person name has changed, given that the UI sends the whole Person to the business layer. To be really sure if a member of a collection has changed, its attributes shoudl be compared one by one. It seems too complex and the benefit probably not worth it.

bumping this post. This issue is a part of Platform 2,0 beta sprint. It would be awesome if any dev can volunteer to look into this :slight_smile:

@raff @wyclif @dkayiwa would anyone of you guys have some time this week to go through this? :slight_smile:

I think this can be scripted to get the tables missing the columns

I went to OpenMRS Data Models, loaded the 1.11 iframe directly, and ran this in the console:

jQuery('.table').each( function(k,v) {
  console.log((jQuery(v).find('.column-name:contains("changed_by")').length)+","+v.id)
})

That gave me a list of table names preceded with “0” (no changed_by column) or “1” (has changed_by column). Putting that into Sublime Text 3, sorting, and then removing the leading zeroes yielded this list of tables without a changed_by:

active_list
active_list_allergy
active_list_problem
active_list_type
clob_datatype_storage
cohort_member
concept_answer
concept_class
concept_complex
concept_datatype
concept_name
concept_name_tag
concept_name_tag_map
concept_numeric
concept_proposal_tag_map
concept_reference_source
concept_set
concept_state_conversion
concept_stop_word
drug_ingredient
drug_order
encounter_type
field_answer
field_type
form_resource
global_property
hl7_in_archive
hl7_in_error
hl7_in_queue
hl7_source
liquibasechangelog
liquibasechangeloglock
location_tag_map
notification_alert_recipient
notification_template
obs
order_type_class_map
orders
patient_identifier_type
privilege
relationship_type
report_schema_xml
role
role_privilege
role_role
scheduler_task_config_property
test_order
user_property
user_role

Glancing through those and applying what I know off the top of my head about how we use these objects (e.g., we don’t track changes to mappings, we don’t edit obs or orders, etc.), yield this list of potential tables missing changed_by columns:

  • concept_answer
  • concept_class
  • concept_datatype
  • concept_name
  • concept_name_tag
  • concept_reference_source
  • concept_set
  • drug_ingredient
  • encounter_type
  • field_answer
  • form_resource
  • global_property
  • patient_identifier_type
  • relationship_type
  • user_property

Most of these are types (_type, _datatype) or answers (_answer), which we may just overwrite/replace (so wouldn’t need changed_by columns). The five (5) I marked in bold are the most curious to me – i.e., the most likely to benefit from adding changed_by columns.

Cheers,

-Burke :burke:

Great work @burke! I think you’re being a bit too stingy with our changed_by columns though. :slightly_smiling:

Typical metadata classes also should be auditable in this way, so we should also include:

  • concept_class
  • concept_name_tag
  • concept_reference_source
  • encounter_type

I would also include:

  • form_resource

I agree with Darius suggestions. ConceptReferenceSource is not updatable in the legacy UI but it’s not enforced in the API, so i would assume it should have the changed_* columns

I updated the original post to reflect the appropriate list of classes. Feel free to revise if I missed something.

@cathychen are you still interested/able to move this forward?

Hi there!

while working on hibernate mapping through annotations for Order https://issues.openmrs.org/browse/TRUNK-4903

I saw that the table orders is also missing these two columns (changed_by, date_changed).

Not sure if they should be added though.

Do we support changing of an order?

Technically we say orders are immutable from the perspective of an API consumer, but they are actually mutable within the API itself.