Skip to content

AC-829 Fixed crash at admission form by using live data #804

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
f4ww4z merged 1 commit into from
Aug 17, 2020
Merged

AC-829 Fixed crash at admission form by using live data #804

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions ...client/src/main/java/org/openmrs/mobile/activities/formadmission/FormAdmissionContract.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ interface FormAdmissionContract {
interface Presenter : BasePresenterContract {
fun getProviders(formAdmissionFragment: FormAdmissionFragment?)
fun updateViews(providerList: List<Provider?>?)
fun getLocation(url: String?)
fun getEncounterRoles()
fun getLocation(url: String?, formAdmissionFragment: FormAdmissionFragment?)
fun getEncounterRoles(formAdmissionFragment: FormAdmissionFragment?)
fun createEncounter(providerUUID: String?, locationUUID: String?, encounterRoleUUID: String?)
}
}
4 changes: 2 additions & 2 deletions ...client/src/main/java/org/openmrs/mobile/activities/formadmission/FormAdmissionFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ class FormAdmissionFragment : ACBaseFragment<FormAdmissionContract.Presenter>(),
formAdmissionBinding = FragmentFormAdmissionBinding.inflate(inflater, container, false)
val root: View = formAdmissionBinding.root
initFragmentFields()
mPresenter?.getEncounterRoles()
mPresenter?.getEncounterRoles(this)
mPresenter?.getProviders(this)
mPresenter?.getLocation(OpenMRS.getInstance().serverUrl)
mPresenter?.getLocation(OpenMRS.getInstance().serverUrl, this)
return root
}

Expand Down
44 changes: 21 additions & 23 deletions ...lient/src/main/java/org/openmrs/mobile/activities/formadmission/FormAdmissionPresenter.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ import org.openmrs.mobile.dao.PatientDAO
import org.openmrs.mobile.databases.AppDatabase
import org.openmrs.mobile.databases.entities.LocationEntity
import org.openmrs.mobile.listeners.retrofitcallbacks.DefaultResponseCallback
import org.openmrs.mobile.listeners.retrofitcallbacks.EncounterResponseCallback
import org.openmrs.mobile.listeners.retrofitcallbacks.LocationResponseCallback
import org.openmrs.mobile.models.Patient
import org.openmrs.mobile.models.Provider
import org.openmrs.mobile.models.Resource
Expand Down Expand Up @@ -59,7 +57,7 @@ class FormAdmissionPresenter : BasePresenter, FormAdmissionContract.Presenter {
restApi = RestServiceBuilder.createService(RestApi::class.java)
this.view.setPresenter(this)
mContext = context
providerRepository = ProviderRepository(restApi)
providerRepository = ProviderRepository(context)
}

constructor(formAdmissionView: FormAdmissionContract.View, restApi: RestApi, context: Context) {
Expand Down Expand Up @@ -87,30 +85,30 @@ class FormAdmissionPresenter : BasePresenter, FormAdmissionContract.Presenter {
}
}

override fun getLocation(url: String?) {
providerRepository.getLocation(url, object : LocationResponseCallback {
override fun onResponse(locationList: List<LocationEntity?>?) {
view.updateLocationAdapter(locationList)
}
override fun getLocation(url: String?, formAdmissionFragment: FormAdmissionFragment?) {
providerRepository.getLocation(url).observe(formAdmissionFragment!!, Observer { locationList: List<LocationEntity?>? -> updateLocationList(locationList) })
}

override fun onErrorResponse(errorMessage: String) {
view.showToast(errorMessage)
view.enableSubmitButton(false)
}
})
fun updateLocationList(locationList: List<LocationEntity?>?) {
if(locationList != null && locationList.isNotEmpty()) {
view.updateLocationAdapter(locationList)
} else {
view.enableSubmitButton(false)
view.showToast(mContext.resources.getString(R.string.error_occurred))
}
}

override fun getEncounterRoles() {
providerRepository.getEncounterRoles(object : EncounterResponseCallback {
override fun onResponse(encounterRoleList: List<Resource?>?) {
view.updateEncounterRoleList(encounterRoleList)
}
override fun getEncounterRoles(formAdmissionFragment: FormAdmissionFragment?) {
providerRepository.encounterRoles.observe(formAdmissionFragment!!, Observer { encounterRolesList: List<Resource?>? -> updateEncounterRoles(encounterRolesList) })
}

override fun onErrorResponse(errorMessage: String) {
view.enableSubmitButton(false)
view.showToast(errorMessage)
}
})
fun updateEncounterRoles(encounterRolesList: List<Resource?>?) {
if(encounterRolesList != null && encounterRolesList.isNotEmpty()) {
view.updateEncounterRoleList(encounterRolesList)
} else {
view.enableSubmitButton(false)
view.showToast(mContext.resources.getString(R.string.error_occurred))
}
}

override fun createEncounter(providerUUID: String?, locationUUID: String?, encounterRoleUUID: String?) {
Expand Down
40 changes: 17 additions & 23 deletions openmrs-client/src/main/java/org/openmrs/mobile/api/repository/ProviderRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,6 @@
import org.openmrs.mobile.databases.AppDatabase;
import org.openmrs.mobile.databases.entities.LocationEntity;
import org.openmrs.mobile.listeners.retrofitcallbacks.DefaultResponseCallback;
import org.openmrs.mobile.listeners.retrofitcallbacks.EncounterResponseCallback;
import org.openmrs.mobile.listeners.retrofitcallbacks.LocationResponseCallback;
import org.openmrs.mobile.models.Provider;
import org.openmrs.mobile.models.Resource;
import org.openmrs.mobile.models.Results;
Expand Down Expand Up @@ -250,54 +248,50 @@ public void onFailure(@NotNull Call<ResponseBody> call, @NotNull Throwable t) {
}
}

public void getLocation(String url, LocationResponseCallback callback) {
public LiveData<List<LocationEntity>> getLocation(String url) {
MutableLiveData<List<LocationEntity>> locations = new MutableLiveData<>();
if (NetworkUtils.hasNetwork()) {
String locationEndPoint = url + ApplicationConstants.API.REST_ENDPOINT + "location";
Call<Results<LocationEntity>> call =
restApi.getLocations(locationEndPoint, ApplicationConstants.API.TAG_ADMISSION_LOCATION, ApplicationConstants.API.FULL);
call.enqueue(new Callback<Results<LocationEntity>>() {
@Override
public void onResponse(Call<Results<LocationEntity>> call, Response<Results<LocationEntity>> response) {
if (callback != null) {
if (response.isSuccessful()) {
callback.onResponse(response.body().getResults());
} else {
callback.onErrorResponse(OpenMRS.getInstance().getString(R.string.error_occurred));
}
if (response.isSuccessful()) {
locations.setValue(response.body().getResults());
} else {
locations.setValue(null);
}
}

@Override
public void onFailure(Call<Results<LocationEntity>> call, Throwable t) {
callback.onErrorResponse(t.getMessage());
locations.setValue(null);
}
});
} else {
if (callback != null) {
callback.onErrorResponse(OpenMRS.getInstance().getString(R.string.error_occurred));
}
locations.setValue(null);
}
return locations;
}

public void getEncounterRoles(EncounterResponseCallback callback) {
public LiveData<List<Resource>> getEncounterRoles() {
MutableLiveData<List<Resource>> encounterRoles = new MutableLiveData<>();
restApi.getEncounterRoles().enqueue(new Callback<Results<Resource>>() {
@Override
public void onResponse(Call<Results<Resource>> call, Response<Results<Resource>> response) {
if (callback != null) {
if (response.isSuccessful()) {
callback.onResponse(response.body().getResults());
} else {
callback.onErrorResponse(OpenMRS.getInstance().getString(R.string.error_occurred));
}
if (response.isSuccessful()) {
encounterRoles.setValue(response.body().getResults());
} else {
encounterRoles.setValue(null);
}
Copy link
Collaborator

@dino-saurabh dino-saurabh Aug 15, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rishabh-997 how are we taking care of the UI responses to the user after the callback has been removed all the views which implemented this callback wont get any callback to handle ?

Copy link
Collaborator Author

@rishabh-997 rishabh-997 Aug 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We were using callbacks because we were not observing data. Now that I have changed return type to LiveData<T>, we don't need to use callbacks, the UI will simply observe results...
LiveData will allow the observer to be removed when the state of the corresponding Lifecycle object changes to DESTROYED. This is useful for activities and fragments because they do not need to worry about leaks—activities and fragments are instantly unsubscribed when their lifecycles are destroyed.

Copy link
Collaborator

@dino-saurabh dino-saurabh Aug 16, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes @rishabh-997 actually i missed the changes you made in presenter I thought they were missing .

}

@Override
public void onFailure(Call<Results<Resource>> call, Throwable t) {
if (callback != null) {
callback.onErrorResponse(t.getMessage());
}
encounterRoles.setValue(null);
}
});
return encounterRoles;
}
}
42 changes: 29 additions & 13 deletions ...s-client/src/test/java/org/openmrs/mobile/test/presenters/FormAdmissionPresenterTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,8 @@
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;

import io.reactivex.Single;
Expand All @@ -66,15 +66,25 @@ public class FormAdmissionPresenterTest extends ACUnitTestBase {
public InstantTaskExecutorRule taskExecutorRule = new InstantTaskExecutorRule();
MutableLiveData<List<Provider>> providerLiveData = Mockito.mock(MutableLiveData.class);
List<Provider> providerList;
List<Resource> resourceList;
List<LocationEntity> locationList;
Provider providerOne = createProvider(1l, "doctor");
Provider providerTwo = createProvider(2l, "nurse");
LocationEntity locationEntityOne = new LocationEntity("entity 1");
LocationEntity locationEntityTwo = new LocationEntity("entity 2");
Resource resourceOne = new Resource("uuid", "display", new ArrayList<>(), 1L);
Resource resourceTwo = new Resource("uuid 2", "display 2", new ArrayList<>(), 2L);
@Mock
private RestApi restApi;
@Mock
private FormAdmissionContract.View formAdmissionView;
@Mock
private Observer<List<Provider>> observer;
@Mock
private Observer<List<LocationEntity>> locationObserver;
@Mock
private Observer<List<Resource>> resourceObserver;
@Mock
private OpenMRSLogger openMRSLogger;
@Mock
private OpenMRS openMRS;
Expand All @@ -90,6 +100,8 @@ public class FormAdmissionPresenterTest extends ACUnitTestBase {
public void setUp() {
mockStaticMethods();
providerList = Arrays.asList(providerOne, providerTwo);
locationList = Arrays.asList(locationEntityOne, locationEntityTwo);
resourceList = Arrays.asList(resourceOne, resourceTwo);
providerLiveData.postValue(providerList);

this.providerRepository = new ProviderRepository(restApi);
Expand Down Expand Up @@ -131,22 +143,24 @@ public void shouldGetProviders_Error() {
public void shouldGetAdmissionLocation_AllOK() {
Mockito.lenient().when(NetworkUtils.hasNetwork()).thenReturn(true);
Mockito.lenient().when(restApi.getLocations(any(), anyString(), anyString()))
.thenReturn(mockSuccessCall(Collections.singletonList(new LocationEntity(""))));
formAdmissionPresenter.getLocation("someUrl");
verify(formAdmissionView).updateLocationAdapter(any());
.thenReturn(mockSuccessCall(locationList));
formAdmissionPresenter.updateLocationList(locationList);
providerRepository.getLocation("some url").observeForever(locationObserver);
verify(formAdmissionView).updateLocationAdapter(locationList);
}

@Test
public void shouldLoadLocations_errorResponse() {
Mockito.lenient().when(NetworkUtils.hasNetwork()).thenReturn(true);
Mockito.lenient().when(restApi.getLocations(any(), anyString(), anyString()))
.thenReturn(mockErrorCall(401));
.thenReturn(mockErrorCall(401));

Resources res = Mockito.mock(context.getResources().getClass());
PowerMockito.when(context.getResources()).thenReturn(res);
Mockito.when(OpenMRS.getInstance().getString(Mockito.anyInt())).thenReturn("error_message");
Mockito.when(res.getString(Mockito.anyInt())).thenReturn("error_message");

formAdmissionPresenter.getLocation("someUrl");
formAdmissionPresenter.updateLocationList(null);
providerRepository.getLocation("some url").observeForever(locationObserver);
verify(formAdmissionView).showToast(anyString());
verify(formAdmissionView).enableSubmitButton(false);
}
Expand All @@ -155,22 +169,24 @@ public void shouldLoadLocations_errorResponse() {
public void shouldGetEncounterRoles_AllOK() {
Mockito.lenient().when(NetworkUtils.hasNetwork()).thenReturn(true);
Mockito.lenient().when(restApi.getEncounterRoles())
.thenReturn(mockSuccessCall(Collections.singletonList(new Resource())));
formAdmissionPresenter.getEncounterRoles();
verify(formAdmissionView).updateEncounterRoleList(any());
.thenReturn(mockSuccessCall(resourceList));
formAdmissionPresenter.updateEncounterRoles(resourceList);
providerRepository.getEncounterRoles().observeForever(resourceObserver);
verify(formAdmissionView).updateEncounterRoleList(resourceList);
}

@Test
public void shouldLoadEncounterRoles_errorResponse() {
Mockito.lenient().when(NetworkUtils.hasNetwork()).thenReturn(true);
Mockito.lenient().when(restApi.getEncounterRoles())
.thenReturn(mockErrorCall(401));
.thenReturn(mockErrorCall(401));

Resources res = Mockito.mock(context.getResources().getClass());
PowerMockito.when(context.getResources()).thenReturn(res);
Mockito.when(OpenMRS.getInstance().getString(Mockito.anyInt())).thenReturn("error_message");
Mockito.when(res.getString(Mockito.anyInt())).thenReturn("error_message");

formAdmissionPresenter.getEncounterRoles();
formAdmissionPresenter.updateEncounterRoles(null);
providerRepository.getEncounterRoles().observeForever(resourceObserver);
verify(formAdmissionView).showToast(anyString());
verify(formAdmissionView).enableSubmitButton(false);
}
Expand Down