Skip to content
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-718 changes for reefer #30

Merged
merged 2 commits into from
Dec 11, 2023
Merged

Dt-718 changes for reefer #30

merged 2 commits into from
Dec 11, 2023

Conversation

preetamnpr
Copy link
Contributor

No description provided.

@preetamnpr preetamnpr requested review from nt-gt and gj0dcsa December 8, 2023 23:17
@@ -237,22 +248,25 @@ private static BookingScenarioListBuilder shipper_GetAmendedBooking404() {
(BookingAction) previousAction));
}

private static BookingScenarioListBuilder uc1_shipper_SubmitBookingRequest() {
private static BookingScenarioListBuilder uc1_shipper_SubmitBookingRequest(String bookingVariant) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you prefer a single value instead of a mix of two booleans (isReeferBooking and isDGBooking), then judging by the way you use this value perhaps an enum BookingVariang { REGULAR, REEFER, DG } would help?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather use the Enums than the Booleans

@preetamnpr preetamnpr merged commit f300764 into dev Dec 11, 2023
1 check passed
@nt-gt nt-gt deleted the DT-718 branch December 11, 2023 12:12
}

@Override
public ObjectNode asJsonNode() {
ObjectNode jsonNode = super.asJsonNode();
jsonNode.set("csp", getCspSupplier().get().toJson());
jsonNode.set("bookingVariant", new TextNode(bookingVariant.getValue()));
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use jsonNode.put in this case rather than manually constructing a text node.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

jsonNode.put("csp", getCspSupplier().get().toJson()); is deprecated. hence used .set for csp and bookingVariant. If you would like I will change it .

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was not challenging the use set for the csp. I was challenging the use of set for a regular string for which .put is a much better (and not deprecated) option.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok.. I will change it.. done !

JsonNode jsonRequestBody =
JsonToolkit.templateFileToJsonNode(
"/standards/booking/messages/booking-api-v20-request.json",
String fileSuffix = bookingVariant.equals("reefer") ? FILE_SUFFIX_REEFER: "";
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would probably put this logic into the enum as each case has its own payload in the booking case.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For simplicity, you could rename the regular one so they all have suffixes and use bookingVariant.toLower() to determine the suffix (probably how I would have done it) but other approaches work too (where the enum has a field for this purpose, etc.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants