org.openmrs.api.APIException: Editing some fields on Obs is not allowed

Development
,
21

First and foremost, that test is in the wrong class, resource tests need to be separated from controller tests, resource tests test structure of a resource i.e the representation where as controller tests test behavior i.e CRUD operations and they should extend different base classes, please take a look at resource and controller tests in the web services module for examples and try to follow the same.

The tests fails because of the call that fetches the GP on line 167 and that’s in core so you can’t really fix it, it triggers a premature hibernate flush that tries to save the changes on the Obs to the DB which leads to the exception you’re running into, this is when hibernate can frustrate you but that’s how it works, I believe this will be addressed as part of the efforts to support saving a complex obs via rest so that we turn off auto commit when saving an Obs.

22

@dkayiwa,

Does this solve it: EA-107? If yes then running the unit test on EMRAPI 1.19-SNAPSHOT still fails… am I missing something?

23

Hi @wyclif,

You are correct about the unit tests. Please note that they represented an intermediate commit to highlight (and reproduce) the issue, and were not meant to stay. I have got rid of my custom resource extending ObsResource* in exchange of another completely custom resource to work on and around complex obs in the context of VDUI: VisitDocumentResource. This was necessary for reasons outside of this thread and makes more sense from a design standpoint.

Although the 2.x-related parts of my module now depend on the latest EMR API (1.9-SNAPSHOT) in order to include the fix for EA-107, “Editing some fields on Obs is not allowed” Error because of hibernate auto flush due to other hibernate call ; the problem still does not go away.

I have a 2.0-based controller unit test that attempts to edit a complex obs’s comment and that generates the above error. See VisitDocumentController2_0Test.postVisitDocument_shouldUpdateObsComment().

For the record the pre-2.0 identical test passes and can be found here.

Could anyone have a peak and confirm that the issue is not on our end? The whole module containing the above tests can be cloned here: commit 047b983.

Cc: @dkayiwa

24

@mksd did you try using the new EA-107 utility class in the emrapi module to disable auto flush and then enable it only after your saving?

25

Ok sorry I thought that EA-107 was solving the issue altogether. Doing this works (in my unit test):

@Override
public VisitDocument save(VisitDocument delegate) {
  FlushMode flushMode = DbSessionUtil.getCurrentFlushMode();
  DbSessionUtil.setManualFlushMode();
  try {
    Context.getObsService().saveObs(delegate.getObs(), REASON);
  } finally {
    DbSessionUtil.setFlushMode(flushMode);
  }
  return delegate;
}
26

@wyclif, @dkayiwa

I just pulled again the latest distro 2.5 and upgraded EMR API to 1.19-SNAPSHOT (in order to benefit from DbSessionUtil), and, despite having the above fix implemented on my 2.0-compatible resource, the problem came back! … with a more precise error message though:

Editing some fields [comment] on Obs is not allowed

Note that, on the other hand, my safeguard unit test still does not fail.

This is with

  • Platform 2.0.1 SNAPSHOT Build 5126e7
  • EMR API 1.19-SNAPSHOT Build 9891ce

Any idea about what may be happening? My whole branch here: f02a55.

27

@mksd is this failing in a unit test?

28

As I said, not anymore…

Could it be that my unit test runs against a slightly different Platform 2.0.1 version than the runtime one?

29

Here is the stack trace upon saving the obs.

30

@dkayiwa, as discussed on IRC, I tried this on a non-SDK server, see below. I did it the “old school” way, every mvn command is run with Java 8:

  • I checked out commit 5126e7 of the Platform.
  • After successful mvn clean install, I ran mvn jetty:run from within /webapp.
  • I went through the initial install and then I shut down the server.
  • I copied the modules in ~/.OpenMRS/modules.
  • Finally I mvn jetty:run again, this time with Ref App 2.5 up & running.

I got the exact same stack trace upon saving the obs…

31

@mksd if i wanted to reproduce this locally, is it simply a matter of compiling your module and install on platform 2.0?

32

With those:

  • Platform 2.0.1 SNAPSHOT Build 5126e7
  • EMR API 1.19-SNAPSHOT Build 9891ce

You will need a patient with an active visit, and then follow up with me on IRC so that I can point to the exact steps to reproduce the issue.

33

@mksd from your stack trace, this looks like a complex obs. Do you have a unit test for posting complex obs?

34

Both VisitDocumentController1_10Test and VisitDocumentController2_0Test create and consume a test complex obs.

See this for instance:

@Before
public void setup() throws IOException {
  obs = testHelper.getTestComplexObs();
}
35

I mean a post like in postVisitDocument_shouldUpdateObsComment but dealing with a complex obs in the exact way like what you are doing in the running app.

36

TestHelper is using the exact same class (ComplexObsSaver) as the controller that saves the complex obs when running the webapp:

obsSaver.saveOtherDocument(patient, encounter, RandomStringUtils.randomAlphabetic(12), multipartFile, ValueComplex.INSTRUCTIONS_DEFAULT);

The test helper class just reproduces what is actually done when you run the app.

There is no REST end point to post a complex obs within VDUI, for instance this is the actual line that does it in the controller that is used to create them:

obsSaver.saveOtherDocument(patient, encounter, fileCaption, multipartFile, instructions);

So the controller tests you are looking at just do the exact same thing:

  1. Save a complex obs with ComplexObsSaver.
  2. Update it with VisitDocumentResource.
37

Switching module versions in the reference application can be very problematic since module dependencies can get complex, why do you have to do it? If you wish to use a snapshot version of any module in the RA, in most cases it is easier to take a snapshot build of the distro itself instead of just replacing a single module with a snapshot version.

38

The snapshot build that I used yesterday depends on EMR API 1.18 that does not contain DbSessionUtil. Hence the need to swap it for EMR API 1.19-SNAPSHOT.

@dkayiwa, have you had a chance to look into this?

39

@mksd am on IRC for you to take me through the details of reproducing.

40

Because they are about to release a new version of the RA, the module versions are not snapshots but in most cases they are, so this is kind of an awkward situation.