Nikodem Graczewski

I don't think this tests anything in the SUT.

I don't think this tests anything in the SUT.

Can we format those in a more readable way?

Can we format those in a more readable way?

OLMIS-6298: Refactored unit tests
OLMIS-6298: Refactored unit tests
OLMIS-6297: Refactored unit tests
OLMIS-6297: Refactored unit tests
OLMIS-6271: Refactored unit tests (part 2)
OLMIS-6271: Refactored unit tests (part 2)
According to the DRY principle, we should not be duplicating code, especially that much. 1. Instead of adding another data builder we could add a buildDto method which would abstract the exporting ...

According to the DRY principle, we should not be duplicating code, especially that much.
1. Instead of adding another data builder we could add a buildDto method which would abstract the exporting part. This way we have both fast and easily used builders as well as no code duplication.
2. So can be using the DataBuilder pattern and duplicating such a big chunk of the code. As long as the complexity is low, and it is the same for a separate builder and using the exporter pattern, we should be good.
3. Same is true using the exporter pattern.

OLMIS-6261: Refactored unit tests (part 1)
OLMIS-6261: Refactored unit tests (part 1)
This feels like a duplication of the RequisitionLineItemDataBuilder. Could we leverage the exporter pattern somehow? Same applies for similar DTO builders.

This feels like a duplication of the RequisitionLineItemDataBuilder. Could we leverage the exporter pattern somehow? Same applies for similar DTO builders.

Probably would be cool to propose it on some other channel. Dev forum perhaps?

Probably would be cool to propose it on some other channel. Dev forum perhaps?

Paweł Cieszko Why was the comment about adding DataBuilder interface removed? I think it is something worth discussing.

Paweł Cieszko Why was the comment about adding DataBuilder interface removed? I think it is something worth discussing.

As I said, it's our convention for creating objects without ID. The "build" methods return objects with ID. Since these are integration tests, I assume those objects are used for populating the dat...

As I said, it's our convention for creating objects without ID. The "build" methods return objects with ID. Since these are integration tests, I assume those objects are used for populating the database with the test data and thus can't have an ID.

Here's an example:
https://github.com/OpenLMIS/openlmis-referencedata/blob/master/src/test/java/org/openlmis/referencedata/testbuilder/GeographicZoneDataBuilder.java#L71

If I remember correctly, it's our convention for creating an object without ID (which is required when saving a new object in the repository).

If I remember correctly, it's our convention for creating an object without ID (which is required when saving a new object in the repository).

OLMIS-6258: Added PendingNotificationDataBuilder
OLMIS-6258: Added PendingNotificationDataBuilder
OLMIS-5390: Rewrote LocalDatabase tests to be synchronous
OLMIS-5390: Rewrote LocalDatabase tests to be synchronous
OLMIS-6234: Fixed issue with error indicator not working on the non full supply tab of the requisition...
OLMIS-6234: Fixed issue with error indicator not working on the non full supply tab of the requisition...
Then we can remove it here.

Then we can remove it here.

OLMIS-6090: Fixed convert to order endpoint for facilities with multiple program handled by the same...
OLMIS-6090: Fixed convert to order endpoint for facilities with multiple program handled by the same...
Do we need this inject?

Do we need this inject?