# GSCoC 2018 / 'Attachments enhancements'

**URL:** <https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701>\
**Category:** GSoC\
**Tags:** attachments, gsoc2018\
**Created:** [April 24, 2018, 6:01am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701 "2018-04-24T06:01:52Z")\
**Posts on this page:** 20\
**Page:** 4

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [May 29, 2018, 6:04pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/61 "2018-05-29T18:04:39Z")

</div>

Is there a way to directly get the [TestHelper](https://github.com/ridmal/openmrs-module-attachments/blob/27a3a5b8a1d8eeb354933ee93c2e83a8365484e6/api/src/test/java/org/openmrs/module/attachments/obs/TestHelper.java#L35) class ( inside the **/api** ) for the [AttachmentServiceTest](https://github.com/ridmal/openmrs-module-attachments/blob/27a3a5b8a1d8eeb354933ee93c2e83a8365484e6/api-2.0/src/test/java/org/openmrs/module/attachments/AttachmentsServiceTest.java#L26) class inside the /api-2.0.

Or may I create a new testHelper class inside the /api-2.0 and extend with the above mentioned TestHelper class and add the additional methods required for the /api-2.0 ?

---

<div class="post-metadata">

**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\
**Post date:** [May 29, 2018, 6:31pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/62 "2018-05-29T18:31:52Z")

</div>

Ah ok I understand the problem.

Just add this dependency to **api-2.0/pom.xml** :

```xml
<dependency>
  <groupId>${project.parent.groupId}</groupId>
  <artifactId>${project.parent.artifactId}-api</artifactId>
  <version>${project.parent.version}</version>
  <classifier>tests</classifier>
  <type>test-jar</type>
  <scope>test</scope>
</dependency>

```

This will expose api’s test classes to the test scope of api-2.0, and in particular the much wanted `TestHelper`. It is to be noted that api’s test-jar is already being built, see [here](https://github.com/openmrs/openmrs-module-attachments/blob/75b6dc2bdf4ff723a6a2cc9417e25801affe516e/api/pom.xml#L108-L119).

More on all this here ‘[How to create a jar containing test classes](https://maven.apache.org/plugins/maven-jar-plugin/examples/create-test-jar.html)’.

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [May 29, 2018, 6:36pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/63 "2018-05-29T18:36:12Z")

</div>

Thank you very much for your help . I will get rid of the additional TestHelper.🙂

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [May 30, 2018, 7:22am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/64 "2018-05-30T07:22:03Z")

</div>

Hi @mksd,

Inside the [AttachmentServiceTest](https://github.com/ridmal/openmrs-module-attachments/blob/27a3a5b8a1d8eeb354933ee93c2e83a8365484e6/api-2.0/src/test/java/org/openmrs/module/attachments/AttachmentsServiceTest.java#L26) class @dileka used

**Assert.assertArrayEquals** (expectedAttachments.stream().map(Attachment::getUuid).collect(Collectors.toList()).toArray(),actualAttachments.stream().map(Attachment::getUuid).collect(Collectors.toList()).toArray()); method

Instead of that can i use **Assert.assertEquals** (expectedAttachments.size(), actualAttachments.size()); ( Because sometimes Array values can be in different orders.)

---

<div class="post-metadata">

**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\
**Post date:** [May 30, 2018, 9:40am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/65 "2018-05-30T09:40:35Z")

</div>

Comparing the sizes is nowhere near as good as verifying that the contents match.

A correct design would be to ensure that the API returns attachments in an order that is anticipated. Such as ‘most recent first’, and _then_ arrange the expected array so that is respects that order ahead of the comparison.

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [May 30, 2018, 6:41pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/66 "2018-05-30T18:41:01Z")

</div>

Thank you for your suggestions. I updated the PR. It is only focus on

**saveComplexObs** (Encounter encounter, int count, int otherCount) method. And i add additional parameter ( **allComplexRelatedToAttach** , I just put this name .if it is not relevent please suggest a new name ) for method which can config the Complex obs creation for used non-attachment concepts and attachment concepts.

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [June 1, 2018, 6:24am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/67 "2018-06-01T06:24:18Z")

</div>

@mksd , For testing the fetch attachment based on the visit ,

1. Do i need to create a separate method in TestHelper ? or
2. Can I used the methods create for encounters **saveComplexObs** (Encounter encounter, int count, int otherCount) ? ( So like you mentioned earlier we can get all the encounters for particular visit and using above method we can fetch all the attachments )

---

<div class="post-metadata">

**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\
**Post date:** [June 1, 2018, 7:52am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/68 "2018-06-01T07:52:35Z")

</div>

I would _not_ add yet another method to `TestHelper`, I would just have a test setup that is a bit longer/more complex and that takes care of populating - say - two or three encounters of a given visit with `saveComplexObs(Encounter, int, int)`, with a slightly different configuration for each encounter.

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [June 4, 2018, 8:35am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/69 "2018-06-04T08:35:57Z")

</div>

Hi , @mksd I updated the PR. Please review and give me your feedback. 🙂

---

<div class="post-metadata">

**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\
**Post date:** [June 4, 2018, 10:25am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/70 "2018-06-04T10:25:22Z")

</div>

I just did.

I also would like you to write an exhaustive Javadoc for the API, so on the interface `AttachmentsService` (not `AttachmentsServiceImpl`). That’s where other developers will go to find the documentation as to what this API exactly does.

Also I just finally had a look at the implementation, so `AttachmentsServiceImpl`, and it’s great! Reusability 🙂 Very good.

Now here is what we’ve got, for a given patient:

- a method that can return attachments of a given encounter ;
- a method that can return attachments of a given visit ;
- a method that can return attachments of all encounters.

There is a fourth category of attachments that are not associated with any encounters (and hence any visits either). We need a way to fetch those as well, let’s introduce:

```java
List<Attachment> getIsolatedAttachments(Patient patient, boolean includeRetired);

```

(We might rename it, I’m not sure yet about the best terminology here.)

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [June 4, 2018, 11:57am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/71 "2018-06-04T11:57:18Z")

</div>

Thank you for your feed backs. I will start to work with above mentioned tasks. 🙂

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [June 5, 2018, 2:40pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/72 "2018-06-05T14:40:08Z")

</div>

Hi @mksd ,

I updated the PR with changes you mentioned. And can you give me a example about how to write a java doc for APIs in OpenMRS ( Is there any specific standard in OpenMRS.? )

---

<div class="post-metadata">

**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\
**Post date:** [June 5, 2018, 4:27pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/73 "2018-06-05T16:27:36Z")

</div>

Mmm, maybe have a look at [this](http://www.oracle.com/technetwork/java/javase/documentation/index-137868.html#format). Nothing specific to OpenMRS though, just enough information for an outsider to understand what an API is about:

- A (short) description of what the method does.
- A short description for each argument (using `@param`).
- A short description of what is returned (using `@return`).

Note that most IDEs (at least Eclipse and most likely IntelliJ) will pre-generate everything for you as soon as you type ‘slash star star ENTER’, so `/ **` +** ENTER**.

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [June 6, 2018, 6:45pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/74 "2018-06-06T18:45:16Z")

</div>

@mksd I added the Javadoc for the API and update the PR. 🙂

---

<div class="post-metadata">

**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\
**Post date:** [June 7, 2018, 10:42am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/75 "2018-06-07T10:42:37Z")

</div>

Just commented on your two last commits.

Thanks for the Javadoc, in the end we will keep it simpler than what you submitted. In general the OpenMRS code base is poorly Javadoc’d, so we won’t go over the top here. But believe me that sometimes you will pull your hair because the original developer didn’t take the necessary 3 min of his life to tell others what some API method is doing exactly.

For instance in the context of OpenMRS, when fetching data, a `Patient` parameter is quite obvious. So that’s a case where there is no need to detail why it’s there… etc.

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [June 8, 2018, 3:39am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/76 "2018-06-08T03:39:55Z")

</div>

Hi @mksd,

I was little bit stuck with getIsolatedAttachments(…) unit test implementation. I tried to create Attachments without configure any encounter and visit.

```
	// setup
	Patient patient = ps.getPatient(2);
	Concept concept = context.getConceptComplex(AttachmentsConstants.ContentFamily.OTHER);
	List<Attachment> expectedAttachments = new ArrayList<>();
	
	
	//
	for (int i = 0; i < 2; i++) {
		Obs obs = new Obs();
		obs.setConcept(concept);
		obs.setObsDatetime(new Date());
		obs.setPerson(patient);
		obs.setValueText("Some text value for a test obs.");
		obs = os.saveObs(obs, null);
		expectedAttachments.add(new Attachment(obs));
	}

```

But there was an error **Configuration required: attachments.defaultConceptComplexUuid**. I can’t find a way to solve this. Is there any other way to create obs without configuring encounter or visit ?

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [June 9, 2018, 4:31am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/77 "2018-06-09T04:31:39Z")

</div>

Hi @mksd

I solved the problem by using different approach and update the PR. Please review and give me your suggestions. 🙂

---

<div class="post-metadata">

**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\
**Post date:** [June 11, 2018, 9:13am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/78 "2018-06-11T09:13:04Z")

</div>

Thanks @ridmal, I just reviewed.

---

<div class="post-metadata">

**Author:** ![ridmal](https://talk.openmrs.org/user_avatar/talk.openmrs.org/ridmal/32/7063_2.png) [@ridmal](https://talk.openmrs.org/u/ridmal)\
**Post date:** [June 11, 2018, 1:25pm UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/79 "2018-06-11T13:25:00Z")

</div>

Thank you for you suggestions and modifications. I add those modifications and update the PR. 🙂

---

<div class="post-metadata">

**Author:** ![mksd](https://talk.openmrs.org/user_avatar/talk.openmrs.org/mksd/32/11729_2.png) [@mksd](https://talk.openmrs.org/u/mksd)\
**Post date:** [June 12, 2018, 4:51am UTC](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701/80 "2018-06-12T04:51:59Z")

</div>



[Previous page](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701.md?page=3)

[Next page](https://talk.openmrs.org/t/gscoc-2018-attachments-enhancements/17701.md?page=5)