EA-186 - Loosen providermanagement to an aware_of dependency #230
Conversation
…g for dispositions to use obs groups
…ve all reporting dependencies into a new submodule
| @@ -1,6 +1,6 @@ | |||
| package org.openmrs.module.emrapi.account; | |||
|
|
|||
| import org.openmrs.module.providermanagement.Provider; | |||
| import org.openmrs.Provider; | |||
There was a problem hiding this comment.
This is a backwards-incompatible change to the ProviderIdentifierGenerator interface, though it now supports a broader, not narrower object, so should not pose any problems.
| /** | ||
| * Provides an abstraction over Provider services | ||
| */ | ||
| @OpenmrsProfile(modules = {"!providermanagement"}) |
There was a problem hiding this comment.
Provides an implementation of provider-related methods that are used when providermanagement is not installed. This would be where we'd implement logic to support setting roles on the core Provider object as Concept role, but that can be added in a separate PR if desired since that was not pre-existing functionality here.
| @@ -0,0 +1,97 @@ | |||
| package org.openmrs.module.emrapi.account.provider; | |||
There was a problem hiding this comment.
Provides an implementation of provider-related methods that are used when providermanagement is installed. This allows us to keep any references to providermanagement hidden behind this conditionally loaded bean. The setProviderRole method is the way it is as the method signature changed from ProviderRole to Object, and this allows supporting a wide range of possible values including bind parameters in controller requests.
| @@ -125,21 +122,23 @@ public Provider getProvider() { | |||
| return provider; | |||
| } | |||
|
|
|||
| public void setProviderRole(ProviderRole providerRole) { | |||
|
|
|||
There was a problem hiding this comment.
This is a necessary backwards-incompatibility on the get/set providerRole methods by broadening to Object as ProviderRole is not available when the providermanagement module is not installed.
|
|
||
| import java.io.File; | ||
|
|
||
| public abstract class EmrApiContextSensitiveTest extends BaseModuleContextSensitiveTest { |
There was a problem hiding this comment.
For now, I just made all of the context-sensitive unit tests in the module extend this, which adds the providermanagement module in so that the module-specific version of the facade is activated, as the existing unit tests expect that to be there. If there is a straightforward to support both variations in the same unit test suite, please let me know. I was not able to get it working.
There was a problem hiding this comment.
Just like you created BaseReportingTest in the previous pull request, shouldn't you rename this to something like BaseProvidermanagementTest?
There was a problem hiding this comment.
Because I make this the superclass of every context-sensitive test in the module, it felt more appropriate to name it this way.
|
|
||
| @Override | ||
| public Object getProviderRole(Provider provider) { | ||
| return null; |
There was a problem hiding this comment.
Shouldn't this also throw the same exception as setProviderRole?
There was a problem hiding this comment.
The way I was seeing it, provider role is always null if the providermanagement module is not running. So why throw an exception? The setter is a different situation - the user is trying to set something and the module is unable to accommodate.
| import static java.util.Collections.EMPTY_LIST; | ||
| import static java.util.Collections.reverseOrder; | ||
| import static java.util.Collections.sort; | ||
| import static java.util.Collections.*; |
There was a problem hiding this comment.
There was a problem hiding this comment.
Strange, I feel like my version of IntelliJ started doing wildcard imports again as well after I turned it off years ago.
| import static java.util.Collections.EMPTY_LIST; | ||
| import static java.util.Collections.reverseOrder; | ||
| import static java.util.Collections.sort; | ||
| import static java.util.Collections.*; |
There was a problem hiding this comment.
Strange, I feel like my version of IntelliJ started doing wildcard imports again as well after I turned it off years ago.
No description provided.