Skip to content

AC-476 Updated PatientDAO for room including six tests#682

Merged
f4ww4z merged 2 commits intoopenmrs:masterfrom
rishabh-997:patientDAO
Jan 24, 2020
Merged

AC-476 Updated PatientDAO for room including six tests#682
f4ww4z merged 2 commits intoopenmrs:masterfrom
rishabh-997:patientDAO

Conversation

@rishabh-997
Copy link
Collaborator

Description of what I changed

Updated PatientDAO for room including six tests

Issue I worked on

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

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.

added tests for PatientRoomDAO

Updated PatientDAO for room including six tests
@rishabh-997 rishabh-997 requested a review from f4ww4z January 20, 2020 14:02
@codecov-io
Copy link

codecov-io commented Jan 20, 2020

Codecov Report

Merging #682 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #682   +/-   ##
=======================================
  Coverage   12.59%   12.59%           
=======================================
  Files         232      232           
  Lines       10643    10643           
  Branches      991      991           
=======================================
  Hits         1340     1340           
  Misses       9222     9222           
  Partials       81       81
Impacted Files Coverage Δ
...java/org/openmrs/mobile/databases/AppDatabase.java 0% <ø> (ø) ⬆️

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 edbd66f...a1a70fd. Read the comment docs.

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.

@rishabh-997 See my comments below.

Assert.assertEquals(patientEntities.get(1).getPhoto(),expectedPatientEntity2.getPhoto());
Assert.assertEquals(patientEntities.get(1).getPostalCode(),expectedPatientEntity2.getPostalCode());
Assert.assertEquals(patientEntities.get(1).getState(),expectedPatientEntity2.getState());
Assert.assertEquals(patientEntities.get(1).isSynced(),expectedPatientEntity2.isSynced());
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 It's easier to check the equality of the objects, like:

Assert.assertEquals(patientEntities.get(0), expectedPatientEntity1);
Assert.assertEquals(patientEntities.get(1), expectedPatientEntity2);

Assert.assertEquals(patientEntities.get(0).getPhoto(),expectedPatientEntity2.getPhoto());
Assert.assertEquals(patientEntities.get(0).getPostalCode(),expectedPatientEntity2.getPostalCode());
Assert.assertEquals(patientEntities.get(0).getState(),expectedPatientEntity2.getState());
Assert.assertEquals(patientEntities.get(0).isSynced(),expectedPatientEntity2.isSynced());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same here. Assert.assertEquals(patientEntities.get(0), expectedPatientEntity2);

Assert.assertEquals(patientEntities.get(0).getPhoto(),expectedPatientEntity1.getPhoto());
Assert.assertEquals(patientEntities.get(0).getPostalCode(),expectedPatientEntity1.getPostalCode());
Assert.assertEquals(patientEntities.get(0).getState(),expectedPatientEntity1.getState());
Assert.assertEquals(patientEntities.get(0).isSynced(),expectedPatientEntity1.isSynced());
Copy link
Collaborator

Choose a reason for hiding this comment

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

See above comment.

Assert.assertEquals(patientEntities.get(0).getPhoto(),expectedPatientEntity1.getPhoto());
Assert.assertEquals(patientEntities.get(0).getPostalCode(),expectedPatientEntity1.getPostalCode());
Assert.assertEquals(patientEntities.get(0).getState(),expectedPatientEntity1.getState());
Assert.assertEquals(patientEntities.get(0).isSynced(),expectedPatientEntity1.isSynced());
Copy link
Collaborator

Choose a reason for hiding this comment

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

See above comment.


@Override
public void onSuccess(List<PatientEntity> patientEntities) {
Assert.assertEquals(patientEntities.size(),1);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Assert.assertEquals(patientEntities.size(),1);
Assert.assertEquals(patientEntities.size(), 1);

Assert.assertEquals(patientEntities.size(),1);

Assert.assertEquals(patientEntities.get(0).getGivenName(),"Rishabh");
Assert.assertEquals(patientEntities.get(0).getFamilyName(),"Agarwal");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Needs a space for correct formatting too.

});
}

private EncounterEntity getEncounterEntity(Long id, String uuid, String display, String visitKeyId,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename this to newEncounterEntity

return encounterEntity;
}

private PatientEntity getPatientEntity(long id, String uuid, String display,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
private PatientEntity getPatientEntity(long id, String uuid, String display,
private PatientEntity newPatientEntity(long id, String uuid, String display,

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.

Nice @rishabh-997 !

@f4ww4z f4ww4z merged commit 1a013dd into openmrs:master Jan 24, 2020
@rishabh-997 rishabh-997 deleted the patientDAO branch January 24, 2020 21:04
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.

3 participants