GSCoC 2018 / 'Attachments enhancements'

@ridmal @zouchine @ahmed14 we have a call in 30 mins.

1 Like

sure

Get Outlook for Androidhttps://aka.ms/ghei36

Ā·Ā·Ā· ________________________________ From: Dimitri R Sent: Thursday, May 3, 2018 2:02:05 PM To: engrmahmed14@outlook.com Subject: Re: [PM] GSCoC 2018 / 'Attachments enhancements'

@ridmal</u/ridmal> @zouchine</u/zouchine> @ahmed14</u/ahmed14> we have a call in 30 mins.


Visit Messagehttps://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/21 or reply to this email to respond.

To unsubscribe from these emails, click herehttps://talk.openmrs.org/email/unsubscribe/8fb7faa53b51f84cf9750c5567a6e2bd1d27bee1fcaee1895dfd3f6872d485f5.

@mksd ,\

i am on skype but havnā€™t received any request or call yet.

Sorry @ahmed14, looks like youā€™re the only one onlineā€¦

I searched your ID in skype and there are two account in same name. Which account do i need to select for sending the request ?

ok, then can we reschedule it later ?

@ridmal I will call you first then invite @ahmed14 to the call.

Please both connect on IRC so that we can setup things in a more ā€œimmediateā€ way out there.

Hi @ridmal, everything ok? I havenā€™t heard from you since our call last weekā€¦

I sent a message to dileka to asking about ATT -24. I havenā€™t get any response yet. I will clone his repo when i get his permission.

I got a message from the Dileka and he gave his approval to use his work and continue with ATT-24. I will clone his repo and get familiar with the existing implementation. :slight_smile:

1 Like

hi @mksd , Did you review the last changes related to the ATT-24 issue ? ( Which is implementing a AttachmentService inside the Attachments module done by dieka. )

And there is many comments with that PR and I am little bit confuse about that. So can please help me to sort it out that what are the further implementation need to be done for the ATT-24-1. ( adding AttachmentService to Api ) :slight_smile:

The newly introduced service provides an API to retrieve, for a given patient, the list of attachments

  • within a given encounter
  • within a given visit

What I was asking Dileka was to put together unit tests that show that it all works as expected, and he was almost there. The bottom line is: attachments are not new entities, they are just wrappers around complex obs, so they are just a special kind of obs that are mixed with the others in encounters and visits.

So writing a unit test that demonstrates that the new API does indeed list the attachments of an encounter or a visit means setting up encounters and visits that mix up complex obs with other obs ; to then show that only the expected ones are returned: the complex obs (but wrapped as attachments).

Does that make sense? Here is how you can start off from where Dileka left it. After you have cloned Attachments, you can do this:

git remote add dileka https://github.com/dilekamadushan/openmrs-module-attachments
git fetch dileka
git checkout -b ATT-24 dileka/ATT-24-1

And at that point you will be on a local branch ATT-24 that carries all latest commits from Dileka.

Yes i got some idea , and I already follow the git commands and setup my local machine. Ok i will check about the unit test above you mentioned. :slight_smile:

SO i read the AttachmentsService ahead of REST API work (ATT-24) thread and here is the first question come to my mind when i check the implementation. Please correct if i am wrong.

There are 3 methods implemented in AttachmentsServiceImpl for get the attachments based on the encounters and visits. So inside that Dileka used getConceptComplexList() method for only fetch the complex obs , which coded by the concepts configured through the module. So that means from the API we can get only the obs relevant to this module. Am i correct? Is there anything to implement or modify inside that methods?

Correct.

Yes.

I want you to focus on the unit tests, not the implementation. Get a bunch of solid unit tests that show that when there is a mix of obs that are saved, fetching the attachments for a given encounter or a given visit produces the expected results. As soon as you have all those tests in place, we can revisit Dilekaā€™s implementation.

Makes sense?

Ok , But there are two existing unit tests implemented in AttachmentsServiceTest for above mentioned purpose. Am I right? So Inside the getAttachments_shouldReturnEncounterAttachments() method, Dileka try to create 3 complex obs and 1 non-complex obs based on encounter and verify with AttachementService API that it only get the complex obs. Seems right thing to do. Is there any modifications need to be done in there?

Imagine that you had two complex obs saved within an encounter: one being managed by Attachments and the other one not. Would the service return only the expected one out of the two?

In order to make it more collection-minded, I would check this test case:

  • Save in the same encounter: 2 obs, 2 non-Attachments managed complex obs and 2 Attachments-managed complex obs.
  • Verify that the service only returns the two Attachments-managed ones.
  • Edit the configuration and encompass one (or two) of the other complex obs to be also Attachments-managed.
  • Verify that the service returns the two original Attachments-managed ones plus the one (or two) that were encompassed through the configuration change.

That would be a solid test for the encounter-based query. Then we can expand on that one to test that visit-based queries also produce the expected results.

As soon as we have solid tests in place, I would like to streamline the implementation as I felt that almost the same code was copy/pasted from a method to the other, instead of introducing reusable blocks. But tests first.

1 Like

Ok , thank you very much for your clarifications.I will modify the unit test as you mentioned . :slight_smile:

I have been stuck a little bit with creating a non-attachments managed complex Obs. I tried to create Complex Obs using following method ,

But It failed i used this complex Concept UUID and Its only work with concept UUIDs relevant to the attachment module. ({ā€œ7cac8397-53cd-4f00-a6fe-028e8d743f8eā€ and ā€œ42ed45fd-f3f6-44b6-bfc2-8bde1bb41e00ā€}).

Any suggestions about this ?

Are you trying to save some other complex obs? If yes you may just load this test data set. That should give you two of them, see ObsServiceTest-complex.xml in Core.

As it is they are not associated with any visits or encounters, so you will have edit and re-save them as part of your testā€™s setup.

1 Like