Conversation
WalkthroughEndpoints for creating and updating places were converted to multipart flows (JSON @RequestPart + optional images) returning 204 No Content; DTOs were flattened (address + categoryId), discount fields and related UI removed, several DTOs/mappers/tests deleted, service methods renamed/adjusted, tests updated, and JwtTool gained cookie-based token extraction. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as Client/UI
participant PC as PlaceController
participant PS as PlaceService
participant Repo as Repositories
participant Geo as GeocodingProvider
rect rgb(245,250,255)
note right of UI: Multipart flow — JSON dto part + optional images
UI->>PC: POST /v2/places/save (multipart: AddPlaceDto + images)
UI->>PC: PUT /v2/places/update (multipart: PlaceUpdateDto + images)
end
PC->>PS: save(dto, userId, images) / update(dto, images, userId)
PS->>Repo: findUserById(userId)
PS->>Repo: findCategoryById(categoryId)
PS->>Geo: geocode(address)
alt geocode results found
PS->>Repo: save(place with openingHours, photos, location)
PS-->>PC: void
PC-->>UI: 204 No Content
else geocode empty / not found
PS-->>PC: throws NotFound/NoGeocode
PC-->>UI: 4xx error
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Notes for reviewer attention:
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 18
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
service/src/test/java/greencity/service/NotificationServiceImplTest.java (1)
91-94: Fix copy-paste bug: assigning category/id to testPlace1 instead of testPlace2This causes duplicated data and can hide errors. Assign to testPlace2.
@@ - Place testPlace2 = getPlace(); - testPlace1.setCategory(getCategory()); - testPlace1.setId(2L); + Place testPlace2 = getPlace(); + testPlace2.setCategory(getCategory()); + testPlace2.setId(2L); @@ - Place testPlace2 = getPlace(); - testPlace1.setCategory(getCategory()); - testPlace1.setId(2L); + Place testPlace2 = getPlace(); + testPlace2.setCategory(getCategory()); + testPlace2.setId(2L); @@ - Place testPlace2 = getPlace(); - testPlace1.setCategory(getCategory()); - testPlace1.setId(2L); + Place testPlace2 = getPlace(); + testPlace2.setCategory(getCategory()); + testPlace2.setId(2L);Also applies to: 130-133, 169-172
core/src/main/resources/templates/core/management_places.html (2)
326-331: Address input: avoid ASCII-only pattern and fix error span id to match DTOThe regex blocks non-Latin addresses (i18n). Also, the error span id should match server field ‘address’ for error mapping.
- <input class="form-control" id="address" name="address" type="text" - pattern="^[A-Za-z0-9\s,.-]+$" - title="Please enter a valid address" - required> - <span class="errorSpan" th:id="errorModalSavelocation"></span> + <input class="form-control" id="address" name="address" type="text" required> + <span class="errorSpan" th:id="errorModalSaveaddress"></span>
470-472: Remove hardcoded Google Maps API key from sourceLeaking API keys in HTML is unsafe. Load via server-side config/env and restrict the key by domain/IP.
core/src/main/resources/static/scripts/management/places/buttonsAJAX.js (1)
221-225: Fix error span ids to match updated HTML and DTO field namesEnsures client-side validation displays in the right spans and aligns with server field names.
- const requiredFields = [ - { id: 'address', errorId: 'errorModalSavelocation', message: messages["greenCity.places.page.add.address"] }, - { id: 'placeName', errorId: 'errorModalSavename', message: messages["greenCity.places.page.add.place.name"] }, - { id: 'category', errorId: 'errorModalSavecategory', message: messages["greenCity.places.page.add.category"] }, - ]; + const requiredFields = [ + { id: 'address', errorId: 'errorModalSaveaddress', message: messages["greenCity.places.page.add.address"] }, + { id: 'placeName', errorId: 'errorModalSavename', message: messages["greenCity.places.page.add.place.name"] }, + { id: 'category', errorId: 'errorModalSavecategoryId', message: messages["greenCity.places.page.add.category"] }, + ];service/src/test/java/greencity/ModelUtils.java (1)
3331-3334: Fix copy/paste bug: longitude assigned to latitude.Second setter should be longitude, not latitude.
.coordinates(AddressDto.builder() - .latitude(50.1234) - .latitude(30.1234) + .latitude(50.1234) + .longitude(30.1234) .build())service/src/main/java/greencity/service/PlaceServiceImpl.java (1)
519-533: Guard against null Photo on upload failure (NPE/IllegalArgument).If upload throws, newPhoto stays null but is still passed to photoRepo.save(newPhoto), causing runtime failure.
Apply:
- Photo savedPhoto = photoRepo.save(newPhoto); - newPhotos.add(savedPhoto); + if (newPhoto != null) { + Photo savedPhoto = photoRepo.save(newPhoto); + newPhotos.add(savedPhoto); + }
🧹 Nitpick comments (14)
service-api/src/main/java/greencity/dto/place/AddPlaceDto.java (2)
32-33: Ensure categoryId is positiveGuard against invalid IDs by requiring positive values.
- @NotNull - private Long categoryId; + @NotNull + @jakarta.validation.constraints.Positive + private Long categoryId;Add import if not present:
import jakarta.validation.constraints.Positive;
29-31: Consider a max length constraint on addressAdd a safe upper bound (e.g., @Length(max = 255)) to prevent oversized input.
core/src/main/java/greencity/webcontroller/ManagementPlacesController.java (2)
91-121: Javadoc vs behavior: align documented responses with actual return typeDocs mention 204/void and various HTTP codes, but the method returns 200 with GenericResponseDto. Adjust Javadoc or response semantics for consistency.
144-175: Javadoc consistency for update endpointSame as save: documented statuses don’t match the 200 + GenericResponseDto behavior. Please align.
core/src/main/resources/templates/core/management_places.html (1)
403-406: Optional: allow multiple image uploads in the formThe update flow supports multiple files; align the create flow with
multiple.- <input type="file" class="form-control" name="imagePath" id="creationFile" placeholder="ferf"> + <input type="file" class="form-control" name="imagePath" id="creationFile" multiple>core/src/main/resources/static/scripts/management/places/buttonsAJAX.js (1)
162-165: POST also append all selected files for ‘images’ (parity with PUT)Supports multiple uploads consistently.
- const file = document.getElementById("creationFile").files[0]; - if (file) { - formData.append("images", file); - } + const fileInput = document.getElementById("creationFile"); + Array.from(fileInput?.files || []).forEach((file) => { + formData.append("images", file); + });core/src/test/java/greencity/controller/PlaceControllerTest.java (1)
469-496: Remove legacy commented-out block.This large commented section adds noise and can confuse future maintenance.
-// String json = """ -// { -// "categoryName": "test", -// "locationName": "вулиця Під Дубом, 7Б, Львів, Львівська область, 79000", -// "openingHoursList": [ -// { -// "closeTime": "20:00", -// "openTime": "08:00", -// "weekDay": "MONDAY" -// } -// ], -// "placeName": "Форум Львів" -// } -// """; -// -// ObjectMapper objectMapper = new ObjectMapper(); -// objectMapper.findAndRegisterModules(); -// AddPlaceDto addPlaceDtoRequest = objectMapper.readValue(json, AddPlaceDto.class); -// String jsonValue = objectMapper.writeValueAsString(addPlaceDtoRequest); -// -// MockMultipartFile jsonFile = new MockMultipartFile("dto", "", -// "application/json", jsonValue.getBytes()); -// mockMvc.perform(multipart(placeLink + "/v2/save") -// .file(jsonFile) -// .principal(principal) -// .accept(MediaType.APPLICATION_JSON) -// .contentType(MediaType.APPLICATION_JSON)) -// .andExpect(status().isCreated());core/src/test/java/greencity/webcontroller/ManagementPlacesControllerTest.java (1)
198-198: Delete obsolete commented verification.Keeps the test suite concise and focused on current behavior.
-// verify(placeService, never()).updateFromUI(any(PlaceUpdateDto.class), any(), any());service-api/src/main/java/greencity/dto/place/PlaceUpdateDto.java (1)
31-35: Consider limiting address length.Add a max-length constraint for address to prevent excessively large inputs (e.g., @Length with a project-standard max). If there’s a ServiceValidationConstants for address, reuse it.
service/src/test/java/greencity/ModelUtils.java (1)
2126-2136: Ensure test helper builds a valid AddPlaceDto.Provide minimal required fields (name, address) to match new validations.
public static AddPlaceDto getAddPlaceDto() { return AddPlaceDto.builder() - .categoryId(1L) + .name("Test Place") + .address("Kyiv") + .categoryId(1L) // .placeName("test") // .locationName("Test") .openingHoursList(Set.of(OpeningHoursDto.builder() .openTime(LocalTime.now()) .closeTime(LocalTime.now()) .weekDay(DayOfWeek.MONDAY) .build())) .build(); }service/src/test/java/greencity/service/PlaceServiceImplTest.java (1)
230-236: Generalize stubbing for coordinates to avoid brittle tests.Match any coordinates instead of hardcoding values that may diverge from geocoder output.
- when(locationService.existsByLatAndLng(50.45, 30.52)).thenReturn(false); + when(locationService.existsByLatAndLng(any(), any())).thenReturn(false);Also remove the debug print inside the argThat block.
- verify(placeRepo).save(argThat(place -> { - System.out.println(place.getLocation()); + verify(placeRepo).save(argThat(place -> { return place.getName().equals("My Cafe") && place.getAuthor().equals(user) && place.getCategory().equals(category) && place.getOpeningHoursList().size() == 1; }));core/src/main/java/greencity/controller/PlaceController.java (3)
96-106: Document 204 response in OpenAPI.Method returns 204 NO_CONTENT but it's not listed in @ApiResponses. Add:
@ApiResponses(value = { + @ApiResponse(responseCode = "204", description = HttpStatuses.NO_CONTENT), @ApiResponse(responseCode = "400", description = HttpStatuses.BAD_REQUEST,
495-499: Add 204 to API responses for create.Endpoint returns 204 NO_CONTENT; include it in docs:
@ApiResponses(value = { + @ApiResponse(responseCode = "204", description = HttpStatuses.NO_CONTENT), @ApiResponse(responseCode = "400", description = HttpStatuses.BAD_REQUEST,
57-75: Align JavaDoc with actual behavior.If you keep only multipart for this method (with JSON handled by an overload), update the doc to reflect that. If you add the JSON overload, mention both variants explicitly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
core/src/main/java/greencity/controller/PlaceController.java(4 hunks)core/src/main/java/greencity/dto/place/PlaceWithUserDto.java(0 hunks)core/src/main/java/greencity/webcontroller/ManagementPlacesController.java(3 hunks)core/src/main/resources/static/scripts/management/places/buttonsAJAX.js(4 hunks)core/src/main/resources/templates/core/management_places.html(2 hunks)core/src/test/java/greencity/controller/PlaceControllerTest.java(3 hunks)core/src/test/java/greencity/webcontroller/ManagementPlacesControllerTest.java(4 hunks)service-api/src/main/java/greencity/dto/category/CategoryDto.java(1 hunks)service-api/src/main/java/greencity/dto/location/LocationAddressAndGeoForUpdateDto.java(0 hunks)service-api/src/main/java/greencity/dto/place/AddPlaceDto.java(2 hunks)service-api/src/main/java/greencity/dto/place/PlaceAddDto.java(0 hunks)service-api/src/main/java/greencity/dto/place/PlaceUpdateDto.java(1 hunks)service-api/src/main/java/greencity/security/jwt/JwtTool.java(2 hunks)service-api/src/main/java/greencity/service/PlaceService.java(2 hunks)service/src/main/java/greencity/mapping/AddPlaceDtoMapper.java(0 hunks)service/src/main/java/greencity/mapping/ProposePlaceMapper.java(0 hunks)service/src/main/java/greencity/service/PlaceServiceImpl.java(2 hunks)service/src/test/java/greencity/ModelUtils.java(3 hunks)service/src/test/java/greencity/mapping/AddPlaceDtoMapperTest.java(0 hunks)service/src/test/java/greencity/mapping/ProposePlaceMapperTest.java(0 hunks)service/src/test/java/greencity/service/NotificationServiceImplTest.java(3 hunks)service/src/test/java/greencity/service/PlaceServiceImplTest.java(3 hunks)
💤 Files with no reviewable changes (7)
- service-api/src/main/java/greencity/dto/location/LocationAddressAndGeoForUpdateDto.java
- service-api/src/main/java/greencity/dto/place/PlaceAddDto.java
- service/src/main/java/greencity/mapping/AddPlaceDtoMapper.java
- service/src/main/java/greencity/mapping/ProposePlaceMapper.java
- service/src/test/java/greencity/mapping/AddPlaceDtoMapperTest.java
- service/src/test/java/greencity/mapping/ProposePlaceMapperTest.java
- core/src/main/java/greencity/dto/place/PlaceWithUserDto.java
🧰 Additional context used
🧬 Code graph analysis (4)
service/src/test/java/greencity/service/PlaceServiceImplTest.java (1)
service/src/test/java/greencity/ModelUtils.java (1)
ModelUtils(291-3705)
service-api/src/main/java/greencity/dto/place/PlaceUpdateDto.java (1)
core/src/main/resources/static/scripts/management/events/createEvent.js (1)
address(299-299)
service/src/main/java/greencity/service/PlaceServiceImpl.java (1)
core/src/main/resources/static/scripts/management/places/buttonsAJAX.js (6)
place(138-143)place(149-155)i(57-57)openTime(239-239)closeTime(240-240)openingHours(271-271)
service-api/src/main/java/greencity/dto/place/AddPlaceDto.java (1)
core/src/main/resources/static/scripts/management/events/createEvent.js (1)
address(299-299)
🔇 Additional comments (7)
service-api/src/main/java/greencity/dto/category/CategoryDto.java (1)
17-17: LGTM: id field addition aligns with new APIAdding
idto CategoryDto is consistent with the refactor and test updates.service/src/test/java/greencity/service/NotificationServiceImplTest.java (1)
107-114: LGTM: CategoryDto construction updated to new signatureUsing
new CategoryDto(1L, ...)matches the added id field.Also applies to: 146-153, 184-191
core/src/test/java/greencity/webcontroller/ManagementPlacesControllerTest.java (1)
77-77: LGTM: CategoryDto with id update.Using CategoryDto(1L, ...) aligns with the new constructor and API shape.
service/src/test/java/greencity/ModelUtils.java (1)
3466-3469: LGTM: CategoryDto factory updated to new ctor.Matches the DTO signature update.
service/src/main/java/greencity/service/PlaceServiceImpl.java (2)
321-340: LGTM: explicit DTO assembly for update info.Clear, focused fields for update; good move away from blind mapping.
614-635: OpeningHours cascade configuration is sufficient.Place.openingHoursListis annotated withcascade={PERSIST, MERGE}, and old entries are explicitly deleted before setting new ones, so newOpeningHoursentities will be persisted correctly without needingorphanRemoval.core/src/main/java/greencity/controller/PlaceController.java (1)
500-507: Restrict save endpoint to multipart, add JSON overload, and enforce validation
- Update saveEcoPlaceFromUi in PlaceController.java to consume only MediaType.MULTIPART_FORM_DATA_VALUE and annotate the @RequestPart AddPlaceDto with @Valid
- Add a separate @PostMapping for application/json using @RequestBody and @Valid (no images)
- Verify no existing callers post JSON to /place/v2/save using the provided script
| @PutMapping(value = "/update", consumes = {MediaType.MULTIPART_FORM_DATA_VALUE, MediaType.APPLICATION_JSON_VALUE}) | ||
| @ResponseStatus(HttpStatus.NO_CONTENT) | ||
| public void updatePlace(@Parameter(required = true) @RequestPart PlaceUpdateDto dto, | ||
| @Parameter(hidden = true) @CurrentUserId Long userId, | ||
| @RequestPart(required = false) @Nullable MultipartFile[] images) { | ||
| placeService.update(dto, images, userId); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Fix multipart vs JSON handling and restore validation (@Valid).
@RequestPart requires multipart; with consumes including application/json this will fail for JSON clients. Also, @Valid on DTO was dropped.
- Either restrict this method to multipart only and add an overload for JSON with @RequestBody, or drop application/json from consumes.
- Add @Valid to the DTO.
Apply this diff here:
- @PutMapping(value = "/update", consumes = {MediaType.MULTIPART_FORM_DATA_VALUE, MediaType.APPLICATION_JSON_VALUE})
+ @PutMapping(value = "/update", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
@@
- public void updatePlace(@Parameter(required = true) @RequestPart PlaceUpdateDto dto,
+ public void updatePlace(@Parameter(required = true) @Valid @RequestPart PlaceUpdateDto dto,Add this overload (outside the selected range) to support pure JSON:
@PutMapping(value = "/update", consumes = MediaType.APPLICATION_JSON_VALUE)
@ResponseStatus(HttpStatus.NO_CONTENT)
public void updatePlaceJson(@Valid @RequestBody PlaceUpdateDto dto,
@Parameter(hidden = true) @CurrentUserId Long userId) {
placeService.update(dto, null, userId);
}Run this to find any app/json callers for /place/update:
🏁 Script executed:
#!/bin/bash
rg -nC3 -P '/place/(v2/)?update' --type js --type ts --type java --type html
rg -nC2 -P '(Content-Type|contentType)\s*[:=]\s*["\']application/json' --type js --type tsLength of output: 2506
🏁 Script executed:
#!/usr/bin/env bash
set +e
# Search for any client calls to /place/update in code and templates
rg -nC3 "/place/update" --type js --type ts --type java --type html
# Search for JSON content-type usage in JS/TS clients
rg -nC2 -e "(Content-Type|contentType)\s*[:=]\s*['\"]application/json" --type js --type tsLength of output: 24298
Separate multipart vs JSON update endpoints and restore validation
@RequestPart only binds multipart; drop application/json from consumes, add @Valid on the DTO, and add a JSON‐only overload with @RequestBody.
In core/src/main/java/greencity/controller/PlaceController.java:
- @PutMapping(value = "/update", consumes = {MediaType.MULTIPART_FORM_DATA_VALUE, MediaType.APPLICATION_JSON_VALUE})
+ @PutMapping(value = "/update", consumes = MediaType.MULTIPART_FORM_DATA_VALUE)
@@
- public void updatePlace(@Parameter(required = true) @RequestPart PlaceUpdateDto dto,
+ public void updatePlace(@Parameter(required = true) @Valid @RequestPart PlaceUpdateDto dto,Add below:
@PutMapping(value = "/update", consumes = MediaType.APPLICATION_JSON_VALUE)
@ResponseStatus(HttpStatus.NO_CONTENT)
public void updatePlaceJson(@Valid @RequestBody PlaceUpdateDto dto,
@CurrentUserId Long userId) {
placeService.update(dto, null, userId);
}🤖 Prompt for AI Agents
In core/src/main/java/greencity/controller/PlaceController.java around lines 107
to 113, the current @PutMapping declares consumes = {MULTIPART_FORM_DATA,
APPLICATION_JSON} while using @RequestPart (which only binds multipart) and the
DTO lacks validation; change the multipart handler to remove APPLICATION_JSON
from consumes and add @Valid to the @RequestPart PlaceUpdateDto parameter;
additionally add a new JSON-only overload mapped to the same "/update" with
consumes = MediaType.APPLICATION_JSON_VALUE that accepts @Valid @RequestBody
PlaceUpdateDto and the @CurrentUserId, and calls placeService.update(dto, null,
userId) so JSON requests are handled correctly and DTO validation is restored.
core/src/main/resources/static/scripts/management/places/buttonsAJAX.js
Outdated
Show resolved
Hide resolved
| "id": "1", | ||
| "name": "Тестове місце", | ||
| "address": "Смиків, південна 7", | ||
| "categoryId": "1", | ||
| "openingHoursList": [ |
There was a problem hiding this comment.
Avoid relying on Jackson string-to-number coercion in JSON payload.
Use numeric literals for id/categoryId to match DTO types and prevent fragile coercion.
- "id": "1",
+ "id": 1,
...
- "categoryId": "1",
+ "categoryId": 1,🤖 Prompt for AI Agents
In core/src/test/java/greencity/controller/PlaceControllerTest.java around lines
351 to 355, the JSON test payload uses string values for numeric fields ("id":
"1", "categoryId": "1"); change these to numeric literals (id: 1, categoryId: 1)
so the JSON matches the DTO types and avoids relying on Jackson's
string-to-number coercion; update any other numeric fields in the payload
similarly and run the tests to ensure deserialization now uses proper numeric
types.
| "name": "Тестове місце", | ||
| "address": "Смиків, південна 7", | ||
| "categoryId": "1", | ||
| "openingHoursList": [ | ||
| { | ||
| "weekDay": "MONDAY", |
There was a problem hiding this comment.
Use numeric value for categoryId.
DTO expects a Long; pass a number instead of a quoted string.
- "categoryId": "1",
+ "categoryId": 1,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "name": "Тестове місце", | |
| "address": "Смиків, південна 7", | |
| "categoryId": "1", | |
| "openingHoursList": [ | |
| { | |
| "weekDay": "MONDAY", | |
| "name": "Тестове місце", | |
| "address": "Смиків, південна 7", | |
| "categoryId": 1, | |
| "openingHoursList": [ | |
| { | |
| "weekDay": "MONDAY", |
🤖 Prompt for AI Agents
In core/src/test/java/greencity/controller/PlaceControllerTest.java around lines
439 to 444, the test JSON uses a quoted string for categoryId ("1") but the DTO
expects a Long; update the test payload to use a numeric value (unquoted) for
categoryId (e.g., 1) so the deserializer maps it to a Long correctly.
| private Set<OpeningHours> mapOpeningHours(Set<OpeningHoursDto> dtos) { | ||
| return dtos.stream() | ||
| .map(i -> OpeningHours.builder() | ||
| .openTime(i.getOpenTime()) | ||
| .closeTime(i.getCloseTime()) | ||
| .weekDay(i.getWeekDay()) | ||
| .build()) | ||
| .collect(Collectors.toSet()); | ||
| } |
There was a problem hiding this comment.
Null-safety for opening hours mapping.
mapOpeningHours() NPEs when dtos is null. Treat null as empty.
- private Set<OpeningHours> mapOpeningHours(Set<OpeningHoursDto> dtos) {
- return dtos.stream()
+ private Set<OpeningHours> mapOpeningHours(Set<OpeningHoursDto> dtos) {
+ if (dtos == null || dtos.isEmpty()) {
+ return new HashSet<>();
+ }
+ return dtos.stream()
.map(i -> OpeningHours.builder()
.openTime(i.getOpenTime())
.closeTime(i.getCloseTime())
.weekDay(i.getWeekDay())
.build())
.collect(Collectors.toSet());
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Set<OpeningHours> mapOpeningHours(Set<OpeningHoursDto> dtos) { | |
| return dtos.stream() | |
| .map(i -> OpeningHours.builder() | |
| .openTime(i.getOpenTime()) | |
| .closeTime(i.getCloseTime()) | |
| .weekDay(i.getWeekDay()) | |
| .build()) | |
| .collect(Collectors.toSet()); | |
| } | |
| private Set<OpeningHours> mapOpeningHours(Set<OpeningHoursDto> dtos) { | |
| if (dtos == null || dtos.isEmpty()) { | |
| return new HashSet<>(); | |
| } | |
| return dtos.stream() | |
| .map(i -> OpeningHours.builder() | |
| .openTime(i.getOpenTime()) | |
| .closeTime(i.getCloseTime()) | |
| .weekDay(i.getWeekDay()) | |
| .build()) | |
| .collect(Collectors.toSet()); | |
| } |
🤖 Prompt for AI Agents
In service/src/main/java/greencity/service/PlaceServiceImpl.java around lines
702 to 710, mapOpeningHours currently NPEs when dtos is null; modify it to treat
null as empty and avoid NPEs by returning Collections.emptySet() (or new
HashSet<>()) when dtos is null, and also defensively filter out any null
elements before mapping (e.g., dtos.stream().filter(Objects::nonNull) ...). Keep
the existing mapping logic otherwise.
| public static AddPlaceDto getPlaceAddDto() { | ||
| AddPlaceDto addPlaceDto = new AddPlaceDto(); | ||
| addPlaceDto.setName("Test"); | ||
| CategoryDto category = new CategoryDto(); | ||
| category.setNameEn("category"); | ||
| placeAddDto.setCategory(category); | ||
| placeAddDto.setLocation(getLocationAddressAndGeoDto()); | ||
| addPlaceDto.setCategoryId(1L); | ||
| HashSet<OpeningHoursDto> openingHoursDtos = new HashSet<>(); | ||
| openingHoursDtos.add(getOpeningHoursDto()); | ||
| placeAddDto.setOpeningHoursList(openingHoursDtos); | ||
| addPlaceDto.setOpeningHoursList(openingHoursDtos); | ||
| HashSet<DiscountValueDto> discountValueDtos = new HashSet<>(); | ||
| discountValueDtos.add(getDiscountValueDto()); | ||
| placeAddDto.setDiscountValues(discountValueDtos); | ||
| return placeAddDto; | ||
| return addPlaceDto; | ||
| } |
There was a problem hiding this comment.
Complete AddPlaceDto and remove unused local variable.
- Set address to reflect new API.
- Drop the unused CategoryDto variable.
public static AddPlaceDto getPlaceAddDto() {
- AddPlaceDto addPlaceDto = new AddPlaceDto();
- addPlaceDto.setName("Test");
- CategoryDto category = new CategoryDto();
- category.setNameEn("category");
- addPlaceDto.setCategoryId(1L);
+ AddPlaceDto addPlaceDto = new AddPlaceDto();
+ addPlaceDto.setName("Test");
+ addPlaceDto.setAddress("Kyiv");
+ addPlaceDto.setCategoryId(1L);
HashSet<OpeningHoursDto> openingHoursDtos = new HashSet<>();
openingHoursDtos.add(getOpeningHoursDto());
addPlaceDto.setOpeningHoursList(openingHoursDtos);
HashSet<DiscountValueDto> discountValueDtos = new HashSet<>();
discountValueDtos.add(getDiscountValueDto());
return addPlaceDto;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static AddPlaceDto getPlaceAddDto() { | |
| AddPlaceDto addPlaceDto = new AddPlaceDto(); | |
| addPlaceDto.setName("Test"); | |
| CategoryDto category = new CategoryDto(); | |
| category.setNameEn("category"); | |
| placeAddDto.setCategory(category); | |
| placeAddDto.setLocation(getLocationAddressAndGeoDto()); | |
| addPlaceDto.setCategoryId(1L); | |
| HashSet<OpeningHoursDto> openingHoursDtos = new HashSet<>(); | |
| openingHoursDtos.add(getOpeningHoursDto()); | |
| placeAddDto.setOpeningHoursList(openingHoursDtos); | |
| addPlaceDto.setOpeningHoursList(openingHoursDtos); | |
| HashSet<DiscountValueDto> discountValueDtos = new HashSet<>(); | |
| discountValueDtos.add(getDiscountValueDto()); | |
| placeAddDto.setDiscountValues(discountValueDtos); | |
| return placeAddDto; | |
| return addPlaceDto; | |
| } | |
| public static AddPlaceDto getPlaceAddDto() { | |
| AddPlaceDto addPlaceDto = new AddPlaceDto(); | |
| addPlaceDto.setName("Test"); | |
| addPlaceDto.setAddress("Kyiv"); | |
| addPlaceDto.setCategoryId(1L); | |
| HashSet<OpeningHoursDto> openingHoursDtos = new HashSet<>(); | |
| openingHoursDtos.add(getOpeningHoursDto()); | |
| addPlaceDto.setOpeningHoursList(openingHoursDtos); | |
| HashSet<DiscountValueDto> discountValueDtos = new HashSet<>(); | |
| discountValueDtos.add(getDiscountValueDto()); | |
| return addPlaceDto; | |
| } |
🤖 Prompt for AI Agents
In service/src/test/java/greencity/ModelUtils.java around lines 1141 to 1153,
remove the unused local CategoryDto variable, set the AddPlaceDto address field
to the new API expected value (e.g., addPlaceDto.setAddress(...) with a test
AddressDto/String as required), and ensure the discountValueDtos collection is
actually assigned to the addPlaceDto (e.g., addPlaceDto.setDiscounts(...) or the
correct setter name). Keep the existing openingHoursList assignment and the
categoryId assignment as-is.
| void updateFromUIThrowsNotFoundExceptionForCategoryTest() { | ||
| PlaceUpdateDto dto = new PlaceUpdateDto(); | ||
| dto.setId(1L); | ||
| dto.setName("Updated Place"); | ||
| dto.setCategory(new CategoryDto("Test Category", "Test Category Ua", 1L)); | ||
| dto.setLocation(new LocationAddressAndGeoForUpdateDto("New Address", 50.45, 30.52, "New Address Ua")); | ||
|
|
||
| MultipartFile[] images = new MultipartFile[] { | ||
| new MockMultipartFile("image1.jpg", "image1.jpg", "image/jpeg", new byte[0]) | ||
| }; | ||
| LocationVO locationVO = LocationVO.builder() | ||
| .id(1L) | ||
| .addressEn("New Address") | ||
| .lat(50.45) | ||
| .lng(30.52) | ||
| .addressUk("New Address Ua") | ||
| dto.setCategoryId(1L); | ||
|
|
||
| when(categoryService.findByName("Nonexistent Category")) | ||
| .thenThrow(new NotFoundException("Category not found")); | ||
| assertThrows(NotFoundException.class, () -> placeServiceImpl.update(dto, null, TestConst.USER_ID)); | ||
| } |
There was a problem hiding this comment.
Broken test: stubs old API (findByName) but update(...) now uses categoryId.
This test won’t fail for missing category with the new flow. Stub CategoryRepo by id instead.
void updateFromUIThrowsNotFoundExceptionForCategoryTest() {
PlaceUpdateDto dto = new PlaceUpdateDto();
dto.setId(1L);
dto.setCategoryId(1L);
-
- when(categoryService.findByName("Nonexistent Category"))
- .thenThrow(new NotFoundException("Category not found"));
+ when(userRepo.findById(TestConst.USER_ID)).thenReturn(Optional.of(user));
+ when(placeRepo.findById(1L)).thenReturn(Optional.of(ModelUtils.getPlace()));
+ when(categoryRepo.findById(1L)).thenReturn(Optional.empty());
assertThrows(NotFoundException.class, () -> placeServiceImpl.update(dto, null, TestConst.USER_ID));
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void updateFromUIThrowsNotFoundExceptionForCategoryTest() { | |
| PlaceUpdateDto dto = new PlaceUpdateDto(); | |
| dto.setId(1L); | |
| dto.setName("Updated Place"); | |
| dto.setCategory(new CategoryDto("Test Category", "Test Category Ua", 1L)); | |
| dto.setLocation(new LocationAddressAndGeoForUpdateDto("New Address", 50.45, 30.52, "New Address Ua")); | |
| MultipartFile[] images = new MultipartFile[] { | |
| new MockMultipartFile("image1.jpg", "image1.jpg", "image/jpeg", new byte[0]) | |
| }; | |
| LocationVO locationVO = LocationVO.builder() | |
| .id(1L) | |
| .addressEn("New Address") | |
| .lat(50.45) | |
| .lng(30.52) | |
| .addressUk("New Address Ua") | |
| dto.setCategoryId(1L); | |
| when(categoryService.findByName("Nonexistent Category")) | |
| .thenThrow(new NotFoundException("Category not found")); | |
| assertThrows(NotFoundException.class, () -> placeServiceImpl.update(dto, null, TestConst.USER_ID)); | |
| } | |
| void updateFromUIThrowsNotFoundExceptionForCategoryTest() { | |
| PlaceUpdateDto dto = new PlaceUpdateDto(); | |
| dto.setId(1L); | |
| dto.setCategoryId(1L); | |
| when(userRepo.findById(TestConst.USER_ID)).thenReturn(Optional.of(user)); | |
| when(placeRepo.findById(1L)).thenReturn(Optional.of(ModelUtils.getPlace())); | |
| when(categoryRepo.findById(1L)).thenReturn(Optional.empty()); | |
| assertThrows(NotFoundException.class, () -> placeServiceImpl.update(dto, null, TestConst.USER_ID)); | |
| } |
🤖 Prompt for AI Agents
In service/src/test/java/greencity/service/PlaceServiceImplTest.java around
lines 1003 to 1011, the test currently stubs
categoryService.findByName("Nonexistent Category") but the production
update(...) path now resolves categories by ID; update the stub to mock the
ID-based lookup instead (e.g., when(categoryService.findById(1L)).thenThrow(new
NotFoundException("Category not found"))), keep dto.setCategoryId(1L) as-is, and
ensure the assertThrows still calls placeServiceImpl.update(dto, null,
TestConst.USER_ID).
| void update_successfullyUpdatesPlace() { | ||
| Place existingPlace = ModelUtils.getPlace(); | ||
| // given | ||
| PlaceUpdateDto dto = PlaceUpdateDto.builder() | ||
| .id(existingPlace.getId()) | ||
| .name("New Place") | ||
| .categoryId(category.getId()) | ||
| .openingHoursList(Set.of(ModelUtils.getOpeningHoursDto())) | ||
| .build(); | ||
| when(categoryService.findByName("Test Category")).thenReturn(categoryDtoResponse); | ||
| when(placeRepo.findById(1L)).thenReturn(Optional.of(genericEntity1)); | ||
| when(userRepo.findById(user.getId())).thenReturn(Optional.of(user)); | ||
| when(locationService.findById(1L)).thenReturn(locationVO); | ||
| List<GeocodingResult> geocodingResults = new ArrayList<>(); | ||
| GeocodingResult ukrLang = new GeocodingResult(); | ||
| ukrLang.formattedAddress = "New Address"; | ||
| ukrLang.geometry = new Geometry(); | ||
| ukrLang.geometry.location = new LatLng(50.45, 30.52); | ||
| GeocodingResult engLang = new GeocodingResult(); | ||
| engLang.formattedAddress = "New Address Ua"; | ||
| engLang.geometry = new Geometry(); | ||
| engLang.geometry.location = new LatLng(50.45, 30.52); | ||
| geocodingResults.add(ukrLang); | ||
| geocodingResults.add(engLang); | ||
| when(googleApiService.getResultFromGeoCode("New Address")).thenReturn(geocodingResults); | ||
| PlaceVO result = placeServiceImpl.updateFromUI(dto, images, user.getId()); | ||
|
|
||
| assertNotNull(result); | ||
| assertEquals("Updated Place", result.getName()); | ||
| verify(placeRepo, atLeastOnce()).save(any(Place.class)); | ||
| verify(photoRepo, times(1)).save(any(Photo.class)); | ||
| verify(locationService, times(1)).update(anyLong(), any(LocationVO.class)); | ||
|
|
||
| MultipartFile[] images = new MultipartFile[0]; | ||
|
|
||
| GeocodingResult geo1 = ModelUtils.getGeocodingResult().getFirst(); | ||
| GeocodingResult geo2 = ModelUtils.getGeocodingResult().getLast(); | ||
|
|
||
| when(userRepo.findById(1L)).thenReturn(Optional.of(user)); | ||
| when(placeRepo.findById(existingPlace.getId())).thenReturn(Optional.of(existingPlace)); | ||
| when(categoryRepo.findById(category.getId())).thenReturn(Optional.of(category)); | ||
| when(googleApiService.getResultFromGeoCode("New Place")) | ||
| .thenReturn(List.of(geo1, geo2)); | ||
|
|
||
| placeService.update(dto, images, 1L); | ||
|
|
||
| verify(placeRepo).save(argThat(place -> place.getId().equals(place.getId()) && | ||
| place.getName().equals(place.getName()) && | ||
| place.getOpeningHoursList().size() == 1)); | ||
| } |
There was a problem hiding this comment.
Fix assertion and geocoding target in update_successfullyUpdatesPlace.
- Geocoding should use address, not name.
- The verify lambda compares the same object to itself; assert against expected values.
Place existingPlace = ModelUtils.getPlace();
// given
PlaceUpdateDto dto = PlaceUpdateDto.builder()
.id(existingPlace.getId())
.name("New Place")
.categoryId(category.getId())
+ .address("Kyiv")
.openingHoursList(Set.of(ModelUtils.getOpeningHoursDto()))
.build();
MultipartFile[] images = new MultipartFile[0];
GeocodingResult geo1 = ModelUtils.getGeocodingResult().getFirst();
GeocodingResult geo2 = ModelUtils.getGeocodingResult().getLast();
when(userRepo.findById(1L)).thenReturn(Optional.of(user));
when(placeRepo.findById(existingPlace.getId())).thenReturn(Optional.of(existingPlace));
when(categoryRepo.findById(category.getId())).thenReturn(Optional.of(category));
- when(googleApiService.getResultFromGeoCode("New Place"))
+ when(googleApiService.getResultFromGeoCode("Kyiv"))
.thenReturn(List.of(geo1, geo2));
placeService.update(dto, images, 1L);
- verify(placeRepo).save(argThat(place -> place.getId().equals(place.getId()) &&
- place.getName().equals(place.getName()) &&
- place.getOpeningHoursList().size() == 1));
+ verify(placeRepo).save(argThat(p ->
+ p.getId().equals(existingPlace.getId()) &&
+ p.getName().equals("New Place") &&
+ p.getCategory().equals(category) &&
+ p.getOpeningHoursList().size() == 1));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| void update_successfullyUpdatesPlace() { | |
| Place existingPlace = ModelUtils.getPlace(); | |
| // given | |
| PlaceUpdateDto dto = PlaceUpdateDto.builder() | |
| .id(existingPlace.getId()) | |
| .name("New Place") | |
| .categoryId(category.getId()) | |
| .openingHoursList(Set.of(ModelUtils.getOpeningHoursDto())) | |
| .build(); | |
| when(categoryService.findByName("Test Category")).thenReturn(categoryDtoResponse); | |
| when(placeRepo.findById(1L)).thenReturn(Optional.of(genericEntity1)); | |
| when(userRepo.findById(user.getId())).thenReturn(Optional.of(user)); | |
| when(locationService.findById(1L)).thenReturn(locationVO); | |
| List<GeocodingResult> geocodingResults = new ArrayList<>(); | |
| GeocodingResult ukrLang = new GeocodingResult(); | |
| ukrLang.formattedAddress = "New Address"; | |
| ukrLang.geometry = new Geometry(); | |
| ukrLang.geometry.location = new LatLng(50.45, 30.52); | |
| GeocodingResult engLang = new GeocodingResult(); | |
| engLang.formattedAddress = "New Address Ua"; | |
| engLang.geometry = new Geometry(); | |
| engLang.geometry.location = new LatLng(50.45, 30.52); | |
| geocodingResults.add(ukrLang); | |
| geocodingResults.add(engLang); | |
| when(googleApiService.getResultFromGeoCode("New Address")).thenReturn(geocodingResults); | |
| PlaceVO result = placeServiceImpl.updateFromUI(dto, images, user.getId()); | |
| assertNotNull(result); | |
| assertEquals("Updated Place", result.getName()); | |
| verify(placeRepo, atLeastOnce()).save(any(Place.class)); | |
| verify(photoRepo, times(1)).save(any(Photo.class)); | |
| verify(locationService, times(1)).update(anyLong(), any(LocationVO.class)); | |
| MultipartFile[] images = new MultipartFile[0]; | |
| GeocodingResult geo1 = ModelUtils.getGeocodingResult().getFirst(); | |
| GeocodingResult geo2 = ModelUtils.getGeocodingResult().getLast(); | |
| when(userRepo.findById(1L)).thenReturn(Optional.of(user)); | |
| when(placeRepo.findById(existingPlace.getId())).thenReturn(Optional.of(existingPlace)); | |
| when(categoryRepo.findById(category.getId())).thenReturn(Optional.of(category)); | |
| when(googleApiService.getResultFromGeoCode("New Place")) | |
| .thenReturn(List.of(geo1, geo2)); | |
| placeService.update(dto, images, 1L); | |
| verify(placeRepo).save(argThat(place -> place.getId().equals(place.getId()) && | |
| place.getName().equals(place.getName()) && | |
| place.getOpeningHoursList().size() == 1)); | |
| } | |
| void update_successfullyUpdatesPlace() { | |
| Place existingPlace = ModelUtils.getPlace(); | |
| // given | |
| PlaceUpdateDto dto = PlaceUpdateDto.builder() | |
| .id(existingPlace.getId()) | |
| .name("New Place") | |
| .categoryId(category.getId()) | |
| .address("Kyiv") | |
| .openingHoursList(Set.of(ModelUtils.getOpeningHoursDto())) | |
| .build(); | |
| MultipartFile[] images = new MultipartFile[0]; | |
| GeocodingResult geo1 = ModelUtils.getGeocodingResult().getFirst(); | |
| GeocodingResult geo2 = ModelUtils.getGeocodingResult().getLast(); | |
| when(userRepo.findById(1L)).thenReturn(Optional.of(user)); | |
| when(placeRepo.findById(existingPlace.getId())).thenReturn(Optional.of(existingPlace)); | |
| when(categoryRepo.findById(category.getId())).thenReturn(Optional.of(category)); | |
| when(googleApiService.getResultFromGeoCode("Kyiv")) | |
| .thenReturn(List.of(geo1, geo2)); | |
| placeService.update(dto, images, 1L); | |
| verify(placeRepo).save(argThat(p -> | |
| p.getId().equals(existingPlace.getId()) && | |
| p.getName().equals("New Place") && | |
| p.getCategory().equals(category) && | |
| p.getOpeningHoursList().size() == 1)); | |
| } |
🤖 Prompt for AI Agents
In service/src/test/java/greencity/service/PlaceServiceImplTest.java around
lines 1014-1040, the test incorrectly calls
googleApiService.getResultFromGeoCode with the place name and the verify lambda
compares the same place instance to itself; change the geocoding call to use the
DTO's address (e.g., dto.getAddress()) instead of "New Place", and update the
verify assertion to compare saved Place fields to expected values (e.g.,
place.getId().equals(dto.getId()), place.getName().equals("New Place"), and
place.getOpeningHoursList().size() == 1) so the test verifies actual changes
rather than self-comparisons.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
service/src/test/java/greencity/ModelUtils.java (1)
2138-2146: Builder-based DTO is now underfilled (no name/address).The builder variant returns an
AddPlaceDtowith onlycategoryIdand opening hours. The upstream refactor removedplaceName/locationNamebut replaced them with mandatorynameandaddress. Leaving both unset yields DTOs that fail controller/service validation and breaks any tests that call this helper.Please set
nameandaddress(and any other new required primitives) on the builder.public static AddPlaceDto getAddPlaceDto() { return AddPlaceDto.builder() .categoryId(1L) -// .placeName("test") -// .locationName("Test") + .name("Test") + .address("Kyiv") .openingHoursList(Set.of(OpeningHoursDto.builder() .openTime(LocalTime.now()) .closeTime(LocalTime.now())
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
service/src/main/java/greencity/service/PlaceServiceImpl.java(3 hunks)service/src/test/java/greencity/ModelUtils.java(3 hunks)service/src/test/java/greencity/service/PlaceServiceImplTest.java(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-31T08:55:30.651Z
Learnt from: urio999
PR: ita-social-projects/GreenCity#8082
File: service/src/main/java/greencity/mapping/events/EventResponseDtoMapper.java:0-0
Timestamp: 2025-01-31T08:55:30.651Z
Learning: In the EventResponseDtoMapper, null checks for address should use Optional or null-safe operators instead of assertions to ensure runtime safety, as assertions can be disabled with the -da flag.
Applied to files:
service/src/main/java/greencity/service/PlaceServiceImpl.java
🧬 Code graph analysis (2)
service/src/test/java/greencity/service/PlaceServiceImplTest.java (1)
service/src/test/java/greencity/ModelUtils.java (1)
ModelUtils(301-3764)
service/src/main/java/greencity/service/PlaceServiceImpl.java (1)
core/src/main/resources/static/scripts/management/places/buttonsAJAX.js (6)
place(138-143)place(149-155)i(57-57)openTime(239-239)closeTime(240-240)openingHours(271-271)
🪛 GitHub Actions: CI/CD GreenCity
service/src/main/java/greencity/service/PlaceServiceImpl.java
[error] 568-572: Cannot find symbol: class SearchCriteria at line 568 and class PlaceSearchSpecification at line 572.
[error] Compilation failed due to missing classes SearchCriteria and PlaceSearchSpecification.
🔇 Additional comments (7)
service/src/main/java/greencity/service/PlaceServiceImpl.java (3)
624-633: Geocode using the address, not the namePassing the place name into
buildLocationsends the wrong signal to the geocoder and can easily produce mismatched coordinates (or fail entirely for generic names). The DTO already carries the address—please use it here.- Location location = buildLocation(dto.getName()); + Location location = buildLocation(dto.getAddress());
697-704: Guard against short geocoding responses
geocodingResults.get(1)throws whenever Google only returns one match (quite common for precise addresses). Add a safe fallback (and ideally fetch per locale) so the update/save flow doesn’t 500 on valid input.- GeocodingResult ukrLang = geocodingResults.getFirst(); - GeocodingResult engLang = geocodingResults.get(1); + GeocodingResult ukrLang = geocodingResults.getFirst(); + GeocodingResult engLang = + geocodingResults.size() > 1 ? geocodingResults.get(1) : ukrLang;
708-716: Handle null opening-hours payloads
mapOpeningHourscurrently NPEs when the client omitsopeningHoursList. Treat null/empty as “no hours” so create/update remains resilient.- private Set<OpeningHours> mapOpeningHours(Set<OpeningHoursDto> dtos) { - return dtos.stream() + private Set<OpeningHours> mapOpeningHours(Set<OpeningHoursDto> dtos) { + if (CollectionUtils.isEmpty(dtos)) { + return new HashSet<>(); + } + return dtos.stream() .map(i -> OpeningHours.builder() .openTime(i.getOpenTime()) .closeTime(i.getCloseTime()) .weekDay(i.getWeekDay()) .build()) .collect(Collectors.toSet()); }service/src/test/java/greencity/service/PlaceServiceImplTest.java (3)
1003-1011: Stub category lookup the same way production code does
update(...)now resolves categories viacategoryRepo.findById, so this test never hits your stub and instead fails on the missing user. Please align the mocks so you actually assert the “category not found” path.- when(categoryService.findByName("Nonexistent Category")) - .thenThrow(new NotFoundException("Category not found")); + when(userRepo.findById(TestConst.USER_ID)).thenReturn(Optional.of(user)); + when(placeRepo.findById(1L)).thenReturn(Optional.of(ModelUtils.getPlace())); + when(categoryRepo.findById(1L)).thenReturn(Optional.empty());
1014-1039: Bring the happy-path update test in line with the new contractThe service should geocode the address—not the name—and the current verification compares the object to itself, so the assertion never fails. Set the DTO address, stub geocoding with that string, and assert the persisted fields against expected values.
PlaceUpdateDto dto = PlaceUpdateDto.builder() .id(existingPlace.getId()) .name("New Place") + .address("Kyiv") .categoryId(category.getId()) .openingHoursList(Set.of(ModelUtils.getOpeningHoursDto())) .build(); @@ - when(googleApiService.getResultFromGeoCode("New Place")) + when(googleApiService.getResultFromGeoCode("Kyiv")) .thenReturn(List.of(geo1, geo2)); @@ - verify(placeRepo).save(argThat(place -> place.getId().equals(place.getId()) && - place.getName().equals(place.getName()) && - place.getOpeningHoursList().size() == 1)); + verify(placeRepo).save(argThat(place -> + place.getId().equals(existingPlace.getId()) + && place.getName().equals("New Place") + && place.getCategory().equals(category) + && place.getOpeningHoursList().size() == 1));
1078-1093: Match the geocoding-empty test to the service behaviourOnce the service geocodes the address, this test needs to set and stub the address as well; otherwise it never reaches the empty-geocode branch you’re trying to cover.
PlaceUpdateDto dto = PlaceUpdateDto.builder() .id(existingPlace.getId()) .categoryId(category.getId()) - .name("Bad Place") + .name("Bad Place") + .address("Bad Place") .build();service/src/test/java/greencity/ModelUtils.java (1)
3478-3481: Ignore thetypeIdreference—CategoryDto’s fourth constructor argument isparentCategoryId, and the existing fixture value (1L) already matches that API.Likely an incorrect or invalid review comment.
| public static AddPlaceDto getPlaceAddDto() { | ||
| AddPlaceDto addPlaceDto = new AddPlaceDto(); | ||
| addPlaceDto.setName("Test"); | ||
| CategoryDto category = new CategoryDto(); | ||
| category.setNameEn("category"); | ||
| placeAddDto.setCategory(category); | ||
| placeAddDto.setLocation(getLocationAddressAndGeoDto()); | ||
| addPlaceDto.setCategoryId(1L); | ||
| HashSet<OpeningHoursDto> openingHoursDtos = new HashSet<>(); | ||
| openingHoursDtos.add(getOpeningHoursDto()); | ||
| placeAddDto.setOpeningHoursList(openingHoursDtos); | ||
| addPlaceDto.setOpeningHoursList(openingHoursDtos); | ||
| HashSet<DiscountValueDto> discountValueDtos = new HashSet<>(); | ||
| discountValueDtos.add(getDiscountValueDto()); | ||
| placeAddDto.setDiscountValues(discountValueDtos); | ||
| return placeAddDto; | ||
| return addPlaceDto; |
There was a problem hiding this comment.
Missing address assignment and stale discount handling in getPlaceAddDto().
The API now expects AddPlaceDto to carry a plain address string and no longer accepts discount payloads. This helper neither sets address nor attaches the opening hours / other fields that downstream validation still requires, and it keeps constructing discountValueDtos that are never written anywhere. Tests that rely on this factory will keep producing DTOs rejected by the updated service flow.
Please populate the new address field (e.g., "Kyiv" to mirror other fixtures), drop the obsolete DiscountValueDto set entirely, and remove the unused CategoryDto variable.
public static AddPlaceDto getPlaceAddDto() {
AddPlaceDto addPlaceDto = new AddPlaceDto();
addPlaceDto.setName("Test");
- CategoryDto category = new CategoryDto();
- category.setNameEn("category");
addPlaceDto.setCategoryId(1L);
+ addPlaceDto.setAddress("Kyiv");
HashSet<OpeningHoursDto> openingHoursDtos = new HashSet<>();
openingHoursDtos.add(getOpeningHoursDto());
addPlaceDto.setOpeningHoursList(openingHoursDtos);
- HashSet<DiscountValueDto> discountValueDtos = new HashSet<>();
- discountValueDtos.add(getDiscountValueDto());
return addPlaceDto;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| public static AddPlaceDto getPlaceAddDto() { | |
| AddPlaceDto addPlaceDto = new AddPlaceDto(); | |
| addPlaceDto.setName("Test"); | |
| CategoryDto category = new CategoryDto(); | |
| category.setNameEn("category"); | |
| placeAddDto.setCategory(category); | |
| placeAddDto.setLocation(getLocationAddressAndGeoDto()); | |
| addPlaceDto.setCategoryId(1L); | |
| HashSet<OpeningHoursDto> openingHoursDtos = new HashSet<>(); | |
| openingHoursDtos.add(getOpeningHoursDto()); | |
| placeAddDto.setOpeningHoursList(openingHoursDtos); | |
| addPlaceDto.setOpeningHoursList(openingHoursDtos); | |
| HashSet<DiscountValueDto> discountValueDtos = new HashSet<>(); | |
| discountValueDtos.add(getDiscountValueDto()); | |
| placeAddDto.setDiscountValues(discountValueDtos); | |
| return placeAddDto; | |
| return addPlaceDto; | |
| public static AddPlaceDto getPlaceAddDto() { | |
| AddPlaceDto addPlaceDto = new AddPlaceDto(); | |
| addPlaceDto.setName("Test"); | |
| addPlaceDto.setCategoryId(1L); | |
| addPlaceDto.setAddress("Kyiv"); | |
| HashSet<OpeningHoursDto> openingHoursDtos = new HashSet<>(); | |
| openingHoursDtos.add(getOpeningHoursDto()); | |
| addPlaceDto.setOpeningHoursList(openingHoursDtos); | |
| return addPlaceDto; | |
| } |
🤖 Prompt for AI Agents
In service/src/test/java/greencity/ModelUtils.java around lines 1151 to 1162,
the test factory getPlaceAddDto() must be updated to match the API changes:
remove the unused CategoryDto local variable, stop constructing or attaching any
DiscountValueDto collection, and set the new plain address field on AddPlaceDto
(use "Kyiv" to match other fixtures); keep the openingHoursList assignment as-is
and ensure addPlaceDto is returned with the address populated and no discount
payloads.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
core/src/main/resources/templates/core/management_places.html (2)
469-471: Remove Google Maps loader or guard initMap; missing #map causes runtime errorThere is no element with id="map" in this template, yet the Google Maps API loads with callback=initMap. This will throw when constructing the map and when trying to write to removed latitude/longitude inputs.
Apply this diff to remove the loader (preferred if map picking is no longer used):
-<script defer - src="https://maps.googleapis.com/maps/api/js?key=AIzaSyB3xs7Kczo46LFcQRFKPMdrE0lU4qsR_S4&callback=initMap"> -</script>If map is still needed elsewhere, add a null‑guard inside initMap in JS and reintroduce the #map container and lat/lng inputs.
325-331: Fix address error span id and relax validation for i18n
- Align the error span id with backend field name “address” to display server‑side validation.
- Current pattern blocks non‑Latin characters and common address symbols; remove it to avoid rejecting valid addresses.
Apply this diff:
- <input class="form-control" id="address" name="address" type="text" - pattern="^[A-Za-z0-9\s,.-]+$" - title="Please enter a valid address" - required> - <span class="errorSpan" th:id="errorModalSavelocation"></span> + <input class="form-control" id="address" name="address" type="text" required> + <span class="errorSpan" th:id="errorModalSaveaddress"></span>core/src/main/resources/static/scripts/management/places/buttonsAJAX.js (3)
181-216: Surface server validation errors from 4xx responses (field-level mapping)With endpoints returning 204 on success and 400 on validation failures, errors won’t appear in the UI. Parse xhr.responseText and map any errors[] to spans using getErrorSpanId.
Apply this diff:
error: function (xhr, status, error) { - console.error('XHR Status: ' + xhr.status); - console.error('Error: ' + error); - console.error('Response Text: ' + xhr.responseText); - - let errorMessage = `Error status: ${xhr.status} - ${error}`; - if (xhr.responseText) { - try { - const response = JSON.parse(xhr.responseText); - errorMessage += `\nMessage: ${response.message || 'Unknown error'}`; - } catch (e) { - errorMessage += `\nMessage: ${xhr.responseText}`; - } - } - - alert(errorMessage); + // Try to display field errors first + try { + const resp = xhr.responseText ? JSON.parse(xhr.responseText) : null; + if (resp && Array.isArray(resp.errors) && resp.errors.length) { + clearAllErrorsSpan(); + resp.errors.forEach(function (el) { + $(document.getElementById(getErrorSpanId(el.fieldName))).text(el.fieldError).show(); + }); + return; + } + } catch (e) { /* fall through to alert */ } + // Fallback alert + const base = `Error status: ${xhr.status} - ${error}`; + const msg = xhr.responseText ? `${base}\nMessage: ${xhr.responseText}` : base; + alert(msg); }
221-225: Align client-side validation spans with backend field namesUpdate error targets to match server fields “address” and “categoryId”.
Apply this diff:
- const requiredFields = [ - { id: 'address', errorId: 'errorModalSavelocation', message: messages["greenCity.places.page.add.address"] }, - { id: 'placeName', errorId: 'errorModalSavename', message: messages["greenCity.places.page.add.place.name"] }, - { id: 'category', errorId: 'errorModalSavecategory', message: messages["greenCity.places.page.add.category"] }, - ]; + const requiredFields = [ + { id: 'address', errorId: 'errorModalSaveaddress', message: messages["greenCity.places.page.add.address"] }, + { id: 'placeName', errorId: 'errorModalSavename', message: messages["greenCity.places.page.add.place.name"] }, + { id: 'category', errorId: 'errorModalSavecategoryId', message: messages["greenCity.places.page.add.category"] }, + ];
15-30: Guard initMap when map UI is absent; avoid null dereferencesPage has no #map or lat/lng inputs after the refactor. Add a null guard to prevent runtime errors.
Apply this diff:
function initMap() { - const mapCenter = {lat: 49.842957, lng: 24.031111}; - map = new google.maps.Map(document.getElementById("map"), { + const mapEl = document.getElementById("map"); + if (!mapEl) { return; } + const mapCenter = {lat: 49.842957, lng: 24.031111}; + map = new google.maps.Map(mapEl, { zoom: 12, center: mapCenter }); @@ - addMarker(event.latLng); - document.getElementById('latitude').value = event.latLng.lat(); - document.getElementById('longitude').value = event.latLng.lng(); + addMarker(event.latLng); + const latEl = document.getElementById('latitude'); + const lngEl = document.getElementById('longitude'); + if (latEl && lngEl) { + latEl.value = event.latLng.lat(); + lngEl.value = event.latLng.lng(); + } });
🧹 Nitpick comments (10)
service-api/src/main/java/greencity/security/jwt/JwtTool.java (1)
127-133: Avoid redundantgetCookies()invocationWe still call
request.getCookies()twice—once to assigncookiesand again inside the Optional. Reuse the array you already fetched to keep the code tidy and avoid extra servlet calls.- Cookie[] cookies = request.getCookies(); - - String accessToken = Optional.ofNullable(request.getCookies()).stream().flatMap(Arrays::stream) + Cookie[] cookies = request.getCookies(); + String accessToken = Optional.ofNullable(cookies).stream().flatMap(Arrays::stream) .filter(c -> c.getName().equals("accessToken")) .findFirst() .map(Cookie::getValue).orElse(null);service-api/src/test/java/greencity/security/jwt/JwtToolTest.java (2)
164-180: Prefer usingHttpHeaders.AUTHORIZATIONfor clarity.Re-using the Spring constant keeps these tests consistent with the rest of the class and avoids string literals for header names.
- when(webRequest.getHeader("Authorization")).thenReturn(null); + when(webRequest.getHeader(HttpHeaders.AUTHORIZATION)).thenReturn(null);
182-197: Consider covering the empty-cookie array case.An additional test where
getCookies()returns an empty array (instead ofnull) would harden the behaviour and guard against regressions if the servlet container supplies a non-null-but-empty array.core/src/main/resources/templates/core/management_places.html (2)
403-405: Optional: Rename file input to ‘images’ for consistencyThe JS appends files under “images”. Renaming the input’s name reduces duplicate/unexpected parts on the request.
Apply this diff:
- <input type="file" class="form-control" name="imagePath" id="creationFile" placeholder="ferf"> + <input type="file" class="form-control" name="images" id="creationFile" placeholder="ferf">
320-322: Remove dead lat/lng/addressUa remnants or restore end‑to‑end flowThese hidden fields were removed, but related JS (initMap) still attempts to set them, causing errors if map is present. Align the stack by removing map code or restoring the inputs.
core/src/main/resources/static/scripts/management/places/buttonsAJAX.js (3)
336-341: Fix category reset in modal#empty-category doesn’t exist. Reset the select to its first option.
Apply this diff:
function clearEditModal() { $('input[name=day]').prop('checked', false); $('#addPlaceModal').find('input').not('input[name=status]').not('#submitAddBtn').val(''); - $('#empty-category').prop("selected", true); + $('#category').prop('selectedIndex', 0); deleteMarkers(); }
391-405: Remove unused page param mutationparams.page isn’t defined; this block is dead code.
Apply this diff:
- if (params.page !== null) { - params.page = "0"; - }
509-517: Remove unused getSelectedStatus()No radio named ‘status’ exists; function is unused.
Apply this diff:
-function getSelectedStatus() { - const radios = document.getElementsByName('status'); - for (const radio of radios) { - if (radio.checked) { - return radio.value; - } - } - return ''; -}core/src/main/java/greencity/controller/PlaceController.java (2)
65-75: Update Javadoc to reflect multipart-only updateDoc says multipart or JSON, but mapping consumes only multipart. Adjust to avoid misleading clients.
Apply this diff:
- * <li><b>Content-Type:</b> {@code multipart/form-data} or - * {@code application/json}</li> + * <li><b>Content-Type:</b> {@code multipart/form-data}</li>
440-443: Remove no-op ResponseEntity call in delete()Method returns void; the ResponseEntity line has no effect.
Apply this diff:
public void delete(@PathVariable Long id) { placeService.deleteById(id); - ResponseEntity.status(HttpStatus.OK); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/src/main/java/greencity/controller/PlaceController.java(4 hunks)core/src/main/resources/static/scripts/management/places/buttonsAJAX.js(4 hunks)core/src/main/resources/templates/core/management_places.html(2 hunks)service-api/src/main/java/greencity/security/jwt/JwtTool.java(2 hunks)service-api/src/test/java/greencity/security/jwt/JwtToolTest.java(2 hunks)
🧰 Additional context used
🪛 GitHub Actions: CI/CD GreenCity
service-api/src/test/java/greencity/security/jwt/JwtToolTest.java
[error] 1-1: File has not been previously formatted. Please run 'mvn -f service-api net.revelc.code.formatter:formatter-maven-plugin:2.23.0:format' before validating.
🔇 Additional comments (5)
service-api/src/test/java/greencity/security/jwt/JwtToolTest.java (1)
145-162: Cookie fallback coverage looks solid.Nice job validating that the extractor correctly switches to the cookie path when the header is absent—the assertions mirror the production logic well.
core/src/main/resources/templates/core/management_places.html (2)
338-340: Confirm status handling; consider removing editable fieldIf status is set server‑side for new places, this readonly input may be redundant and confusing. Consider dropping it from the form or making it a hidden field.
342-349: Add category error span and map to server field “categoryId”Without an error span here, neither client nor server validation can surface errors to the user. Backend reports field “categoryId”.
Apply this diff:
<select class="form-control" id="category" name="category"> <option th:each="category : ${categoryList}" th:text="${category.getNameEn()}" th:value="${category.getId()}"> </option> </select> + <span class="errorSpan" th:id="errorModalSavecategoryId"></span>core/src/main/resources/static/scripts/management/places/buttonsAJAX.js (1)
137-146: Verify URL for create/update matches backend changesPOST/PUT are sent to “/management/places”, while backend save is at “/place/v2/save” and update at “/place/update”. Ensure the management endpoint forwards to the new API and accepts the same part names: addPlaceDto/placeUpdateDto and images.
core/src/main/java/greencity/controller/PlaceController.java (1)
107-113: Update endpoint looks correct (multipart + @Valid + 204)Consumes multipart, validates DTO, injects userId, and delegates with optional images. LGTM.
|
BohdanLys
left a comment
BohdanLys
left a comment
There was a problem hiding this comment.
Fix SonarQube issues to maintain the quality of code
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (6)
service/src/test/java/greencity/service/PlaceServiceImplTest.java (3)
1003-1011: Broken test: stubs obsolete API (findByName) but update now uses categoryId lookup.This test won't fail for a missing category with the current flow. The service implementation calls
categoryRepo.findById(1L), notcategoryService.findByName(...). Update the stub to mock the repository lookup by ID.void updateFromUIThrowsNotFoundExceptionForCategoryTest() { PlaceUpdateDto dto = new PlaceUpdateDto(); dto.setId(1L); dto.setCategoryId(1L); + when(userRepo.findById(TestConst.USER_ID)).thenReturn(Optional.of(user)); + when(placeRepo.findById(1L)).thenReturn(Optional.of(ModelUtils.getPlace())); - when(categoryService.findByName("Nonexistent Category")) - .thenThrow(new NotFoundException("Category not found")); + when(categoryRepo.findById(1L)).thenReturn(Optional.empty()); + assertThrows(NotFoundException.class, () -> placeServiceImpl.update(dto, null, TestConst.USER_ID)); }
1038-1040: Fix self-comparison in verification assertion.The verify lambda compares the same object to itself (
place.getId().equals(place.getId())), which always passes regardless of whether the update worked. Compare against expected values instead.placeService.update(dto, images, 1L); - verify(placeRepo).save(argThat(place -> place.getId().equals(place.getId()) && - place.getName().equals(place.getName()) && + verify(placeRepo).save(argThat(place -> + place.getId().equals(existingPlace.getId()) && + place.getName().equals("New Place") && + place.getCategory().equals(category) && place.getOpeningHoursList().size() == 1));
1086-1090: Missing address field prevents geocoding mock from matching.The test mocks
getResultFromGeoCode("Bad Place")at line 1084, but the DTO doesn't set an address field. If the service callsgoogleApiService.getResultFromGeoCode(dto.getAddress()), the mock won't match (address will be null). Addaddress("Bad Place")to the DTO builder.PlaceUpdateDto dto = PlaceUpdateDto.builder() .id(existingPlace.getId()) .categoryId(category.getId()) .name("Bad Place") + .address("Bad Place") .build();core/src/test/java/greencity/webcontroller/ManagementPlacesControllerTest.java (2)
103-103: Use numeric type for categoryId in JSON payload.The test uses
"categoryId": "1"(string) instead of"categoryId": 1(number). While JSON parsing may coerce this, using the correct numeric type makes the test more strict and future-proof."name": "Тестове місце", "address": "Смиків, південна 7", - "categoryId": "1", + "categoryId": 1, "openingHoursList": [
136-139: Normalize id and categoryId to numeric types in update payload.Both
"id"and"categoryId"are specified as strings when they should be numeric values to match the DTO types strictly.{ - "id": "1", + "id": 1, "name": "Тестове місце", "address": "Смиків, південна 7", - "categoryId": "1", + "categoryId": 1, "openingHoursList": [service/src/test/java/greencity/ModelUtils.java (1)
1144-1156: Complete the getPlaceAddDto refactoring: remove unused code and add required address field.This helper has three issues that make it produce incomplete test data:
- Unused CategoryDto variable (lines 1147-1148): Created but never used—remove it.
- Missing address field: The new API requires
address, but it's not set. Tests using this helper may fail.- Orphaned DiscountValueDto set (lines 1153-1154): Created but never assigned to the DTO, suggesting incomplete refactoring from the discount removal.
public static AddPlaceDto getPlaceAddDto() { AddPlaceDto addPlaceDto = new AddPlaceDto(); addPlaceDto.setName("Test"); - CategoryDto category = new CategoryDto(); - category.setNameEn("category"); addPlaceDto.setCategoryId(1L); + addPlaceDto.setAddress("Kyiv"); HashSet<OpeningHoursDto> openingHoursDtos = new HashSet<>(); openingHoursDtos.add(getOpeningHoursDto()); addPlaceDto.setOpeningHoursList(openingHoursDtos); - HashSet<DiscountValueDto> discountValueDtos = new HashSet<>(); - discountValueDtos.add(getDiscountValueDto()); return addPlaceDto; }
🧹 Nitpick comments (5)
service-api/src/main/java/greencity/security/jwt/JwtTool.java (2)
17-17: Consider using explicit imports instead of wildcard.The wildcard import
java.util.*reduces code clarity and can lead to naming conflicts. Prefer explicit imports likejava.util.Arrays,java.util.Calendar,java.util.Collections, etc.Apply this change to use explicit imports:
-import java.util.*; +import java.util.Arrays; +import java.util.Calendar; +import java.util.Collections; +import java.util.Date; +import java.util.List; +import java.util.Optional; +import java.util.Set;
132-137: Eliminate unused variable and redundant method call.Line 132 declares
cookiesbut never uses it, and line 134 redundantly callsrequest.getCookies()again. This reduces code clarity and makes an unnecessary method call.Apply this diff to use the declared variable:
Cookie[] cookies = request.getCookies(); -String accessToken = Optional.ofNullable(request.getCookies()).stream().flatMap(Arrays::stream) +String accessToken = Optional.ofNullable(cookies).stream().flatMap(Arrays::stream) .filter(c -> c.getName().equals("accessToken")) .findFirst() .map(Cookie::getValue).orElse(null);service/src/test/java/greencity/service/PlaceServiceImplTest.java (1)
234-240: Remove debug println from test verification.The
System.out.println(place.getLocation())at line 235 pollutes test output and serves no verification purpose. Remove it to keep the test clean.verify(placeRepo).save(argThat(place -> { - System.out.println(place.getLocation()); return place.getName().equals("My Cafe") && place.getAuthor().equals(user) && place.getCategory().equals(category) && place.getOpeningHoursList().size() == 1; }));core/src/test/java/greencity/webcontroller/ManagementPlacesControllerTest.java (1)
151-156: Inconsistent variable naming: consider renaming for clarity.The variable is named
addPlaceDtobut it's used for an update operation and the multipart parameter name is"placeUpdateDto". Consider renaming the variable toplaceUpdateDtoorupdateDtofor consistency.- MockMultipartFile addPlaceDto = new MockMultipartFile( + MockMultipartFile placeUpdateDto = new MockMultipartFile( "placeUpdateDto", "", "application/json", (json) .getBytes()); this.mockMvc.perform(multipart(HttpMethod.PUT, "/management/places/") - .file(addPlaceDto) + .file(placeUpdateDto)service/src/test/java/greencity/ModelUtils.java (1)
2092-2094: Consider removing commented-out code.The commented lines for
placeNameandlocationNameappear to be remnants from the refactoring. If they're no longer needed, remove them to keep the codebase clean.public static AddPlaceDto getAddPlaceDto() { return AddPlaceDto.builder() .categoryId(1L) -// .placeName("test") -// .locationName("Test") .openingHoursList(Set.of(OpeningHoursDto.builder()
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
core/src/main/java/greencity/webcontroller/ManagementPlacesController.java(3 hunks)core/src/test/java/greencity/webcontroller/ManagementPlacesControllerTest.java(4 hunks)service-api/src/main/java/greencity/security/jwt/JwtTool.java(2 hunks)service-api/src/test/java/greencity/security/jwt/JwtToolTest.java(2 hunks)service/src/test/java/greencity/ModelUtils.java(3 hunks)service/src/test/java/greencity/service/NotificationServiceImplTest.java(3 hunks)service/src/test/java/greencity/service/PlaceServiceImplTest.java(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- core/src/main/java/greencity/webcontroller/ManagementPlacesController.java
- service/src/test/java/greencity/service/NotificationServiceImplTest.java
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-31T08:55:30.651Z
Learnt from: urio999
Repo: ita-social-projects/GreenCity PR: 8082
File: service/src/main/java/greencity/mapping/events/EventResponseDtoMapper.java:0-0
Timestamp: 2025-01-31T08:55:30.651Z
Learning: In the EventResponseDtoMapper, null checks for address should use Optional or null-safe operators instead of assertions to ensure runtime safety, as assertions can be disabled with the -da flag.
Applied to files:
service/src/test/java/greencity/service/PlaceServiceImplTest.javaservice/src/test/java/greencity/ModelUtils.java
🧬 Code graph analysis (1)
service/src/test/java/greencity/service/PlaceServiceImplTest.java (2)
service/src/test/java/greencity/ModelUtils.java (1)
ModelUtils(298-3682)service/src/test/java/greencity/TestConst.java (1)
TestConst(3-15)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
service-api/src/test/java/greencity/security/jwt/JwtToolTest.java (1)
150-202: Excellent test coverage for cookie-based JWT extraction.The three new test methods comprehensively cover the cookie extraction scenarios:
- Successful extraction when the
accessTokencookie is present- Exception handling when cookies exist but don't contain the token
- Exception handling when the cookies array is null
The tests follow the AAA pattern, use appropriate mocking, and verify both behavior and error messages.
service/src/test/java/greencity/service/PlaceServiceImplTest.java (2)
244-287: Good test coverage for save failure scenarios.The new tests effectively cover edge cases for the save flow: user not found, category not found, empty geocoding results, and duplicate place detection. This provides solid protection against regressions.
1044-1076: Solid coverage of update failure paths.These tests ensure that update operations properly validate user existence, place existence, and category existence before proceeding. Good defensive testing.
8 participants
GreenCity PR
Issue Links 📋
Summary by CodeRabbit
New Features
Refactor
Documentation
Tests