Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Dt 740 #27
Dt 740 #27
Changes from 2 commits
6838f00
0d9eb00
3f7c40e
5c58271
928ff07
ae5a6e4
dd30c31
8d93f87
17bf9db
7ff3153
dd0b4e2
7e55896
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would replace this line with:
The noAction() is a stepping stone for enabling the
.thenHappyPathFrom
call.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does not look correct. It should be possible to end with a AMENDEDMENT that was declined/cancelled when reaching the
UC12_Carrier_ConfirmBookingCompletedAction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because we are not calling UC12 for amendment Declined or cancelled. https://miro.com/app/board/uXjVND1csI8=/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
orAMENDMENT CANCELLED
.Either way, the code is inconsistent by saying that if there is an amended status, it must be
AMENDMENT CONFIRMED
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We call the UC8A and then go to UC12.. Hence I put that AmendedBookingStatus check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I would expect the
amendedBookingStatus
in UC12 should match the value ofdsp.amendedBookingStatus()
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to the miro board, UC12 shouldnt be having any AMENDMENT status.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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
, theRECEIVED
state is actuallyRECEIVED
+UPDATE CONFIRMED
even though theUPDATE 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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed with internally, at UC12 we dont need to check the final AmendmentStatus for Booking.