GreenCityUBS bug in saving events #975

LiliaMokhnatska · 2023-01-29T22:22:19Z

Green City UBS bug in writing order history PR

Summary Of Issue :

Create a new order. Change the confirmed number of bags. Open order history. The changed order details show that the quantity has been changed from 0 to "Підтверджена к-ть" .

It is expected that the changed order details show that the quantity has been changed from "Планована к-ть" to "Підтверджена к-ть"

Issue Link :

ita-social-projects/GreenCity#5058

Summary Of Changes :

add getAmount method in UpdateOrderDetailRepository
change method collectEventAboutSetOrderDetails in UBSManagementServiceImpl
add test for new lines of code

Code reviewers

@github_username

Added

add getAmount method in UpdateOrderDetailRepository
add test for new lines of code

Changed

change method collectEventAboutSetOrderDetails in UBSManagementServiceImpl

Deleted

How to test :

User need to be logged in http://localhost:8060/swagger-ui.html#/own-security-controller
Register Token
Create an order https://www.testgreencity.ga/GreenCityClient/#/ubs/order
Admin need to be logged in http://localhost:8060/swagger-ui.html#/own-security-controller
Register Token
Choose "Деталі замовлення" https://www.testgreencity.ga/GreenCityClient/#/ubs-admin/order/1116
Choose "Підтверджена к-ть" change and submit
Choose "Історія замовлення"

CHECK LIST

Code is up-to-date with the dev branch.
You've successfully built and run the tests locally.
There are new or updated unit tests validating the change.
JIRA/ Github Issue number & title in PR title (ISSUE-#5058: Ticket title)
This template filled (above this section).
Sonar's report does not contain bugs, vulnerabilities, security issues, code smells ar duplication
NEED_REVIEW and READY_FOR_REVIEW labels are added.
All files reviewed before sending to reviewers

RomaMocherniuk · 2023-01-30T10:59:24Z

service/src/test/java/greencity/service/ubs/UBSManagementServiceImplTest.java

@@ -981,6 +981,7 @@ void testSetOrderDetailIfPaidAndPriceLessThanPaidSum() {
        verify(orderRepository).updateOrderPointsToUse(1L, 0);
    }

+    //


why do we need it?

I need to delete it, I forgot

RomaMocherniuk · 2023-01-30T11:01:13Z

service/src/main/java/greencity/service/ubs/UBSManagementServiceImpl.java

                }
                if (entry.getValue().longValue() != confirmWasteWas.orElse(0L)) {
                    if (countOfChanges == 0) {
                        values.append(OrderHistory.CHANGE_ORDER_DETAILS + " ");
                    }
                    values.append(bag.getName()).append(" ").append(capacity).append(" л: ")
-                        .append(confirmWasteWas.orElse(0L))
+                        .append(confirmWasteWas.orElse(startAmount.orElse(0L)))
                        .append(" шт на ").append(entry.getValue()).append(" шт.");


should this text be localized?

Yes, it should be localized. Here is a link to this issue: ita-social-projects/GreenCity#5117

RomaMocherniuk · 2023-01-30T11:02:48Z

service/src/main/java/greencity/service/ubs/UBSManagementServiceImpl.java

            if (bagOptional.isPresent() && checkOrderStatusAboutConfirmWaste(order)) {
                Optional<Long> confirmWasteWas = Optional.empty();
+                Optional<Long> startAmount = Optional.empty();


I guess it is better to use initial instead of start

RomaMocherniuk · 2023-01-30T11:03:45Z

service/src/main/java/greencity/service/ubs/UBSManagementServiceImpl.java

                Bag bag = bagOptional.get();
                if (Boolean.TRUE.equals(updateOrderRepository.ifRecordExist(orderId, entry.getKey().longValue()) > 0)) {
                    confirmWasteWas =
                        Optional.ofNullable(updateOrderRepository.getConfirmWaste(orderId, entry.getKey().longValue()));
+                    startAmount =
+                        Optional.ofNullable(updateOrderRepository.getAmount(orderId, entry.getKey().longValue()));
                }
                if (entry.getValue().longValue() != confirmWasteWas.orElse(0L)) {
                    if (countOfChanges == 0) {
                        values.append(OrderHistory.CHANGE_ORDER_DETAILS + " ");
                    }
                    values.append(bag.getName()).append(" ").append(capacity).append(" л: ")


same here
should this text be localized?

Yes, it will be solved in the next issue: ita-social-projects/GreenCity#5117

RomaMocherniuk · 2023-01-30T11:05:41Z

service/src/test/java/greencity/service/ubs/UBSManagementServiceImplTest.java

+        when(orderRepository.getOrderDetails(anyLong()))
+            .thenReturn(Optional.ofNullable(ModelUtils.getOrdersStatusFormedDto()));
+        when(bagRepository.findById(1)).thenReturn(Optional.of(ModelUtils.getTariffBag()));
+        when(bagRepository.findById(1)).thenReturn(Optional.of(ModelUtils.getTariffBag()));


duplication

RomaMocherniuk · 2023-01-30T11:12:50Z

dao/src/main/java/greencity/repository/UpdateOrderDetailRepository.java

+     */
+    @Query(value = "SELECT AMOUNT FROM ORDER_BAG_MAPPING "
+        + "WHERE ORDER_ID = :orderId AND BAG_ID = :bagId", nativeQuery = true)
+    Long getAmount(Long orderId, Long bagId);


can we use JPA naming here and not use nativeQuery?

https://bushansirgur.in/spring-data-jpa-finder-query-methods-by-multiple-field-names-with-examples/

Something similar like this
https://github.com/ita-social-projects/GreenCityUBS/blob/dev/dao/src/main/java/greencity/repository/BagRepository.java#L90

Ok. I'll change it

RomaMocherniuk · 2023-01-30T11:16:56Z

service/src/test/java/greencity/service/ubs/UBSManagementServiceImplTest.java

@@ -1000,6 +1001,27 @@ void testSetOrderDetailConfirmed() {
        verify(updateOrderRepository).updateConfirm(anyInt(), anyLong(), anyLong());
    }

+    @Test
+    void testSetOrderDetailConfirmed2() {
+        when(orderRepository.findById(1L)).thenReturn(Optional.ofNullable(ModelUtils.getOrdersStatusConfirmedDto()));


RomaMocherniuk · 2023-01-30T11:17:10Z

service/src/test/java/greencity/service/ubs/UBSManagementServiceImplTest.java

+    @Test
+    void testSetOrderDetailConfirmed2() {
+        when(orderRepository.findById(1L)).thenReturn(Optional.ofNullable(ModelUtils.getOrdersStatusConfirmedDto()));
+        when(bagRepository.findCapacityById(1)).thenReturn(1);


add verify

RomaMocherniuk · 2023-01-30T11:17:42Z

service/src/test/java/greencity/service/ubs/UBSManagementServiceImplTest.java

+        doNothing().when(updateOrderRepository).updateConfirm(anyInt(), anyLong(), anyLong());
+        when(orderRepository.getOrderDetails(anyLong()))
+            .thenReturn(Optional.ofNullable(ModelUtils.getOrdersStatusFormedDto()));
+        when(bagRepository.findById(1)).thenReturn(Optional.of(ModelUtils.getTariffBag()));


add verify

RomaMocherniuk · 2023-01-30T11:17:55Z

service/src/test/java/greencity/service/ubs/UBSManagementServiceImplTest.java

+            .thenReturn(Optional.ofNullable(ModelUtils.getOrdersStatusFormedDto()));
+        when(bagRepository.findById(1)).thenReturn(Optional.of(ModelUtils.getTariffBag()));
+        when(bagRepository.findById(1)).thenReturn(Optional.of(ModelUtils.getTariffBag()));
+        when(updateOrderRepository.ifRecordExist(any(), any())).thenReturn(1L);


add verify

RomaMocherniuk · 2023-01-30T15:16:23Z

service/src/main/java/greencity/service/ubs/UBSManagementServiceImpl.java

                Bag bag = bagOptional.get();
                if (Boolean.TRUE.equals(updateOrderRepository.ifRecordExist(orderId, entry.getKey().longValue()) > 0)) {
                    confirmWasteWas =
                        Optional.ofNullable(updateOrderRepository.getConfirmWaste(orderId, entry.getKey().longValue()));
+                    initialAmount =
+                        Optional.ofNullable(updateOrderRepository.getAmount(orderId, entry.getKey().longValue()));


It seems like the UpdateOrderRepository class should be renamed because it not only updates orders

Yes, I renamed this interface and variables accordingly.

add getAmount in UpdateOrderDetailRepository

…ository change name of variables updateOrderRepository on orderDetailRepository

sonarqubecloud · 2023-01-31T13:42:12Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

90.6% Coverage
0.0% Duplication

LiliaMokhnatska changed the title ~~change method collectEventAboutSetOrderDetails~~ GreenCityUBS bug in saving events Jan 30, 2023

LiliaMokhnatska added the review label Jan 30, 2023

RomaMocherniuk reviewed Jan 30, 2023

View reviewed changes

LiliaMokhnatska requested review from ABbondar, JJJazl, MaksymGolik, NKorzhik and OlegVat January 30, 2023 14:54

RomaMocherniuk reviewed Jan 30, 2023

View reviewed changes

RomaMocherniuk approved these changes Jan 31, 2023

View reviewed changes

JJJazl approved these changes Jan 31, 2023

View reviewed changes

LiliaMokhnatska added 4 commits January 31, 2023 15:36

change method collectEventAboutSetOrderDetails

8c4368b

add getAmount in UpdateOrderDetailRepository

add Test to cover new line

c233c39

changes according to comments

c8f3157

changes name of Interface UpdateOderDetailRepository on OderDetailRep…

567d0fb

…ository change name of variables updateOrderRepository on orderDetailRepository

LiliaMokhnatska force-pushed the UBS_bug_history_order branch from 9fa1c1b to 567d0fb Compare January 31, 2023 13:37

MaksymGolik approved these changes Jan 31, 2023

View reviewed changes

LiliaMokhnatska requested review from jpgcz and juseti January 31, 2023 15:42

juseti approved these changes Jan 31, 2023

View reviewed changes

LiliaMokhnatska merged commit e934675 into dev Jan 31, 2023

LiliaMokhnatska deleted the UBS_bug_history_order branch January 31, 2023 20:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GreenCityUBS bug in saving events #975

GreenCityUBS bug in saving events #975

LiliaMokhnatska commented Jan 29, 2023 •

edited

Loading

RomaMocherniuk Jan 30, 2023

LiliaMokhnatska Jan 30, 2023

RomaMocherniuk Jan 30, 2023

LiliaMokhnatska Jan 31, 2023

RomaMocherniuk Jan 30, 2023

LiliaMokhnatska Jan 30, 2023

RomaMocherniuk Jan 30, 2023

LiliaMokhnatska Jan 31, 2023

RomaMocherniuk Jan 30, 2023

RomaMocherniuk Jan 30, 2023

RomaMocherniuk Jan 30, 2023

RomaMocherniuk Jan 30, 2023

LiliaMokhnatska Jan 30, 2023

RomaMocherniuk Jan 30, 2023

LiliaMokhnatska Jan 30, 2023

RomaMocherniuk Jan 30, 2023

LiliaMokhnatska Jan 30, 2023

RomaMocherniuk Jan 30, 2023

LiliaMokhnatska Jan 30, 2023

RomaMocherniuk Jan 30, 2023

LiliaMokhnatska Jan 30, 2023

RomaMocherniuk Jan 30, 2023

LiliaMokhnatska Jan 31, 2023

sonarqubecloud bot commented Jan 31, 2023

GreenCityUBS bug in saving events #975

GreenCityUBS bug in saving events #975

Conversation

LiliaMokhnatska commented Jan 29, 2023 • edited Loading

Green City UBS bug in writing order history PR

Summary Of Issue :

Issue Link :

Summary Of Changes :

Code reviewers

Added

Changed

Deleted

How to test :

CHECK LIST

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Jan 31, 2023

LiliaMokhnatska commented Jan 29, 2023 •

edited

Loading