Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -39,13 +39,29 @@ public List<ActiveAddressVm> getUserAddressList() {
List<AddressDetailVm> addressVmList = locationService.getAddressesByIdList(
userAddressList.stream().map(UserAddress::getAddressId).collect(Collectors.toList()));

List<ActiveAddressVm> addressActiveVms = userAddressList.stream().flatMap(userAddress -> addressVmList.stream()
.filter(addressDetailVm -> userAddress.getAddressId().equals(addressDetailVm.id())).map(
addressDetailVm -> new ActiveAddressVm(addressDetailVm.id(), addressDetailVm.contactName(),
addressDetailVm.phone(), addressDetailVm.addressLine1(), addressDetailVm.city(),
addressDetailVm.zipCode(), addressDetailVm.districtId(), addressDetailVm.districtName(),
addressDetailVm.stateOrProvinceId(), addressDetailVm.stateOrProvinceName(),
addressDetailVm.countryId(), addressDetailVm.countryName(), userAddress.getIsActive())))
List<ActiveAddressVm> addressActiveVms = userAddressList.stream()
.flatMap(userAddress -> addressVmList.stream()
.filter(addressDetailVm -> userAddress.getAddressId().equals(addressDetailVm.id()))
.map(
addressDetailVm -> ActiveAddressVm.builder()
.id(addressDetailVm.id())
.contactName(addressDetailVm.contactName())
.phone(addressDetailVm.phone())
.addressLine1(addressDetailVm.addressLine1())
.city(addressDetailVm.city())
.zipCode(addressDetailVm.zipCode())
.districtId(addressDetailVm.districtId())
.districtName(addressDetailVm.districtName())
.stateOrProvinceId(addressDetailVm.stateOrProvinceId())
.stateOrProvinceName(addressDetailVm.stateOrProvinceName())
.stateOrProvinceCode(addressDetailVm.stateOrProvinceCode())
.countryId(addressDetailVm.countryId())
.countryName(addressDetailVm.countryName())
.countryCode2(addressDetailVm.countryCode2())
.countryCode3(addressDetailVm.countryCode3())
.isActive(userAddress.getIsActive())
.build()
))
.toList();

//sort by isActive
Expand Down Expand Up @@ -97,4 +113,4 @@ public void chooseDefaultAddress(Long id) {
}
userAddressRepository.saveAll(userAddressList);
}
}
}
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
package com.yas.customer.viewmodel.address;

import lombok.Builder;

@Builder(toBuilder = true)
public record ActiveAddressVm(
Long id,
String contactName,
Expand All @@ -11,9 +14,12 @@ public record ActiveAddressVm(
String districtName,
Long stateOrProvinceId,
String stateOrProvinceName,
String stateOrProvinceCode,
Long countryId,
String countryName,
String countryCode2,
String countryCode3,
Boolean isActive
) {

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,20 @@

import lombok.Builder;

@Builder
public record AddressDetailVm(Long id, String contactName, String phone,
String addressLine1, String city, String zipCode,
Long districtId, String districtName, Long stateOrProvinceId,
String stateOrProvinceName, Long countryId, String countryName) {
@Builder(toBuilder = true)
public record AddressDetailVm(Long id,
String contactName,
String phone,
String addressLine1,
String city,
Comment on lines +9 to +10

Choose a reason for hiding this comment

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

Missing addressLine2 field in record

The AddressDetailVm record is missing the addressLine2 field, which is included in the data returned by the locationService.getAddressById method. This will cause deserialization failures when the customer module receives AddressDetailVm instances from the location service that include this field. Add String addressLine2 after addressLine1 to match the expected data structure.

Code suggestion
Check the AI-generated fix before applying
Suggested change
String addressLine1,
String city,
String addressLine1,
String addressLine2,
String city,

Code Review Run #f96c13


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

String zipCode,
Long districtId,
String districtName,
Long stateOrProvinceId,
String stateOrProvinceName,
String stateOrProvinceCode,
Long countryId,
String countryName,
String countryCode2,
String countryCode3) {
}
Original file line number Diff line number Diff line change
Expand Up @@ -106,36 +106,42 @@ void testChooseDefaultAddress_whenNormalCase_responseOk() throws Exception {

private List<ActiveAddressVm> getActiveAddressVms() {
return List.of(
new ActiveAddressVm(
1L,
"John Doe",
"123-456-7890",
"123 Elm Street",
"Springfield",
"12345",
101L,
"District A",
10L,
"State A",
1L,
"Country A",
true
),
new ActiveAddressVm(
2L,
"Jane Smith",
"987-654-3210",
"456 Oak Avenue",
"Springfield",
"67890",
102L,
"District B",
11L,
"State B",
2L,
"Country B",
false
)
ActiveAddressVm.builder()
.id(1L)
.contactName("John Doe")
.phone("123-456-7890")
.addressLine1("123 Elm Street")
.city("Springfield")
.zipCode("12345")
.districtId(101L)
.districtName("District A")
.stateOrProvinceId(10L)
.stateOrProvinceName("State A")
.stateOrProvinceCode("ST")
.countryId(1L)
.countryName("Country A")
.countryCode2("CA")
.countryCode3("CAN")
.isActive(true)
.build(),
ActiveAddressVm.builder()
.id(2L)
.contactName("Jane Smith")
.phone("987-654-3210")
.addressLine1("456 Oak Avenue")
.city("Springfield")
.zipCode("67890")
.districtId(102L)
.districtName("District B")
.stateOrProvinceId(11L)
.stateOrProvinceName("State B")
.stateOrProvinceCode("SB")
.countryId(2L)
.countryName("Country B")
.countryCode2("CB")
.countryCode3("CBB")
.isActive(false)
.build()
);

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,20 +126,20 @@ void testCreateAddress() {
}

private AddressDetailVm getAddressDetailVm() {
return new AddressDetailVm(
1L,
"John Doe",
"+1234567890",
"123 Elm Street",
"Springfield",
"62701",
101L,
"Downtown",
201L,
"Illinois",
301L,
"United States"
);
return AddressDetailVm.builder()
.id(1L)
.contactName("John Doe")
.phone("+1234567890")
.addressLine1("123 Elm Street")
.city("Springfield")
.zipCode("62701")
.districtId(101L)
.districtName("Downtown")
.stateOrProvinceId(201L)
.stateOrProvinceName("Illinois")
.countryId(301L)
.countryName("United States")
.build();
}

private AddressPostVm getAddressPostVm() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,23 @@
import com.yas.location.model.Address;
import lombok.Builder;

@Builder
public record AddressDetailVm(Long id, String contactName, String phone,
String addressLine1, String addressLine2, String city, String zipCode,
Long districtId, String districtName, Long stateOrProvinceId,
String stateOrProvinceName, Long countryId, String countryName) {
@Builder(toBuilder = true)
public record AddressDetailVm(Long id,
String contactName,
String phone,
String addressLine1,
String addressLine2,
String city,
String zipCode,
Long districtId,
String districtName,
Long stateOrProvinceId,
String stateOrProvinceName,
String stateOrProvinceCode,
Long countryId,
String countryName,
String countryCode2,
String countryCode3) {
public static AddressDetailVm fromModel(Address address) {
return AddressDetailVm.builder()
.id(address.getId())
Expand All @@ -20,8 +32,11 @@ public static AddressDetailVm fromModel(Address address) {
.districtName(address.getDistrict().getName())
.stateOrProvinceId(address.getStateOrProvince().getId())
.stateOrProvinceName(address.getStateOrProvince().getName())
.stateOrProvinceCode(address.getStateOrProvince().getCode())
.countryId(address.getCountry().getId())
.countryName(address.getCountry().getName())
.countryCode2(address.getCountry().getCode2())
.countryCode3(address.getCountry().getCode3())
.zipCode(address.getZipCode())
.build();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@
import org.springframework.boot.autoconfigure.SpringBootApplication;
import org.springframework.boot.context.properties.EnableConfigurationProperties;

@SpringBootApplication
@SpringBootApplication(scanBasePackages = {"com.yas.order", "com.yas.commonlibrary"})
@EnableConfigurationProperties(ServiceUrlConfig.class)
public class OrderApplication {

public static void main(String[] args) {
SpringApplication.run(OrderApplication.class, args);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,5 +4,5 @@

@ConfigurationProperties(prefix = "yas.services")
public record ServiceUrlConfig(
String cart, String customer, String product, String tax, String promotion) {
}
String cart, String customer, String product, String tax, String promotion, String payment) {

Choose a reason for hiding this comment

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

Missing payment service URL config

Adding 'String payment' to the ServiceUrlConfig record introduces a new required configuration property. The PaymentService in the order module uses serviceUrlConfig.payment() to construct URLs for payment provider calls, but without the corresponding 'yas.services.payment' property in application.properties, Spring Boot will fail to bind the properties at startup, causing the entire order service to crash. This breaks the payment integration functionality that relies on this configuration.

Code suggestion
Check the AI-generated fix before applying
 -yas.services.promotion=http://api.yas.local/promotion
 +yas.services.promotion=http://api.yas.local/promotion
 +yas.services.payment=http://api.yas.local/payment

Code Review Run #f96c13


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

}
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import com.yas.commonlibrary.constants.ApiConstant;
import com.yas.order.service.CheckoutService;
import com.yas.order.viewmodel.ErrorVm;
import com.yas.order.viewmodel.checkout.CheckoutPatchVm;
import com.yas.order.viewmodel.checkout.CheckoutPaymentMethodPutVm;
import com.yas.order.viewmodel.checkout.CheckoutPostVm;
import com.yas.order.viewmodel.checkout.CheckoutStatusPutVm;
Expand All @@ -12,9 +13,11 @@
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.annotations.responses.ApiResponses;
import jakarta.validation.Valid;
import java.util.Map;
import lombok.RequiredArgsConstructor;
import org.springframework.http.ResponseEntity;
import org.springframework.web.bind.annotation.GetMapping;
import org.springframework.web.bind.annotation.PatchMapping;
import org.springframework.web.bind.annotation.PathVariable;
import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.PutMapping;
Expand Down Expand Up @@ -56,4 +59,18 @@ public ResponseEntity<Void> updatePaymentMethod(@PathVariable final String id,
checkoutService.updateCheckoutPaymentMethod(id, checkoutPaymentMethodPutVm);
return ResponseEntity.ok().build();
}

@PatchMapping("/storefront/checkouts/{id}")
@ApiResponses(value = {
@ApiResponse(responseCode = ApiConstant.CODE_200, description = ApiConstant.OK,
content = @Content()),
@ApiResponse(responseCode = ApiConstant.CODE_404, description = ApiConstant.NOT_FOUND,
content = @Content(schema = @Schema(implementation = ErrorVm.class))),
@ApiResponse(responseCode = ApiConstant.CODE_400, description = ApiConstant.BAD_REQUEST,
content = @Content(schema = @Schema(implementation = ErrorVm.class)))})
public ResponseEntity<CheckoutVm> updateCheckout(@PathVariable final String id,
@Valid @RequestBody
CheckoutPatchVm checkoutPatchVm) {
return ResponseEntity.ok(checkoutService.updateCheckoutByFields(id, checkoutPatchVm));

Choose a reason for hiding this comment

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

Missing input validation for shipping address ID

The new PATCH endpoint for updating checkout fields calls checkoutService.updateCheckoutByFields, which in turn invokes updateAddress in the CheckoutService. This method attempts to parse the shippingAddressId string to a Long without proper error handling, risking a NumberFormatException that would cause a 500 error instead of a proper 400 Bad Request. This breaks the API's error handling for invalid input, potentially crashing the update operation in production.

Code suggestion
Check the AI-generated fix before applying
 -    public void updateAddress(Checkout existingCheckout, String value) {
 -        Optional<ActiveAddressVm> address = customerService.getUserAddresses()
 -            .stream().filter(add -> Objects.equals(add.id(), Long.valueOf(value)))
 -            .findFirst();
 -        if (address.isEmpty()) {
 -            throw new NotFoundException(ADDRESS_NOT_AVAILABLE, value);
 -        }
 -        existingCheckout.setShippingAddressId(Long.parseLong(value));
 +    public void updateAddress(Checkout existingCheckout, String value) {
 +        Long addressId;
 +        try {
 +            addressId = Long.parseLong(value);
 +        } catch (NumberFormatException e) {
 +            throw new BadRequestException("Invalid shipping address id: " + value);
 +        }
 +        Optional<ActiveAddressVm> address = customerService.getUserAddresses()
 +            .stream().filter(add -> Objects.equals(add.id(), addressId))
 +            .findFirst();
 +        if (address.isEmpty()) {
 +            throw new NotFoundException(ADDRESS_NOT_AVAILABLE, value);
 +        }
 +        existingCheckout.setShippingAddressId(addressId);

Code Review Run #f96c13


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
package com.yas.order.mapper;

import com.yas.order.model.CheckoutAddress;
import com.yas.order.viewmodel.customer.ActiveAddressVm;
import org.mapstruct.Mapper;
import org.mapstruct.Mapping;
import org.mapstruct.MappingTarget;
import org.springframework.stereotype.Component;

@Mapper(componentModel = "spring")
@Component
public interface AddressMapper {
@Mapping(target = "id", ignore = true)
CheckoutAddress toModel(ActiveAddressVm activeAddressVm);

ActiveAddressVm toVm(CheckoutAddress checkoutAddress);

@Mapping(target = "id", ignore = true)
CheckoutAddress updateModel(@MappingTarget CheckoutAddress checkoutAddress, ActiveAddressVm activeAddressVm);
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,10 @@ public interface CheckoutMapper {
CheckoutItemVm toVm(CheckoutItem checkoutItem);

@Mapping(target = "checkoutItemVms", ignore = true)
@Mapping(target = "shippingAddressDetail", ignore = true)
CheckoutVm toVm(Checkout checkout);

default BigDecimal map(BigDecimal value) {
return value != null ? value : BigDecimal.ZERO;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,12 @@
import jakarta.persistence.Entity;
import jakarta.persistence.EnumType;
import jakarta.persistence.Enumerated;
import jakarta.persistence.FetchType;
import jakarta.persistence.GeneratedValue;
import jakarta.persistence.GenerationType;
import jakarta.persistence.Id;
import jakarta.persistence.OneToMany;
import jakarta.persistence.OneToOne;
import jakarta.persistence.Table;
import java.math.BigDecimal;
import java.util.ArrayList;
Expand Down Expand Up @@ -57,12 +59,18 @@ public class Checkout extends AbstractAuditEntity {
@SuppressWarnings("unused")
private String shipmentMethodId;

@SuppressWarnings("unused")
private String shipmentServiceId;

@Column(name = "payment_method_id")
private String paymentMethodId;

@SuppressWarnings("unused")
private Long shippingAddressId;

@OneToOne(fetch = FetchType.EAGER, cascade = CascadeType.PERSIST)

Choose a reason for hiding this comment

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

Incorrect JPA OneToOne mapping

The newly added @OnetoOne relationship to CheckoutAddress lacks the mappedBy attribute, causing both entities to attempt owning the foreign key. This invalid JPA mapping will result in a MappingException at runtime when the application tries to persist or load entities, breaking checkout functionality in CheckoutService.createCheckout and related operations. Add mappedBy = "checkout" to the annotation since CheckoutAddress already owns the relationship via @joincolumn.

Code suggestion
Check the AI-generated fix before applying
Suggested change
@OneToOne(fetch = FetchType.EAGER, cascade = CascadeType.PERSIST)
@OneToOne(fetch = FetchType.EAGER, cascade = CascadeType.PERSIST, mappedBy = "checkout")

Code Review Run #f96c13


Should Bito avoid suggestions like this for future reviews? (Manage Rules)

  • Yes, avoid them

private CheckoutAddress checkoutAddress;

@SuppressWarnings("unused")
@JdbcTypeCode(SqlTypes.JSON)
@Column(name = "last_error", columnDefinition = "jsonb")
Expand Down
Loading