Dt 740 #27

preetamnpr · 2023-12-06T12:16:30Z

No description provided.

booking/src/main/java/org/dcsa/conformance/standards/booking/BookingScenarioListBuilder.java

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java

booking/src/main/java/org/dcsa/conformance/standards/booking/BookingScenarioListBuilder.java

nt-gt · 2023-12-13T12:13:16Z

...rg/dcsa/conformance/standards/booking/action/UC12_Carrier_ConfirmBookingCompletedAction.java

-              BookingState.COMPLETED
+              BookingState.COMPLETED,
+              dsp.amendedBookingStatus() != null ? BookingState.AMENDMENT_CONFIRMED : null
            ),


This does not look correct. It should be possible to end with a AMENDEDMENT that was declined/cancelled when reaching the UC12_Carrier_ConfirmBookingCompletedAction

Because we are not calling UC12 for amendment Declined or cancelled. https://miro.com/app/board/uXjVND1csI8=/

If a confirmed booking never has a grey state, then having line 53 is incorrect.
If a confirmed booking can have a grey state, then it could have any of the grey states AMENDMENT CONFIRMED, AMENDMENT DECLINED or AMENDMENT CANCELLED.

Either way, the code is inconsistent by saying that if there is an amended status, it must be AMENDMENT CONFIRMED.

For me, this one is still "unanswered". I suspect it is because the scenarios do not cover the case of (as an example): ... -> UC5 -> UC7 -> UC9 -> UC12

We call the UC8A and then go to UC12.. Hence I put that AmendedBookingStatus check.

I get ur point. The amendedBookingStatus shouldnt be checked in the UC12. 👍 because the flow of confirmed amendment itself complete after UC8A . am I right ?

Personally, I would expect the amendedBookingStatus in UC12 should match the value of dsp.amendedBookingStatus().

According to the miro board, UC12 shouldnt be having any AMENDMENT status.

Please verify that assertion with Flavia. Sometimes the states can inherit the amendment/updated state even though it is not visible in the Miro board. (In eBL, after UC4a, the RECEIVED state is actually RECEIVED + UPDATE CONFIRMED even though the UPDATE CONFIRMED is not visible).

When discussing the scenario with Flavia, be sure to show an exceptional case where you could end up in UC12 with an amended state.

Example: ... -> UC5 -> UC7 -> UC8a -> UC12

I hope she agrees with you, because then we can just "reset" the amendment state + content when doing UC12 and simplify the check here. But as said, please confirm with the PO.

As discussed with internally, at UC12 we dont need to check the final AmendmentStatus for Booking.

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java

booking/src/main/java/org/dcsa/conformance/standards/booking/party/Carrier.java

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java

nt-gt · 2023-12-14T10:47:31Z

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java

  private static final Set<BookingState> MAY_UPDATE_REQUEST_STATES = Set.of(
    RECEIVED,
    PENDING_UPDATE,
-    PENDING_UPDATE_CONFIRMATION
+    UPDATE_RECEIVED
  );



This is mixing original and amended states but it is used only with original states in putBooking, which makes me suspect the logic is not correctly updated.

Is NOT_APPLICABLE_FOR_SIMPLE_STATE_CHANGE and MAY_UPDATE_REQUEST_STATES still used? If not, please remove them.

If they are, then that code is probably still broken because of how we do state checks now.

I removed NOT_APPLICABLE_FOR_SIMPLE_STATE_CHANGE.
MAY_UPDATE_REQUEST_STATES is still being used in PutBooking.. Hence I didnt touch it

Ok, but the use of MAY_UPDATE_REQUEST_STATES in putBooking is from the time where getBookingState() could return the amended or the original booking. Given getBookingState() (now getOriginalBookingState()) always returns the original booking state, the state in putBooking must be updated to align with the new semantics.

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java

nt-gt · 2023-12-14T10:53:11Z

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java

@@ -123,7 +130,9 @@ public void confirmBooking(String reference, Supplier<String> cbrGenerator, Stri
    setReason(reason);
  }

-
+  public void resetAmendedBookingState() {


This method should be private as it is not intended for external usage.

preetamnpr · 2023-12-15T08:58:13Z

Conformance Checks pass

nt-gt · 2023-12-15T08:59:40Z

booking/src/main/java/org/dcsa/conformance/standards/booking/party/Carrier.java

+    String cbrr = actionPrompt.get("cbrr").asText();
    String cbr = actionPrompt.get("cbr").asText();


(for a future change): The actionPrompt.get should be actiopnPrompt.required (generally anywhere you see them).

I have begun replacing them in the eBL code. But as said, do it in a later commit

nt-gt · 2023-12-15T09:05:38Z

booking/src/main/java/org/dcsa/conformance/standards/booking/BookingScenarioListBuilder.java

+      case AMENDMENT_CONFIRMED -> then(
+        shipper_GetBooking(CONFIRMED,bookingState)
+          .thenEither(
+            uc12_carrier_confirmBookingCompleted().thenHappyPathFrom(COMPLETED),


I would replace this line with:

Suggested change

uc12_carrier_confirmBookingCompleted().thenHappyPathFrom(COMPLETED),

noAction().thenHappyPathFrom(CONFIRMED),

The noAction() is a stepping stone for enabling the .thenHappyPathFrom call.

nt-gt · 2023-12-15T09:06:21Z

booking/src/main/java/org/dcsa/conformance/standards/booking/BookingScenarioListBuilder.java

+      case AMENDMENT_CANCELLED, AMENDMENT_DECLINED -> then(
+        shipper_GetBooking(originalBookingState,bookingState)
+          .then(
+            uc6_carrier_requestUpdateToConfirmedBooking().thenHappyPathFrom(PENDING_AMENDMENT)
+          )
+      );
    };


From AMENDMENT_CANCELLED and AMENDMENT_DECLINED the possible actions depend on the original booking state.

I think this case can be simplified into:

Suggested change

case AMENDMENT_CANCELLED, AMENDMENT_DECLINED -> then(

shipper_GetBooking(originalBookingState,bookingState)

.then(

uc6_carrier_requestUpdateToConfirmedBooking().thenHappyPathFrom(PENDING_AMENDMENT)

)

);

};

case AMENDMENT_CANCELLED, AMENDMENT_DECLINED -> then(

shipper_GetBooking(originalBookingState,bookingState)

.thenHappyPathFrom(originalBookingState));

nt-gt · 2023-12-18T10:00:24Z

booking/src/main/java/org/dcsa/conformance/standards/booking/BookingScenarioListBuilder.java

-            uc12_carrier_confirmBookingCompleted().thenHappyPathFrom(COMPLETED),
-            uc6_carrier_requestUpdateToConfirmedBooking().thenHappyPathFrom(PENDING_AMENDMENT)
+            noAction().thenHappyPathFrom(CONFIRMED),
+            noAction().thenHappyPathFrom(PENDING_AMENDMENT)


Logically, this looks weird. Code wise, this looks like we are in two different states at the same time.

Line 104 asserts we are in CONFIRMED so there for the thenHappyPathFrom(PENDING_AMENDMENT) does not fit the bill. Either line 104 is not correct or line 107 is misleading.

Because the State of the booking is in two states CONFIRMED and PENDING_AMENDMENT

For future reference: As far as this state flow generation works, it is in state PENDING_AMENDMENT (for when computing future states)

As an example, it does not make sense for the booking to be completed while it is in PENDING_AMENDMENT. The PENDING_AMENDMENT state means the carrier cannot complete the booking in its current form. This means that the complete booking action is too forgiving. It should not allow a booking in PENDING_AMENDMENT to complete.

preetamnpr added 2 commits December 5, 2023 18:18

Dt-740 first set changes

6838f00

DT-716

0d9eb00

preetamnpr requested review from nt-gt and gj0dcsa December 6, 2023 12:16

nt-gt reviewed Dec 6, 2023

View reviewed changes

booking/src/main/java/org/dcsa/conformance/standards/booking/BookingScenarioListBuilder.java Outdated Show resolved Hide resolved

nt-gt reviewed Dec 6, 2023

View reviewed changes

booking/src/main/java/org/dcsa/conformance/standards/booking/BookingScenarioListBuilder.java Outdated Show resolved Hide resolved

Dt-740

3f7c40e

nt-gt reviewed Dec 7, 2023

View reviewed changes

booking/src/main/java/org/dcsa/conformance/standards/booking/BookingScenarioListBuilder.java Outdated Show resolved Hide resolved

Dt-740

5c58271

nt-gt reviewed Dec 8, 2023

View reviewed changes

booking/src/main/java/org/dcsa/conformance/standards/booking/BookingScenarioListBuilder.java Outdated Show resolved Hide resolved

preetamnpr added 3 commits December 13, 2023 09:42

merged dev

928ff07

DT-740 and DT-737

ae5a6e4

DT-740 this change was missed during the merge from test to dev.

dd30c31

nt-gt reviewed Dec 13, 2023

View reviewed changes

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java Outdated Show resolved Hide resolved

nt-gt reviewed Dec 13, 2023

View reviewed changes

booking/src/main/java/org/dcsa/conformance/standards/booking/BookingScenarioListBuilder.java Outdated Show resolved Hide resolved

nt-gt reviewed Dec 13, 2023

View reviewed changes

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java Outdated Show resolved Hide resolved

nt-gt reviewed Dec 13, 2023

View reviewed changes

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java Outdated Show resolved Hide resolved

nt-gt reviewed Dec 13, 2023

View reviewed changes

booking/src/main/java/org/dcsa/conformance/standards/booking/party/Carrier.java Outdated Show resolved Hide resolved

preetamnpr added 2 commits December 14, 2023 11:30

Dt-740 Changes after the review

8d93f87

Merge branch 'dev' into DT-740

17bf9db

nt-gt reviewed Dec 14, 2023

View reviewed changes

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java Outdated Show resolved Hide resolved

nt-gt reviewed Dec 14, 2023

View reviewed changes

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java Outdated Show resolved Hide resolved

nt-gt reviewed Dec 14, 2023

View reviewed changes

...ng/src/main/java/org/dcsa/conformance/standards/booking/model/PersistableCarrierBooking.java Outdated Show resolved Hide resolved

nt-gt reviewed Dec 14, 2023

View reviewed changes

preetamnpr added 2 commits December 15, 2023 09:55

DT-740

7ff3153

DT-740 removed unused constant

dd0b4e2

nt-gt reviewed Dec 15, 2023

View reviewed changes

DT-740

7e55896

nt-gt reviewed Dec 18, 2023

View reviewed changes

preetamnpr merged commit d409e2c into dev Dec 18, 2023
1 check passed

nt-gt deleted the DT-740 branch December 18, 2023 10:07

nt-gt mentioned this pull request Dec 18, 2023

DT-740 removed PENDING_AMENDMENT from AMENDMENT_CONFIRMED. #41

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dt 740 #27

Dt 740 #27

preetamnpr commented Dec 6, 2023

nt-gt Dec 13, 2023

preetamnpr Dec 13, 2023

nt-gt Dec 13, 2023

nt-gt Dec 15, 2023

preetamnpr Dec 15, 2023

preetamnpr Dec 15, 2023

nt-gt Dec 15, 2023

preetamnpr Dec 15, 2023

nt-gt Dec 15, 2023 •

edited

Loading

preetamnpr Dec 15, 2023

nt-gt Dec 14, 2023

nt-gt Dec 15, 2023

preetamnpr Dec 15, 2023

nt-gt Dec 15, 2023

nt-gt Dec 14, 2023

preetamnpr commented Dec 15, 2023

nt-gt Dec 15, 2023

nt-gt Dec 15, 2023

nt-gt Dec 15, 2023 •

edited

Loading

nt-gt Dec 18, 2023

preetamnpr Dec 18, 2023 •

edited

Loading

nt-gt Dec 18, 2023

		String cbrr = actionPrompt.get("cbrr").asText();
		String cbr = actionPrompt.get("cbr").asText();

	uc12_carrier_confirmBookingCompleted().thenHappyPathFrom(COMPLETED),
	noAction().thenHappyPathFrom(CONFIRMED),

Dt 740 #27

Dt 740 #27

Conversation

preetamnpr commented Dec 6, 2023

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

nt-gt Dec 15, 2023 • edited Loading

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

preetamnpr commented Dec 15, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nt-gt Dec 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

preetamnpr Dec 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nt-gt Dec 15, 2023 •

edited

Loading

nt-gt Dec 15, 2023 •

edited

Loading

preetamnpr Dec 18, 2023 •

edited

Loading