Skip to content

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

Merged
f4ww4z merged 1 commit intoopenmrs:masterfrom
rishabh-997:AC-829-admission
Aug 17, 2020
Merged

AC-829 Fixed crash at admission form by using live data#804
f4ww4z merged 1 commit intoopenmrs:masterfrom
rishabh-997:AC-829-admission

Conversation

@rishabh-997
Copy link
Collaborator

Description of what I changed

We need to observe the encounter roles and locations using live data as in dark mode, activity is re-created immediately and the results cause NPE as the fragment is destroyed on which it needed to show.

Issue I worked on

JIRA Issue: https://issues.openmrs.org/browse/AC-829

Checklist: I completed these to help reviewers :)

  • My pull request only contains ONE single commit
    (the number above, next to the 'Commits' tab is 1).
  • I have added tests to cover my changes. (If you refactored
    existing code that was well tested you do not have to add tests)
  • All new and existing tests passed.
  • My pull request is based on the latest changes of the master branch.

@rishabh-997 rishabh-997 requested a review from f4ww4z August 13, 2020 22:56
@codecov-commenter
Copy link

codecov-commenter commented Aug 13, 2020

Codecov Report

Merging #804 into master will not change coverage.
The diff coverage is 61.53%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #804   +/-   ##
=======================================
  Coverage   13.47%   13.47%           
=======================================
  Files         238      238           
  Lines        9672     9672           
  Branches      928      925    -3     
=======================================
  Hits         1303     1303           
  Misses       8263     8263           
  Partials      106      106           
Impacted Files Coverage Δ
.../activities/formadmission/FormAdmissionFragment.kt 7.01% <0.00%> (ø)
...activities/formadmission/FormAdmissionPresenter.kt 20.68% <54.54%> (-4.32%) ⬇️
...nmrs/mobile/api/repository/ProviderRepository.java 36.58% <76.92%> (+2.18%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1ecacfc...c43dea8. Read the comment docs.

encounterRoles.setValue(response.body().getResults());
} else {
encounterRoles.setValue(null);
}
Copy link
Collaborator

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

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

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 .

Copy link
Collaborator

@f4ww4z f4ww4z left a comment

Choose a reason for hiding this comment

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

Good work @rishabh-997

@f4ww4z f4ww4z merged commit ca4ad94 into openmrs:master Aug 17, 2020
@rishabh-997 rishabh-997 deleted the AC-829-admission branch August 19, 2020 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants