RA-2030: Fix UI framework error in the reference application

[wikumChamith]

No description provided.

[wikumChamith]

@dkayiwa wouldn't it be safer to rename this rather than remove it?

[dkayiwa]

Why are you removing this?

[wikumChamith]

This variable does not get used after removing createConceptSource(conceptService);

[dkayiwa]

Why do we remove the createConceptSource?

[wikumChamith]

It is what creates the duplicate org.openmrs.module.emrapi concept_reference_source. According to this comment this method was created for use only in tests.

openmrs-module-emrapi/api/src/main/java/org/openmrs/module/emrapi/EmrApiActivator.java

Lines 343 to 358 in 35032fa

	/**
	* (public so that it can be used in tests, but you shouldn't use this in production code)
	* Creates a single ConceptSource which we will use to tag concepts relevant to this module
	*
	*/
	public ConceptSource createConceptSource(ConceptService conceptService) {
	ConceptSource conceptSource = conceptService.getConceptSourceByName(EmrApiConstants.EMR_CONCEPT_SOURCE_NAME);
	if (conceptSource == null) {
	conceptSource = new ConceptSource();
	conceptSource.setName(EmrApiConstants.EMR_CONCEPT_SOURCE_NAME);
	conceptSource.setDescription(EmrApiConstants.EMR_CONCEPT_SOURCE_DESCRIPTION);
	conceptSource.setUuid(EmrApiConstants.EMR_CONCEPT_SOURCE_UUID);
	conceptService.saveConceptSource(conceptSource);
	}
	return conceptSource;
	}

[dkayiwa]

So where are we creating the original org.openmrs.module.emrapi concept source from?

[dkayiwa]

@wikumChamith in that case let us just rename this https://github.com/openmrs/openmrs-module-referencemetadata/blob/master/api/src/main/resources/Reference_Application_Concepts-27.xml#L885 to something like org.openmrs.module.emrapi2

[mogoodrich]

Thanks @dkayiwa @wikumChamith !

I think I understand what is going, not sure we want to just rename the EMR API concept source in the reference data, let me give this some thought...

[mogoodrich]

@ibacher @dkayiwa what is the history (and ideal future state?) of the concepts loaded via the reference metadata module vs those loaded via the OCL packages in the reference distro? I may need to hack around this a bit and I'm wondering if one option may be to install all the EMR API concepts and concept mappings via OCL... (this actually may already be happened). I'm guessing we are trying to move to install all the concepts via OCL anyway? fyi @wikumChamith

[mseaton]

My general take is that we should never have been creating a concept source in the EMR API module activator - as the comment says, this was never supposed to be done in production. Unfortunately, it's been there since 2013! :)

So I'm in favor of removing this from the activator. The big question to figure out is how to fix this in systems where it has already been installed. In these cases, we will need to do an update to replace the uuid of any existing source with the expected uuid as defined elsewhere in that distribution (eg. in MDS packages, etc).

Ideally, core would be smart enough to deal with this, but it doesn't appear to be so. That would be another place were this could be addressed. Basically - don't allow creating concept sources with duplicate names.

[mogoodrich]

Thanks @mseaton !

Yeah, actually, thinking about what you said, it might be as simple as just removing this and relying on a distro to make sure this mapping is present (if needed). It looks like both the PIH EMR and the Ref App already do this.

(I don't know if we need to do anything to update the UUID because the EMR API method we'd be removing does not specify a uuid, so if having a consistent uuid is a requirement, this already would have been causing problems if this method was hit before any other install?)

[mogoodrich]

So after thinking through @mseaton 's comments I think he is correct, and we actually can just merge this PR as-is with the original fix from @wikumChamith an no longer create the EMR API concept source.

Any distribution that needs this concept source will need to make sure that it creates it on it's on, but I suspect most/many do... (Ref App and PIH EMR do) fyi @rbuisson

I don't think we need to worry about changing/cleaning the uuids on this concept source... see my comments above... is there a use case I'm not thinking of @mseaton ?

Approving this PR....

[mogoodrich]

Thanks @wikumChamith and @dkayiwa !