Request to join events 6222 #6807
Conversation
…osed event, getting all users requested for the event
…ll users requested for the event
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
WalkthroughThis pull request introduces comprehensive event request management functionality to the application. The changes span multiple layers of the system, including security configuration, controller endpoints, database schema, and service implementations. The new feature allows users to request joining events, with organizers having the ability to approve or decline these requests. The implementation includes robust error handling, notification mechanisms, and pagination support for requested users. Changes
Sequence DiagramsequenceDiagram
participant User
participant EventController
participant EventService
participant EventRepository
participant NotificationService
User->>EventController: Request to join event
EventController->>EventService: addToRequested(eventId, userEmail)
EventService->>EventRepository: Verify event and user
EventService->>EventRepository: Add user to requesters
EventService->>NotificationService: Notify event organizer
EventController->>User: Confirm request added
Poem
Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (20)
service-api/src/main/java/greencity/enums/NotificationType.java (1)
32-35: LGTM! Consider adding documentation for the new notification types.The new notification types are well-named and align perfectly with the event request management feature. To improve maintainability, consider adding Javadoc comments describing when each notification type is triggered.
Example documentation:
+ /** Notification sent when a user's request to join an event is accepted by the organizer */ EVENT_REQUEST_ACCEPTED, + /** Notification sent when a user's request to join an event is declined by the organizer */ EVENT_REQUEST_DECLINED, + /** Notification sent when a user is invited to join an event */ EVENT_INVITE;service-api/src/main/java/greencity/dto/user/UserForListDto.java (3)
17-22: Consider refining @EqualsAndHashCode implementationThe current implementation includes all fields in equals and hashCode calculations. Consider using
@EqualsAndHashCode(of = {"id", "email"})to:
- Improve performance by using only essential fields
- Prevent potential issues with mutable fields in hash-based collections
- Maintain object identity based on business keys
32-37: Consider adding temporal validation for registration dateThe
dateOfRegistrationfield might benefit from additional validation:
- Add
@PastOrPresentto ensure logical temporal consistency- Consider making it non-nullable since every user should have a registration date
- private LocalDateTime dateOfRegistration; + @NotNull + @PastOrPresent + private LocalDateTime dateOfRegistration;
44-45: Add size constraint for userCredo fieldConsider adding a maximum length constraint to prevent potential abuse through extremely long credo text.
- private String userCredo; + @Size(max = ServiceValidationConstants.MAX_CREDO_LENGTH) + private String userCredo;service-api/src/main/java/greencity/constant/EmailNotificationMessagesConstants.java (1)
30-37: LGTM! Clear and user-friendly notification messages.The messages are well-crafted, particularly the declined request message which maintains a positive tone by suggesting alternative engagement. Consider adding the event title to the declined message for better context.
public static final String JOIN_REQUEST_DECLINED_MESSAGE = - "While we can't confirm your attendance this time, there's another way to stay connected! Join our " - + "organizer's friend list for future updates and opportunities."; + "While we can't confirm your attendance for %s this time, there's another way to stay connected! Join our " + + "organizer's friend list for future updates and opportunities.";service-api/src/main/java/greencity/service/EventService.java (1)
214-251: Add parameter descriptions in Javadoc.The methods are well-documented, but consider adding
@paramdescriptions for better clarity. For example:/** * Method for approving request for joining the event. * + * @param eventId - ID of the event + * @param email - email of the user approving the request + * @param userId - ID of the user whose request is being approved * @author Olha Pitsyk. */ void approveRequest(Long eventId, String email, Long userId);core/src/test/java/greencity/controller/EventControllerTest.java (4)
707-715: Enhance test coverage for addToRequested endpointWhile the happy path is covered, consider adding test cases for:
- Invalid event ID
- Already requested event
- Non-existent user
@Test @SneakyThrows void addToRequestedTest() { Long eventId = 1L; mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/addToRequested/{eventId}", eventId) .principal(principal)) .andExpect(status().isOk()); verify(eventService).addToRequested(eventId, principal.getName()); + + // Test invalid event ID + mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/addToRequested/{eventId}", "invalid") + .principal(principal)) + .andExpect(status().isBadRequest()); + + // Test already requested event + doThrow(new BadRequestException(ErrorMessage.USER_HAS_ALREADY_ADDED_EVENT_TO_REQUESTED)) + .when(eventService).addToRequested(eventId, principal.getName()); + mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/addToRequested/{eventId}", eventId) + .principal(principal)) + .andExpect(status().isBadRequest()); }
717-725: Add error scenarios for removeFromRequested endpointSimilar to the previous test, consider adding test cases for:
- Event not found
- Event not in requested list
@Test @SneakyThrows void removeFromRequestedTest() { Long eventId = 1L; mockMvc.perform(delete(EVENTS_CONTROLLER_LINK + "/removeFromRequested/{eventId}", eventId) .principal(principal)) .andExpect(status().isOk()); verify(eventService).removeFromRequested(eventId, principal.getName()); + + // Test event not found + doThrow(new NotFoundException(ErrorMessage.EVENT_NOT_FOUND)) + .when(eventService).removeFromRequested(eventId, principal.getName()); + mockMvc.perform(delete(EVENTS_CONTROLLER_LINK + "/removeFromRequested/{eventId}", eventId) + .principal(principal)) + .andExpect(status().isNotFound()); }
727-736: Improve pagination testing in getRequestedUsers endpointThe current test should validate pagination behavior:
- Different page sizes
- Page navigation
- Empty result handling
@Test @SneakyThrows void getRequestedUsersTest() { Long eventId = 1L; Pageable pageable = PageRequest.of(0, 20); mockMvc.perform(get(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users", eventId) .principal(principal)) .andExpect(status().isOk()); verify(eventService).getRequestedUsers(eventId, principal.getName(), pageable); + + // Test different page sizes + mockMvc.perform(get(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users", eventId) + .param("size", "10") + .param("page", "1") + .principal(principal)) + .andExpect(status().isOk()); + + // Test empty result + when(eventService.getRequestedUsers(eventId, principal.getName(), pageable)) + .thenReturn(new PageableDto<>(Collections.emptyList(), 0L, 0, 0)); + mockMvc.perform(get(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users", eventId) + .principal(principal)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.content").isEmpty()); }
749-758: Reduce duplication in request handling testsConsider extracting common test setup and validation logic for approve/decline endpoints:
+private void testRequestHandling(String endpoint, ThrowingConsumer<MockHttpServletRequestBuilder> action) throws Exception { + Long eventId = 1L; + Long userId = 1L; + MockHttpServletRequestBuilder request = post(EVENTS_CONTROLLER_LINK + + "/{eventId}/requested-users/{userId}/" + endpoint, eventId, userId) + .principal(principal); + + action.accept(request); +} @Test @SneakyThrows void declineRequest() { - Long eventId = 1L; - Long userId = 1L; - mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users/{userId}/decline", eventId, userId) - .principal(principal)) - .andExpect(status().isOk()); - verify(eventService).declineRequest(eventId, principal.getName(), userId); + testRequestHandling("decline", request -> + mockMvc.perform(request) + .andExpect(status().isOk())); }core/src/main/java/greencity/controller/EventController.java (4)
498-515: Enhance API documentation and response handlingConsider the following improvements:
- Add more detailed API documentation including request/response examples
- Return a more informative response
@Operation(summary = "Add an event to requested") @ApiResponses(value = { @ApiResponse(responseCode = "200", description = HttpStatuses.OK), @ApiResponse(responseCode = "400", description = HttpStatuses.BAD_REQUEST), @ApiResponse(responseCode = "401", description = HttpStatuses.UNAUTHORIZED), - @ApiResponse(responseCode = "404", description = HttpStatuses.NOT_FOUND) + @ApiResponse(responseCode = "404", description = HttpStatuses.NOT_FOUND, + content = @Content(examples = @ExampleObject(value = "{\"message\": \"Event not found\"}"))), + @ApiResponse(responseCode = "409", description = "Event already requested", + content = @Content(examples = @ExampleObject(value = "{\"message\": \"Already requested\"}"))) }) @PostMapping("/addToRequested/{eventId}") public ResponseEntity<Object> addToRequested(@PathVariable Long eventId, @Parameter(hidden = true) Principal principal) { eventService.addToRequested(eventId, principal.getName()); - return ResponseEntity.status(HttpStatus.OK).build(); + return ResponseEntity.status(HttpStatus.OK) + .body(Map.of("message", "Successfully added to requested events")); }
517-534: Improve RESTful design of request endpointsThe current path structure could be more RESTful:
-@DeleteMapping("/removeFromRequested/{eventId}") +@DeleteMapping("/{eventId}/requests") public ResponseEntity<Object> removeFromRequested(@PathVariable Long eventId, @Parameter(hidden = true) Principal principal) { eventService.removeFromRequested(eventId, principal.getName()); - return ResponseEntity.status(HttpStatus.OK).build(); + return ResponseEntity.status(HttpStatus.OK) + .body(Map.of("message", "Successfully removed from requested events")); }Consider restructuring the request-related endpoints to follow RESTful conventions:
- POST
/events/{eventId}/requests(add request)- DELETE
/events/{eventId}/requests(remove request)- GET
/events/{eventId}/requests(get requests)
557-575: Add request validation and improve response contentEnhance the endpoint with:
- Parameter validation
- Meaningful response body
- More specific error documentation
@PostMapping("/{eventId}/requested-users/{userId}/approve") public ResponseEntity<Object> approveRequest( - @PathVariable Long eventId, - @PathVariable Long userId, + @PathVariable @Positive Long eventId, + @PathVariable @Positive Long userId, @Parameter(hidden = true) Principal principal) { eventService.approveRequest(eventId, principal.getName(), userId); - return ResponseEntity.status(HttpStatus.OK).build(); + return ResponseEntity.status(HttpStatus.OK) + .body(Map.of( + "message", "Request approved successfully", + "eventId", eventId, + "userId", userId + )); }
577-595: Reduce duplication in request handling endpointsConsider extracting common logic for approve/decline endpoints:
+private ResponseEntity<Object> handleRequestResponse( + Long eventId, + Long userId, + String action, + Consumer<Long> serviceMethod) { + serviceMethod.accept(eventId); + return ResponseEntity.status(HttpStatus.OK) + .body(Map.of( + "message", String.format("Request %s successfully", action), + "eventId", eventId, + "userId", userId + )); +} @PostMapping("/{eventId}/requested-users/{userId}/decline") public ResponseEntity<Object> declineRequest( - @PathVariable Long eventId, - @PathVariable Long userId, + @PathVariable @Positive Long eventId, + @PathVariable @Positive Long userId, @Parameter(hidden = true) Principal principal) { - eventService.declineRequest(eventId, principal.getName(), userId); - return ResponseEntity.status(HttpStatus.OK).build(); + return handleRequestResponse( + eventId, userId, "declined", + id -> eventService.declineRequest(id, principal.getName(), userId)); }service/src/test/java/greencity/service/EventServiceImplTest.java (2)
1586-1718: Good test coverage for request management functionality.The test methods thoroughly cover the core scenarios for managing event requests. The tests validate both successful operations and error cases.
However, consider adding these test cases to improve coverage:
- Test concurrent request handling
- Test request limits per event
- Test request state transitions
1719-1784: Enhance request approval test coverage.The approval tests validate basic functionality but could be more comprehensive.
Consider adding tests for:
@Test void approveRequestWithConcurrentModification() { // Test concurrent approval of the same request } @Test void approveRequestWithCapacityLimit() { // Test approval when event reaches capacity }dao/src/main/resources/db/changelog/logs/ch-add-table-events-requesters-Pitsyk.xml (1)
17-29: Consider adding an index for request lookups.While the foreign key constraints are properly defined, consider adding an index to optimize request lookups.
<createIndex tableName="events_requesters" indexName="idx_events_requesters_user"> <column name="user_id"/> </createIndex>dao/src/main/java/greencity/repository/UserRepo.java (1)
903-914: LGTM! Consider enhancing query readability.The implementation is correct and efficient. The native query properly joins the required tables and supports pagination.
Consider formatting the SQL query for better readability:
- @Query(nativeQuery = true, value = "SELECT users.* FROM users " - + "JOIN events_requesters ON users.id = events_requesters.user_id " - + "WHERE events_requesters.event_id = :eventId") + @Query(nativeQuery = true, value = """ + SELECT users.* + FROM users + JOIN events_requesters ON users.id = events_requesters.user_id + WHERE events_requesters.event_id = :eventId + """)service/src/main/java/greencity/service/EventServiceImpl.java (2)
889-906: Consider simplifying the stream operation.The implementation is correct, but the stream operation to filter requesters can be simplified.
- event.setRequesters(event.getRequesters() - .stream() - .filter(user -> !user.getId().equals(currentUser.getId())) - .collect(Collectors.toSet())); + event.getRequesters().remove(currentUser);
908-930: Consider adding database index for performance.The implementation correctly handles permissions and pagination. For better performance at scale:
Consider adding a composite index on the events_requesters table:
CREATE INDEX idx_event_requesters ON events_requesters(event_id, user_id);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
core/src/main/java/greencity/config/SecurityConfig.java(3 hunks)core/src/main/java/greencity/controller/EventController.java(1 hunks)core/src/test/java/greencity/controller/EventControllerTest.java(1 hunks)dao/src/main/java/greencity/entity/User.java(2 hunks)dao/src/main/java/greencity/entity/event/Event.java(3 hunks)dao/src/main/java/greencity/enums/NotificationType.java(1 hunks)dao/src/main/java/greencity/repository/UserRepo.java(1 hunks)dao/src/main/resources/db/changelog/db.changelog-master.xml(1 hunks)dao/src/main/resources/db/changelog/logs/ch-add-table-events-requesters-Pitsyk.xml(1 hunks)service-api/src/main/java/greencity/constant/EmailNotificationMessagesConstants.java(1 hunks)service-api/src/main/java/greencity/constant/ErrorMessage.java(1 hunks)service-api/src/main/java/greencity/dto/user/UserForListDto.java(1 hunks)service-api/src/main/java/greencity/enums/NotificationType.java(1 hunks)service-api/src/main/java/greencity/service/EventService.java(2 hunks)service/src/main/java/greencity/service/EventServiceImpl.java(4 hunks)service/src/main/java/greencity/service/NotificationServiceImpl.java(2 hunks)service/src/main/resources/notification.properties(1 hunks)service/src/main/resources/notification_ua.properties(1 hunks)service/src/test/java/greencity/ModelUtils.java(1 hunks)service/src/test/java/greencity/service/EventServiceImplTest.java(1 hunks)
🔇 Additional comments (20)
service-api/src/main/java/greencity/dto/user/UserForListDto.java (2)
1-16: Well-organized imports and package structure!The imports are logically grouped and include all necessary dependencies for validation, enums, and Lombok functionality.
23-31: Verify username length constraintsThe validation looks good, but let's verify the constant values being used for username length constraints.
✅ Verification successful
✓ Username length constraints are appropriate and consistently implemented
The validation constants (min=6, max=30 characters) are well-chosen and consistently applied across all user-related DTOs in the codebase. These constraints strike a good balance between usability and security.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for USERNAME length constants rg "USERNAME_[A-Z_]*LENGTH\s*=\s*\d+" -A 1Length of output: 469
Script:
#!/bin/bash # Search for other username validations or size constraints rg "username|userName" -i --type java -A 2 -B 2Length of output: 35890
dao/src/main/java/greencity/enums/NotificationType.java (1)
32-35: LGTM! Well-structured notification types for event requests.The new notification types are well-named and align perfectly with the event request management functionality.
dao/src/main/java/greencity/entity/event/Event.java (1)
36-36: LGTM! Well-implemented many-to-many relationship.The implementation follows JPA best practices with:
- Appropriate lazy loading
- Proper initialization of the Set
- Consistent naming of the join table
- Correct exclusion from equals/hashCode
Also applies to: 75-80
dao/src/main/java/greencity/entity/User.java (2)
193-194: Well-structured annotation updates!Good practice excluding the new
requestedEventscollection from equals/hashCode and toString methods to prevent potential recursion issues.Also applies to: 197-198
323-324: Properly configured bidirectional relationship!The ManyToMany relationship is well-defined with appropriate lazy loading strategy. Let's verify the corresponding mapping in the Event entity.
✅ Verification successful
✅ Bidirectional relationship verified and correctly implemented!
The Event entity properly defines the owner side of the relationship with:
- Correct join table configuration (
events_requesters)- Matching fetch strategy (LAZY)
- Proper initialization of the collection
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify the corresponding "requesters" mapping in Event entity ast-grep --pattern 'class Event { $$$ @ManyToMany $$$ private Set<User> requesters; $$$ }'Length of output: 102
Script:
#!/bin/bash # First find the Event class file fd -e java "Event.java" # Then examine its content if found fd -e java "Event.java" -x cat {}Length of output: 3616
service-api/src/main/java/greencity/constant/ErrorMessage.java (1)
178-182: Clear and comprehensive error messages!The new error messages are well-defined and follow the existing pattern. They cover all necessary scenarios for the event request functionality:
- Already requested events
- Events not in requested state
- User not requesting the event
- Event comment not found
service/src/main/java/greencity/service/NotificationServiceImpl.java (2)
272-272: Good categorization of the new notification type!EVENT_INVITE is appropriately added to the invites list, maintaining consistency with the notification system's design.
281-283: Well-organized notification types!The new event request notification types are correctly categorized as system notifications, maintaining a clean separation of concerns.
core/src/main/java/greencity/config/SecurityConfig.java (3)
272-272: Appropriate role-based access control!The GET endpoint for requested users is properly secured with the correct roles, maintaining consistent access control.
311-313: Well-structured endpoint security!The POST endpoints for approving, declining, and adding event requests are properly secured with appropriate roles.
409-410: Consistent security configuration!The DELETE endpoint for removing requests is properly secured, maintaining consistency with other similar endpoints in the application.
service/src/test/java/greencity/service/EventServiceImplTest.java (1)
1785-1851: Well-structured decline request tests.The decline request tests effectively validate permissions and state transitions.
service/src/test/java/greencity/ModelUtils.java (1)
1801-1801: LGTM! Test data properly initialized.The requesters field is correctly initialized with test data, maintaining consistency with the Event model changes.
dao/src/main/resources/db/changelog/logs/ch-add-table-events-requesters-Pitsyk.xml (1)
8-15: Well-structured table definition with appropriate constraints.The table definition follows best practices with proper column types and NOT NULL constraints.
service/src/main/resources/notification.properties (1)
52-57: Clear and user-friendly notification messages.The notification messages are well-written, consistent with existing messages, and provide clear communication to users.
dao/src/main/resources/db/changelog/db.changelog-master.xml (1)
263-263: LGTM! Properly structured changelog include.The new changelog file for events requesters is correctly included and follows the established naming convention.
service/src/main/java/greencity/service/EventServiceImpl.java (2)
870-887: LGTM! Method handles edge cases properly.The implementation includes proper validation and error handling. Good practice sending notifications to the event organizer.
932-958: LGTM! Comprehensive validation and proper notification handling.The implementation includes all necessary checks and properly handles the state transition from requester to attender.
service/src/main/resources/notification_ua.properties (1)
32-37: LGTM! Clear and consistent notification messages.The notification messages are well-written, properly encoded, and maintain consistency with the existing notification style.
| @NotNull | ||
| private UserStatus userStatus; | ||
|
|
||
| @NotNull | ||
| private Role role; | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
While the @NotNull validation is appropriate, I've identified some concerning issues in the codebase:
- Duplicate enum definitions exist in both
daoandservice-apimodules forUserStatusandRole - The
Roleenum values are inconsistent between modules:service-api: includesROLE_UBS_EMPLOYEEdao: missingROLE_UBS_EMPLOYEE
Consider:
- Consolidating enum definitions into a shared module to prevent duplication
- Ensuring consistent enum values across all modules to prevent potential runtime issues
🔗 Analysis chain
Verify enum completeness for UserStatus and Role
The validation looks good. Let's verify that all necessary status and role values are covered in the enums.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for UserStatus and Role enum definitions
echo "=== UserStatus enum values ==="
ast-grep --pattern 'enum UserStatus {
$$$
}'
echo "=== Role enum values ==="
ast-grep --pattern 'enum Role {
$$$
}'
Length of output: 221
Script:
#!/bin/bash
# First find relevant enum files
echo "=== Finding enum files ==="
fd -e java "UserStatus|Role"
echo -e "\n=== Searching for UserStatus enum ==="
rg "enum\s+UserStatus" -A 10
echo -e "\n=== Searching for Role enum ==="
rg "enum\s+Role" -A 10
Length of output: 2210
Script:
#!/bin/bash
# Check which Role enum is imported in UserForListDto
echo "=== Checking Role import in UserForListDto ==="
rg "import.*Role" service-api/src/main/java/greencity/dto/user/UserForListDto.java
Length of output: 216
| @Test | ||
| @SneakyThrows | ||
| void approveRequest() { | ||
| Long eventId = 1L; | ||
| Long userId = 1L; | ||
| mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users/{userId}/approve", eventId, userId) | ||
| .principal(principal)) | ||
| .andExpect(status().isOk()); | ||
| verify(eventService).approveRequest(eventId, principal.getName(), userId); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add security validation tests for approveRequest endpoint
Add test cases to verify:
- Only event organizer can approve requests
- Unauthorized access handling
- Invalid user ID handling
@Test
@SneakyThrows
void approveRequest() {
Long eventId = 1L;
Long userId = 1L;
mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users/{userId}/approve", eventId, userId)
.principal(principal))
.andExpect(status().isOk());
verify(eventService).approveRequest(eventId, principal.getName(), userId);
+
+ // Test unauthorized access
+ doThrow(new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION))
+ .when(eventService).approveRequest(eventId, principal.getName(), userId);
+ mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users/{userId}/approve", eventId, userId)
+ .principal(principal))
+ .andExpect(status().isForbidden());
}Committable suggestion skipped: line range outside the PR's diff.
| @Override | ||
| public void declineRequest(Long eventId, String email, Long userId) { | ||
| UserVO userVO = restClient.findByEmail(email); | ||
| User currentUser = modelMapper.map(userVO, User.class); | ||
| Event event = eventRepo.findById(eventId) | ||
| .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND)); | ||
|
|
||
| if (!Objects.equals(currentUser.getId(), event.getOrganizer().getId())) { | ||
| throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION); | ||
| } | ||
| if (event.getRequesters().stream().noneMatch(u -> Objects.equals(u.getId(), userId))) { | ||
| throw new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId); | ||
| } | ||
| User userToJoin = | ||
| userRepo.findById(userId) | ||
| .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_ID + userId)); | ||
|
|
||
| event | ||
| .setRequesters(event.getRequesters().stream().filter(u -> !Objects.equals(u.getId(), userId)) | ||
| .collect(Collectors.toSet())); | ||
|
|
||
| eventRepo.save(event); | ||
|
|
||
| userNotificationService.createNotification(userVO, modelMapper.map(userToJoin, UserVO.class), | ||
| NotificationType.EVENT_REQUEST_DECLINED, eventId, event.getTitle()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Extract common validation logic.
The implementation duplicates validation logic from approveRequest method.
Consider extracting common validation into a private method:
private void validateEventRequestOperation(Event event, User currentUser, Long userId) {
if (!Objects.equals(currentUser.getId(), event.getOrganizer().getId())) {
throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION);
}
if (event.getRequesters().stream().noneMatch(u -> Objects.equals(u.getId(), userId))) {
throw new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId);
}
}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
service/src/test/java/greencity/service/EventServiceImplTest.java (4)
1587-1630: Enhance assertions in request addition tests.While the tests cover the main scenarios, they could be more robust with additional assertions:
- Verify the state of the event's requesters list after addition
- Assert the exact exception messages in error cases
Example enhancement for
addToRequestedTest:@Test void addToRequestedTest() { Event event = ModelUtils.getEvent(); User user = ModelUtils.getUser(); user.setId(2L); when(eventRepo.findById(any())).thenReturn(Optional.of(event)); when(userRepo.findByEmail(TestConst.EMAIL)).thenReturn(Optional.of(user)); when(eventRepo.save(event)).thenReturn(event); eventService.addToRequested(1L, TestConst.EMAIL); + assertTrue(event.getRequesters().contains(user), "User should be in requesters list"); verify(eventRepo).findById(any()); verify(userRepo).findByEmail(TestConst.EMAIL); verify(eventRepo).save(event); }
1632-1675: Add state verification in removal tests.The removal tests could be strengthened by:
- Verifying the event's requesters list is empty after removal
- Adding specific exception message assertions
Example enhancement for
removeFromRequestedTest:@Test void removeFromRequestedTest() { Event event = ModelUtils.getEvent(); User user = ModelUtils.getUser(); + event.getRequesters().add(user); when(eventRepo.findById(any())).thenReturn(Optional.of(event)); when(userRepo.findByEmail(TestConst.EMAIL)).thenReturn(Optional.of(user)); when(eventRepo.save(event)).thenReturn(event); eventService.removeFromRequested(1L, TestConst.EMAIL); + assertFalse(event.getRequesters().contains(user), "User should not be in requesters list"); verify(eventRepo).findById(any()); verify(userRepo).findByEmail(TestConst.EMAIL); verify(eventRepo).save(event); }
1677-1717: Add edge cases to requested users retrieval tests.Consider adding tests for:
- Empty page results
- Multiple pages of results
- Different page sizes
- Invalid page numbers
Example additional test:
@Test void getRequestedUsersEmptyPageTest() { Event event = ModelUtils.getEvent(); Pageable pageable = PageRequest.of(0, 20); Page<User> emptyPage = new PageImpl<>(Collections.emptyList(), pageable, 0); when(userRepo.findByEmail(anyString())).thenReturn(Optional.of(ModelUtils.getUser())); when(eventRepo.findById(any())).thenReturn(Optional.of(event)); when(userRepo.findUsersByRequestedEvents(any(), any(Pageable.class))) .thenReturn(emptyPage); Page<User> result = eventService.getRequestedUsers(1L, "", pageable); assertEquals(0, result.getTotalElements()); assertTrue(result.getContent().isEmpty()); }
1586-1852: Consider adding test utilities and improving test organization.The test suite could benefit from:
- Creating test utility methods for common setup code
- Using
@BeforeEachfor common mock setup- Grouping related tests using nested classes
Example refactoring:
@Nested class EventRequestTests { @BeforeEach void setUp() { // Common mock setup for request-related tests } @Nested class AddRequest { // Add request tests } @Nested class RemoveRequest { // Remove request tests } // Other nested classes }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
service/src/test/java/greencity/service/EventServiceImplTest.java(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
| @Test | ||
| void approveRequestTest() { | ||
| UserVO userVO = ModelUtils.getUserVO(); | ||
| User user = ModelUtils.getUser(); | ||
| User userToJoin = User.builder().build(); | ||
| Event event = ModelUtils.getEvent(); | ||
| event.getRequesters().add(userToJoin); | ||
|
|
||
| when(restClient.findByEmail(anyString())).thenReturn(userVO); | ||
| when(modelMapper.map(userVO, User.class)).thenReturn(user); | ||
| when(eventRepo.findById(any())).thenReturn(Optional.of(event)); | ||
| when(userRepo.findById(any())) | ||
| .thenReturn(Optional.of(userToJoin)); | ||
| when(eventRepo.save(any())).thenReturn(null); | ||
|
|
||
| eventService.approveRequest(event.getId(), userVO.getEmail(), userToJoin.getId()); | ||
|
|
||
| verify(restClient).findByEmail(anyString()); | ||
| verify(modelMapper).map(userVO, User.class); | ||
| verify(eventRepo).findById(any()); | ||
| verify(userRepo).findById(any()); | ||
| verify(eventRepo).save(any()); | ||
| } | ||
|
|
||
| @Test | ||
| void approveRequestUserHasNoAccessTest() { | ||
| UserVO userVO = ModelUtils.getUserVO(); | ||
| User user = ModelUtils.getTestUser(); | ||
| Event event = ModelUtils.getEvent(); | ||
|
|
||
| when(restClient.findByEmail(anyString())).thenReturn(userVO); | ||
| when(modelMapper.map(userVO, User.class)).thenReturn(user); | ||
| when(eventRepo.findById(any())).thenReturn(Optional.of(event)); | ||
|
|
||
| Long eventId = event.getId(); | ||
| String email = userVO.getEmail(); | ||
| assertThrows(UserHasNoPermissionToAccessException.class, | ||
| () -> eventService.approveRequest(eventId, email, null)); | ||
|
|
||
| verify(restClient).findByEmail(anyString()); | ||
| verify(modelMapper).map(userVO, User.class); | ||
| verify(eventRepo).findById(any()); | ||
| } | ||
|
|
||
| @Test | ||
| void approveRequestUserDidNotRequestTest() { | ||
| UserVO userVO = ModelUtils.getUserVO(); | ||
| User user = ModelUtils.getUser(); | ||
| User userToJoin = User.builder().build(); | ||
| Event event = ModelUtils.getEvent(); | ||
| event.getRequesters().remove(userToJoin); | ||
|
|
||
| when(restClient.findByEmail(anyString())).thenReturn(userVO); | ||
| when(modelMapper.map(userVO, User.class)).thenReturn(user); | ||
| when(eventRepo.findById(any())).thenReturn(Optional.of(event)); | ||
|
|
||
| Long eventId = event.getId(); | ||
| Long userToJoinId = userToJoin.getId(); | ||
| String email = userVO.getEmail(); | ||
| assertThrows(BadRequestException.class, | ||
| () -> eventService.approveRequest(eventId, email, userToJoinId)); | ||
|
|
||
| verify(restClient).findByEmail(anyString()); | ||
| verify(modelMapper).map(userVO, User.class); | ||
| verify(eventRepo).findById(any()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance state verification in approval tests.
The approval tests should verify:
- User is removed from requesters list
- User is added to attenders list
- Notification service interactions
Example enhancement for approveRequestTest:
@Test
void approveRequestTest() {
UserVO userVO = ModelUtils.getUserVO();
User user = ModelUtils.getUser();
User userToJoin = User.builder().build();
Event event = ModelUtils.getEvent();
event.getRequesters().add(userToJoin);
when(restClient.findByEmail(anyString())).thenReturn(userVO);
when(modelMapper.map(userVO, User.class)).thenReturn(user);
when(eventRepo.findById(any())).thenReturn(Optional.of(event));
when(userRepo.findById(any())).thenReturn(Optional.of(userToJoin));
when(eventRepo.save(any())).thenReturn(event);
eventService.approveRequest(event.getId(), userVO.getEmail(), userToJoin.getId());
+ assertFalse(event.getRequesters().contains(userToJoin), "User should be removed from requesters");
+ assertTrue(event.getAttenders().contains(userToJoin), "User should be added to attenders");
verify(restClient).findByEmail(anyString());
verify(modelMapper).map(userVO, User.class);
verify(eventRepo).findById(any());
verify(userRepo).findById(any());
verify(eventRepo).save(any());
+ verify(userNotificationService).notifyUser(any());
}Committable suggestion skipped: line range outside the PR's diff.
| @Test | ||
| void declineRequestTest() { | ||
| UserVO userVO = ModelUtils.getUserVO(); | ||
| User user = ModelUtils.getUser(); | ||
| User userToJoin = User.builder().build(); | ||
| Event event = ModelUtils.getEvent(); | ||
| event.getRequesters().add(userToJoin); | ||
|
|
||
| when(restClient.findByEmail(anyString())).thenReturn(userVO); | ||
| when(modelMapper.map(userVO, User.class)).thenReturn(user); | ||
| when(eventRepo.findById(any())).thenReturn(Optional.of(event)); | ||
| when(userRepo.findById(any())) | ||
| .thenReturn(Optional.of(userToJoin)); | ||
| when(eventRepo.save(any())).thenReturn(null); | ||
|
|
||
| eventService.declineRequest(event.getId(), userVO.getEmail(), userToJoin.getId()); | ||
|
|
||
| verify(restClient).findByEmail(anyString()); | ||
| verify(modelMapper).map(userVO, User.class); | ||
| verify(eventRepo).findById(any()); | ||
| verify(userRepo).findById(any()); | ||
| verify(eventRepo).save(any()); | ||
| } | ||
|
|
||
| @Test | ||
| void declineRequestUserHasNoAccessTest() { | ||
| UserVO userVO = ModelUtils.getUserVO(); | ||
| User user = ModelUtils.getTestUser(); | ||
| Event event = ModelUtils.getEvent(); | ||
|
|
||
| when(restClient.findByEmail(anyString())).thenReturn(userVO); | ||
| when(modelMapper.map(userVO, User.class)).thenReturn(user); | ||
| when(eventRepo.findById(any())).thenReturn(Optional.of(event)); | ||
|
|
||
| Long eventId = event.getId(); | ||
| String email = userVO.getEmail(); | ||
| assertThrows(UserHasNoPermissionToAccessException.class, | ||
| () -> eventService.declineRequest(eventId, email, null)); | ||
|
|
||
| verify(restClient).findByEmail(anyString()); | ||
| verify(modelMapper).map(userVO, User.class); | ||
| verify(eventRepo).findById(any()); | ||
| } | ||
|
|
||
| @Test | ||
| void declineRequestUserDidNotRequestTest() { | ||
| UserVO userVO = ModelUtils.getUserVO(); | ||
| User user = ModelUtils.getUser(); | ||
| User userToJoin = User.builder().build(); | ||
| Event event = ModelUtils.getEvent(); | ||
| event.getRequesters().remove(userToJoin); | ||
|
|
||
| when(restClient.findByEmail(anyString())).thenReturn(userVO); | ||
| when(modelMapper.map(userVO, User.class)).thenReturn(user); | ||
| when(eventRepo.findById(any())).thenReturn(Optional.of(event)); | ||
|
|
||
| Long eventId = event.getId(); | ||
| Long userToJoinId = userToJoin.getId(); | ||
| String email = userVO.getEmail(); | ||
| assertThrows(BadRequestException.class, | ||
| () -> eventService.declineRequest(eventId, email, userToJoinId)); | ||
|
|
||
| verify(restClient).findByEmail(anyString()); | ||
| verify(modelMapper).map(userVO, User.class); | ||
| verify(eventRepo).findById(any()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Strengthen decline request test assertions.
The decline tests should verify:
- User is removed from requesters list
- User is not added to attenders list
- Notification service interactions
Example enhancement for declineRequestTest:
@Test
void declineRequestTest() {
UserVO userVO = ModelUtils.getUserVO();
User user = ModelUtils.getUser();
User userToJoin = User.builder().build();
Event event = ModelUtils.getEvent();
event.getRequesters().add(userToJoin);
when(restClient.findByEmail(anyString())).thenReturn(userVO);
when(modelMapper.map(userVO, User.class)).thenReturn(user);
when(eventRepo.findById(any())).thenReturn(Optional.of(event));
when(userRepo.findById(any())).thenReturn(Optional.of(userToJoin));
when(eventRepo.save(any())).thenReturn(event);
eventService.declineRequest(event.getId(), userVO.getEmail(), userToJoin.getId());
+ assertFalse(event.getRequesters().contains(userToJoin), "User should be removed from requesters");
+ assertFalse(event.getAttenders().contains(userToJoin), "User should not be added to attenders");
verify(restClient).findByEmail(anyString());
verify(modelMapper).map(userVO, User.class);
verify(eventRepo).findById(any());
verify(userRepo).findById(any());
verify(eventRepo).save(any());
+ verify(userNotificationService).notifyUser(any());
}Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Except from the comments in the code, here is what can be improved:
According to jacoco report, there are missed unit tests for EventServiceImpl class
addToRequested() method => line 876 when thrown NotFoundException if user not found .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_EMAIL + email));
removeFromRequested() method => line 895 when thrown NotFoundException if user not found .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_EMAIL + email));
removeFromRequested() method => line 903 .filter(user -> !user.getId().equals(currentUser.getId()))
getRequestedUsers() method => line 914 when thrown NotFoundException if event not found .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND));
approveRequest() method => line 938 when thrown NotFoundException if event not found .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND));
approveRequest() method => line 947 when thrown NotFoundException if user not found .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_ID + userId));
declineRequest() method => line 965 when thrown NotFoundException if event not found .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND));
When trying to delete a request that does not belong to the user received "This event is not in requested" But should be something like 403 with message "Current user has no permission for this action"
Users can request to join their own events. It not supposed to work this way. The user Should receive 400 status and an error message that he can`t request to join his own event.
|
|
@coderabbitai pause |
✅ Actions performedReviews paused. |
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
service/src/main/java/greencity/service/EventServiceImpl.java (1)
889-903: Consider notifying organizer about request withdrawal.The implementation efficiently removes the request, but consider adding a notification to inform the event organizer when a user withdraws their request.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/src/main/java/greencity/config/SecurityConfig.java(3 hunks)core/src/main/java/greencity/controller/EventController.java(2 hunks)core/src/test/java/greencity/controller/EventControllerTest.java(1 hunks)service/src/main/java/greencity/service/EventServiceImpl.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/test/java/greencity/controller/EventControllerTest.java
🔇 Additional comments (7)
core/src/main/java/greencity/config/SecurityConfig.java (2)
272-272: Security configuration for event request endpoints looks good!The new endpoints are properly secured with appropriate roles and follow consistent authorization patterns.
Also applies to: 311-313
409-410: Security configuration for delete endpoint is appropriate.The endpoint is properly secured and follows the established pattern for delete operations.
core/src/main/java/greencity/controller/EventController.java (3)
499-519: Implementation follows REST best practices.The endpoint is well-documented with comprehensive API responses and follows consistent error handling patterns.
521-540: Consistent implementation with addToRequested endpoint.The endpoint maintains consistency in error handling and response patterns.
542-562: 🛠️ Refactor suggestionAdd parameter validation and improve type safety.
Consider adding validation annotations and using a more specific return type:
@GetMapping("/{eventId}/requested-users") -public ResponseEntity<PageableDto<UserForListDto>> getRequestedUsers( +public ResponseEntity<PageableDto<UserForListDto>> getRequestedUsers( - @PathVariable Long eventId, + @PathVariable @Positive Long eventId, @Parameter(hidden = true) Principal principal, - @Parameter(hidden = true) Pageable pageable) { + @Parameter(hidden = true) @Valid Pageable pageable) {Likely invalid or redundant comment.
service/src/main/java/greencity/service/EventServiceImpl.java (2)
870-887: Service implementation follows best practices.The method properly handles error cases and includes appropriate notification logic.
905-927: Well-structured implementation with proper permission checks.The method correctly handles pagination and access control.
| /** | ||
| * Method for declining join request. | ||
| * | ||
| * @author Olha Pitsyk. | ||
| */ | ||
| @Operation(summary = "Decline join request") | ||
| @ApiResponses(value = { | ||
| @ApiResponse(responseCode = "200", description = HttpStatuses.OK), | ||
| @ApiResponse(responseCode = "400", description = HttpStatuses.BAD_REQUEST), | ||
| @ApiResponse(responseCode = "401", description = HttpStatuses.UNAUTHORIZED), | ||
| @ApiResponse(responseCode = "403", description = HttpStatuses.FORBIDDEN), | ||
| @ApiResponse(responseCode = "404", description = HttpStatuses.NOT_FOUND) | ||
| }) | ||
| @PostMapping("/{eventId}/requested-users/{userId}/decline") | ||
| public ResponseEntity<Object> declineRequest(@PathVariable Long eventId, @PathVariable Long userId, | ||
| @Parameter(hidden = true) Principal principal) { | ||
| eventService.declineRequest(eventId, principal.getName(), userId); | ||
| return ResponseEntity.status(HttpStatus.OK).build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add parameter validation consistent with approveRequest.
Apply the same validation pattern as suggested for approveRequest:
@PostMapping("/{eventId}/requested-users/{userId}/decline")
public ResponseEntity<Object> declineRequest(
- @PathVariable Long eventId,
- @PathVariable Long userId,
+ @PathVariable @Positive Long eventId,
+ @PathVariable @Positive Long userId,
@Parameter(hidden = true) Principal principal) {📝 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.
| /** | |
| * Method for declining join request. | |
| * | |
| * @author Olha Pitsyk. | |
| */ | |
| @Operation(summary = "Decline join request") | |
| @ApiResponses(value = { | |
| @ApiResponse(responseCode = "200", description = HttpStatuses.OK), | |
| @ApiResponse(responseCode = "400", description = HttpStatuses.BAD_REQUEST), | |
| @ApiResponse(responseCode = "401", description = HttpStatuses.UNAUTHORIZED), | |
| @ApiResponse(responseCode = "403", description = HttpStatuses.FORBIDDEN), | |
| @ApiResponse(responseCode = "404", description = HttpStatuses.NOT_FOUND) | |
| }) | |
| @PostMapping("/{eventId}/requested-users/{userId}/decline") | |
| public ResponseEntity<Object> declineRequest(@PathVariable Long eventId, @PathVariable Long userId, | |
| @Parameter(hidden = true) Principal principal) { | |
| eventService.declineRequest(eventId, principal.getName(), userId); | |
| return ResponseEntity.status(HttpStatus.OK).build(); | |
| } | |
| /** | |
| * Method for declining join request. | |
| * | |
| * @author Olha Pitsyk. | |
| */ | |
| @Operation(summary = "Decline join request") | |
| @ApiResponses(value = { | |
| @ApiResponse(responseCode = "200", description = HttpStatuses.OK), | |
| @ApiResponse(responseCode = "400", description = HttpStatuses.BAD_REQUEST), | |
| @ApiResponse(responseCode = "401", description = HttpStatuses.UNAUTHORIZED), | |
| @ApiResponse(responseCode = "403", description = HttpStatuses.FORBIDDEN), | |
| @ApiResponse(responseCode = "404", description = HttpStatuses.NOT_FOUND) | |
| }) | |
| @PostMapping("/{eventId}/requested-users/{userId}/decline") | |
| public ResponseEntity<Object> declineRequest( | |
| @PathVariable @Positive Long eventId, | |
| @PathVariable @Positive Long userId, | |
| @Parameter(hidden = true) Principal principal) { | |
| eventService.declineRequest(eventId, principal.getName(), userId); | |
| return ResponseEntity.status(HttpStatus.OK).build(); | |
| } |
| /** | ||
| * Method for approving join request. | ||
| * | ||
| * @author Olha Pitsyk. | ||
| */ | ||
| @Operation(summary = "Approve join request") | ||
| @ApiResponses(value = { | ||
| @ApiResponse(responseCode = "200", description = HttpStatuses.OK), | ||
| @ApiResponse(responseCode = "400", description = HttpStatuses.BAD_REQUEST), | ||
| @ApiResponse(responseCode = "401", description = HttpStatuses.UNAUTHORIZED), | ||
| @ApiResponse(responseCode = "403", description = HttpStatuses.FORBIDDEN), | ||
| @ApiResponse(responseCode = "404", description = HttpStatuses.NOT_FOUND) | ||
| }) | ||
| @PostMapping("/{eventId}/requested-users/{userId}/approve") | ||
| public ResponseEntity<Object> approveRequest(@PathVariable Long eventId, @PathVariable Long userId, | ||
| @Parameter(hidden = true) Principal principal) { | ||
| eventService.approveRequest(eventId, principal.getName(), userId); | ||
| return ResponseEntity.status(HttpStatus.OK).build(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add parameter validation for request parameters.
Consider adding validation annotations to ensure data integrity:
@PostMapping("/{eventId}/requested-users/{userId}/approve")
public ResponseEntity<Object> approveRequest(
- @PathVariable Long eventId,
- @PathVariable Long userId,
+ @PathVariable @Positive Long eventId,
+ @PathVariable @Positive Long userId,
@Parameter(hidden = true) Principal principal) {📝 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.
| /** | |
| * Method for approving join request. | |
| * | |
| * @author Olha Pitsyk. | |
| */ | |
| @Operation(summary = "Approve join request") | |
| @ApiResponses(value = { | |
| @ApiResponse(responseCode = "200", description = HttpStatuses.OK), | |
| @ApiResponse(responseCode = "400", description = HttpStatuses.BAD_REQUEST), | |
| @ApiResponse(responseCode = "401", description = HttpStatuses.UNAUTHORIZED), | |
| @ApiResponse(responseCode = "403", description = HttpStatuses.FORBIDDEN), | |
| @ApiResponse(responseCode = "404", description = HttpStatuses.NOT_FOUND) | |
| }) | |
| @PostMapping("/{eventId}/requested-users/{userId}/approve") | |
| public ResponseEntity<Object> approveRequest(@PathVariable Long eventId, @PathVariable Long userId, | |
| @Parameter(hidden = true) Principal principal) { | |
| eventService.approveRequest(eventId, principal.getName(), userId); | |
| return ResponseEntity.status(HttpStatus.OK).build(); | |
| } | |
| /** | |
| * Method for approving join request. | |
| * | |
| * @author Olha Pitsyk. | |
| */ | |
| @Operation(summary = "Approve join request") | |
| @ApiResponses(value = { | |
| @ApiResponse(responseCode = "200", description = HttpStatuses.OK), | |
| @ApiResponse(responseCode = "400", description = HttpStatuses.BAD_REQUEST), | |
| @ApiResponse(responseCode = "401", description = HttpStatuses.UNAUTHORIZED), | |
| @ApiResponse(responseCode = "403", description = HttpStatuses.FORBIDDEN), | |
| @ApiResponse(responseCode = "404", description = HttpStatuses.NOT_FOUND) | |
| }) | |
| @PostMapping("/{eventId}/requested-users/{userId}/approve") | |
| public ResponseEntity<Object> approveRequest( | |
| @PathVariable @Positive Long eventId, | |
| @PathVariable @Positive Long userId, | |
| @Parameter(hidden = true) Principal principal) { | |
| eventService.approveRequest(eventId, principal.getName(), userId); | |
| return ResponseEntity.status(HttpStatus.OK).build(); | |
| } |
| @Override | ||
| public void approveRequest(Long eventId, String email, Long userId) { | ||
| UserVO userVO = restClient.findByEmail(email); | ||
| User currentUser = modelMapper.map(userVO, User.class); | ||
|
|
||
| Event event = eventRepo.findById(eventId) | ||
| .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND)); | ||
|
|
||
| if (!Objects.equals(currentUser.getId(), event.getOrganizer().getId())) { | ||
| throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION); | ||
| } | ||
| if (event.getRequesters().stream().noneMatch(u -> Objects.equals(u.getId(), userId))) { | ||
| throw new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId); | ||
| } | ||
| User userToJoin = userRepo.findById(userId) | ||
| .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_ID + userId)); | ||
|
|
||
| event.getRequesters().remove(userToJoin); | ||
| event.getAttenders().add(userToJoin); | ||
|
|
||
| eventRepo.save(event); | ||
|
|
||
| userNotificationService.createNotification(modelMapper.map(userToJoin, UserVO.class), userVO, | ||
| NotificationType.EVENT_REQUEST_ACCEPTED, eventId, event.getTitle()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Optimize user lookup and extract common validation.
Consider these improvements:
- Replace restClient with direct repository access
- Extract common validation logic
- UserVO userVO = restClient.findByEmail(email);
- User currentUser = modelMapper.map(userVO, User.class);
+ User currentUser = userRepo.findByEmail(email)
+ .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_EMAIL + email));
+ validateEventRequestOperation(event, currentUser, userId);
- if (!Objects.equals(currentUser.getId(), event.getOrganizer().getId())) {
- throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION);
- }
- if (event.getRequesters().stream().noneMatch(u -> Objects.equals(u.getId(), userId))) {
- throw new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId);
- }Add this private method:
private void validateEventRequestOperation(Event event, User currentUser, Long userId) {
if (!Objects.equals(currentUser.getId(), event.getOrganizer().getId())) {
throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION);
}
if (event.getRequesters().stream().noneMatch(u -> Objects.equals(u.getId(), userId))) {
throw new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId);
}
}| @Override | ||
| public void declineRequest(Long eventId, String email, Long userId) { | ||
| UserVO userVO = restClient.findByEmail(email); | ||
| User currentUser = modelMapper.map(userVO, User.class); | ||
| Event event = eventRepo.findById(eventId) | ||
| .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND)); | ||
|
|
||
| if (!Objects.equals(currentUser.getId(), event.getOrganizer().getId())) { | ||
| throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION); | ||
| } | ||
| if (event.getRequesters().stream().noneMatch(u -> Objects.equals(u.getId(), userId))) { | ||
| throw new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId); | ||
| } | ||
| User userToJoin = | ||
| userRepo.findById(userId) | ||
| .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_ID + userId)); | ||
|
|
||
| event.getRequesters().remove(userToJoin); | ||
| eventRepo.save(event); | ||
|
|
||
| userNotificationService.createNotification(modelMapper.map(userToJoin, UserVO.class), userVO, | ||
| NotificationType.EVENT_REQUEST_DECLINED, eventId, event.getTitle()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same optimizations as approveRequest.
Use the same improvements suggested for approveRequest:
- Replace restClient with userRepo
- Use the extracted validation method
- UserVO userVO = restClient.findByEmail(email);
- User currentUser = modelMapper.map(userVO, User.class);
+ User currentUser = userRepo.findByEmail(email)
+ .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_EMAIL + email));
+ validateEventRequestOperation(event, currentUser, userId);
- if (!Objects.equals(currentUser.getId(), event.getOrganizer().getId())) {
- throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION);
- }
- if (event.getRequesters().stream().noneMatch(u -> Objects.equals(u.getId(), userId))) {
- throw new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId);
- }Committable suggestion skipped: line range outside the PR's diff.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (8)
core/src/main/java/greencity/controller/EventController.java (4)
500-519: Enhance input validation and REST compliance.Consider these improvements:
- Add @positive validation for eventId
- Return 201 Created instead of 200 OK for resource creation
- Include more detailed API documentation with request/response examples
-@PostMapping("/{eventId}/addToRequested") -public ResponseEntity<Object> addToRequested(@PathVariable Long eventId, +@PostMapping("/{eventId}/addToRequested") +public ResponseEntity<Object> addToRequested( + @PathVariable @Positive(message = "Event ID must be positive") Long eventId, @Parameter(hidden = true) Principal principal) { eventService.addToRequested(eventId, principal.getName()); - return ResponseEntity.status(HttpStatus.OK).build(); + return ResponseEntity.status(HttpStatus.CREATED).build(); }
521-540: Add input validation for removeFromRequested endpoint.Add @positive validation for eventId parameter to ensure valid input:
@DeleteMapping("/{eventId}/removeFromRequested") -public ResponseEntity<Object> removeFromRequested(@PathVariable Long eventId, +public ResponseEntity<Object> removeFromRequested( + @PathVariable @Positive(message = "Event ID must be positive") Long eventId, @Parameter(hidden = true) Principal principal) { eventService.removeFromRequested(eventId, principal.getName()); return ResponseEntity.status(HttpStatus.OK).build(); }
564-582: Add validation and improve error handling for approveRequest endpoint.Add validation annotations and consider more specific error responses:
@PostMapping("/{eventId}/requested-users/{userId}/approve") public ResponseEntity<Object> approveRequest( - @PathVariable Long eventId, @PathVariable Long userId, + @PathVariable @Positive(message = "Event ID must be positive") Long eventId, + @PathVariable @Positive(message = "User ID must be positive") Long userId, @Parameter(hidden = true) Principal principal) { eventService.approveRequest(eventId, principal.getName(), userId); return ResponseEntity.status(HttpStatus.OK).build(); }
584-602: Add validation for declineRequest endpoint parameters.Add validation annotations for path variables:
@PostMapping("/{eventId}/requested-users/{userId}/decline") public ResponseEntity<Object> declineRequest( - @PathVariable Long eventId, @PathVariable Long userId, + @PathVariable @Positive(message = "Event ID must be positive") Long eventId, + @PathVariable @Positive(message = "User ID must be positive") Long userId, @Parameter(hidden = true) Principal principal) { eventService.declineRequest(eventId, principal.getName(), userId); return ResponseEntity.status(HttpStatus.OK).build(); }service/src/main/java/greencity/service/EventServiceImpl.java (4)
870-887: Simplify collection operations and extract validation.Consider these improvements:
- Use direct collection methods instead of streams
- Extract validation logic to a separate method
+private void validateEventRequest(Event event, User user) { + if (event.getRequesters().contains(user)) { + throw new BadRequestException(ErrorMessage.USER_HAS_ALREADY_ADDED_EVENT_TO_REQUESTED); + } +} @Override public void addToRequested(Long eventId, String email) { Event event = eventRepo.findById(eventId) .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND_BY_ID + eventId)); User currentUser = userRepo.findByEmail(email) .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_EMAIL + email)); - if (event.getRequesters().contains(currentUser)) { - throw new BadRequestException(ErrorMessage.USER_HAS_ALREADY_ADDED_EVENT_TO_REQUESTED); - } + validateEventRequest(event, currentUser); event.getRequesters().add(currentUser); eventRepo.save(event);
889-903: Simplify collection operations in removeFromRequested.Use direct collection methods for better readability and performance:
@Override public void removeFromRequested(Long eventId, String email) { Event event = eventRepo.findById(eventId) .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND_BY_ID + eventId)); User currentUser = userRepo.findByEmail(email) .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_EMAIL + email)); if (!event.getRequesters().contains(currentUser)) { throw new BadRequestException(ErrorMessage.EVENT_IS_NOT_IN_REQUESTED); } - event.getRequesters().remove(currentUser); + if (!event.getRequesters().remove(currentUser)) { + throw new BadRequestException(ErrorMessage.EVENT_IS_NOT_IN_REQUESTED); + } eventRepo.save(event); }
905-927: Extract authorization check and simplify mapping.Consider these improvements:
- Extract authorization check to a reusable method
- Use direct mapping for the user list
+private void validateEventOrganizer(Event event, User user) { + if (!user.equals(event.getOrganizer())) { + throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION); + } +} @Override public PageableDto<UserForListDto> getRequestedUsers(Long eventId, String email, Pageable pageable) { User user = userRepo.findByEmail(email) .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_EMAIL + email)); Event event = eventRepo.findById(eventId) .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND)); - if (!user.equals(event.getOrganizer())) { - throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION); - } + validateEventOrganizer(event, user); Page<User> usersPage = userRepo.findUsersByRequestedEvents(eventId, pageable); - List<UserForListDto> userList = usersPage.stream() - .map(users -> modelMapper.map(users, UserForListDto.class)) - .collect(Collectors.toList()); + List<UserForListDto> userList = modelMapper.map(usersPage.getContent(), + new TypeToken<List<UserForListDto>>() {}.getType());
955-977: Optimize service calls and reuse validation logic.Similar to approveRequest, optimize the implementation:
@Override public void declineRequest(Long eventId, String email, Long userId) { - UserVO userVO = restClient.findByEmail(email); - User currentUser = modelMapper.map(userVO, User.class); + User currentUser = userRepo.findByEmail(email) + .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_EMAIL + email)); Event event = eventRepo.findById(eventId) .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND)); - if (!Objects.equals(currentUser.getId(), event.getOrganizer().getId())) { - throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION); - } - if (event.getRequesters().stream().noneMatch(u -> Objects.equals(u.getId(), userId))) { - throw new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId); - } + validateEventRequestOperation(event, currentUser, userId); User userToJoin = userRepo.findById(userId) .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_ID + userId)); event.getRequesters().remove(userToJoin); eventRepo.save(event); userNotificationService.createNotification(modelMapper.map(userToJoin, UserVO.class), - userVO, NotificationType.EVENT_REQUEST_DECLINED, eventId, event.getTitle()); + modelMapper.map(currentUser, UserVO.class), + NotificationType.EVENT_REQUEST_DECLINED, eventId, event.getTitle()); }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/src/main/java/greencity/config/SecurityConfig.java(3 hunks)core/src/main/java/greencity/controller/EventController.java(2 hunks)core/src/test/java/greencity/controller/EventControllerTest.java(1 hunks)service/src/main/java/greencity/service/EventServiceImpl.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/main/java/greencity/config/SecurityConfig.java
🔇 Additional comments (6)
core/src/test/java/greencity/controller/EventControllerTest.java (4)
749-758: 🛠️ Refactor suggestionAdd error handling tests for declineRequest endpoint.
Similar to approveRequest, add test cases for:
- Non-organizer access attempts
- Invalid user/event ID handling
- User not in requested list
- Error responses (400, 401, 403, 404)
@Test @SneakyThrows void declineRequestTest() { Long eventId = 1L; Long userId = 1L; mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users/{userId}/decline", eventId, userId) .principal(principal)) .andExpect(status().isOk()); verify(eventService).declineRequest(eventId, principal.getName(), userId); } + +@Test +@SneakyThrows +void declineRequestUnauthorizedTest() { + Long eventId = 1L; + Long userId = 1L; + doThrow(new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION)) + .when(eventService) + .declineRequest(eventId, principal.getName(), userId); + mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users/{userId}/decline", eventId, userId) + .principal(principal)) + .andExpect(status().isForbidden()); +}Likely invalid or redundant comment.
738-747: 🛠️ Refactor suggestionImprove test coverage for approveRequest endpoint.
Add test cases for:
- Non-organizer access attempts
- Invalid user/event ID handling
- User not in requested list
- Error responses (400, 401, 403, 404)
@Test @SneakyThrows void approveRequestTest() { Long eventId = 1L; Long userId = 1L; mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users/{userId}/approve", eventId, userId) .principal(principal)) .andExpect(status().isOk()); verify(eventService).approveRequest(eventId, principal.getName(), userId); } + +@Test +@SneakyThrows +void approveRequestNotRequestedUserTest() { + Long eventId = 1L; + Long userId = 1L; + doThrow(new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId)) + .when(eventService) + .approveRequest(eventId, principal.getName(), userId); + mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users/{userId}/approve", eventId, userId) + .principal(principal)) + .andExpect(status().isBadRequest()); +}Likely invalid or redundant comment.
727-736: 🛠️ Refactor suggestionExpand test coverage for getRequestedUsers endpoint.
Add tests to verify:
- Pagination parameter validation
- Authorization checks for non-organizer access
- Invalid event ID handling
- Response content verification
@Test @SneakyThrows void getRequestedUsersTest() { Long eventId = 1L; Pageable pageable = PageRequest.of(0, 20); mockMvc.perform(get(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users", eventId) .principal(principal)) .andExpect(status().isOk()); verify(eventService).getRequestedUsers(eventId, principal.getName(), pageable); } + +@Test +@SneakyThrows +void getRequestedUsersUnauthorizedTest() { + Long eventId = 1L; + doThrow(new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION)) + .when(eventService) + .getRequestedUsers(eq(eventId), anyString(), any(Pageable.class)); + mockMvc.perform(get(EVENTS_CONTROLLER_LINK + "/{eventId}/requested-users", eventId) + .principal(principal)) + .andExpect(status().isForbidden()); +}Likely invalid or redundant comment.
707-715: 🛠️ Refactor suggestionEnhance test coverage for addToRequested endpoint.
While the happy path is covered, consider adding tests for:
- Invalid event ID handling
- Unauthorized access
- Case when user has already requested to join
- Error responses (400, 401, 403, 404)
@Test @SneakyThrows void addToRequestedTest() { Long eventId = 1L; mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/addToRequested", eventId) .principal(principal)) .andExpect(status().isOk()); verify(eventService).addToRequested(eventId, principal.getName()); } + +@Test +@SneakyThrows +void addToRequestedBadRequestTest() { + String notValidId = "id"; + mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/addToRequested", notValidId) + .principal(principal)) + .andExpect(status().isBadRequest()); +} + +@Test +@SneakyThrows +void addToRequestedAlreadyRequestedTest() { + Long eventId = 1L; + doThrow(new BadRequestException(ErrorMessage.USER_HAS_ALREADY_ADDED_EVENT_TO_REQUESTED)) + .when(eventService) + .addToRequested(eventId, principal.getName()); + mockMvc.perform(post(EVENTS_CONTROLLER_LINK + "/{eventId}/addToRequested", eventId) + .principal(principal)) + .andExpect(status().isBadRequest()); +}Likely invalid or redundant comment.
core/src/main/java/greencity/controller/EventController.java (1)
542-562: Improve type safety and validation in getRequestedUsers endpoint.The current implementation could benefit from:
- Specific return type instead of Object
- Validation annotations for parameters
@GetMapping("/{eventId}/requested-users") public ResponseEntity<PageableDto<UserForListDto>> getRequestedUsers( - @PathVariable Long eventId, + @PathVariable @Positive(message = "Event ID must be positive") Long eventId, @Parameter(hidden = true) Principal principal, - @Parameter(hidden = true) Pageable pageable) { + @Parameter(hidden = true) @Valid Pageable pageable) { return ResponseEntity.status(HttpStatus.OK) .body(eventService.getRequestedUsers(eventId, principal.getName(), pageable)); }service/src/main/java/greencity/service/EventServiceImpl.java (1)
929-953: Extract common validation logic and optimize service calls.The implementation duplicates validation logic and could be optimized:
+private void validateEventRequestOperation(Event event, User currentUser, Long userId) { + if (!Objects.equals(currentUser.getId(), event.getOrganizer().getId())) { + throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION); + } + if (event.getRequesters().stream().noneMatch(u -> Objects.equals(u.getId(), userId))) { + throw new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId); + } +} @Override public void approveRequest(Long eventId, String email, Long userId) { - UserVO userVO = restClient.findByEmail(email); - User currentUser = modelMapper.map(userVO, User.class); + User currentUser = userRepo.findByEmail(email) + .orElseThrow(() -> new NotFoundException(ErrorMessage.USER_NOT_FOUND_BY_EMAIL + email)); Event event = eventRepo.findById(eventId) .orElseThrow(() -> new NotFoundException(ErrorMessage.EVENT_NOT_FOUND)); - if (!Objects.equals(currentUser.getId(), event.getOrganizer().getId())) { - throw new UserHasNoPermissionToAccessException(ErrorMessage.USER_HAS_NO_PERMISSION); - } - if (event.getRequesters().stream().noneMatch(u -> Objects.equals(u.getId(), userId))) { - throw new BadRequestException(ErrorMessage.USER_DID_NOT_REQUEST_FOR_EVENT + userId); - } + validateEventRequestOperation(event, currentUser, userId);
| @Test | ||
| @SneakyThrows | ||
| void removeFromRequestedTest() { | ||
| Long eventId = 1L; | ||
| mockMvc.perform(delete(EVENTS_CONTROLLER_LINK + "/{eventId}/removeFromRequested", eventId) | ||
| .principal(principal)) | ||
| .andExpect(status().isOk()); | ||
| verify(eventService).removeFromRequested(eventId, principal.getName()); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling tests for removeFromRequested endpoint.
Similar to addToRequested, enhance test coverage for error scenarios:
- Invalid event ID handling
- Unauthorized access
- Case when event is not in requested list
- Error responses (400, 401, 403, 404)
@Test
@SneakyThrows
void removeFromRequestedTest() {
Long eventId = 1L;
mockMvc.perform(delete(EVENTS_CONTROLLER_LINK + "/{eventId}/removeFromRequested", eventId)
.principal(principal))
.andExpect(status().isOk());
verify(eventService).removeFromRequested(eventId, principal.getName());
}
+
+@Test
+@SneakyThrows
+void removeFromRequestedNotFoundTest() {
+ Long eventId = 1L;
+ doThrow(new NotFoundException(ErrorMessage.EVENT_NOT_FOUND))
+ .when(eventService)
+ .removeFromRequested(eventId, principal.getName());
+ mockMvc.perform(delete(EVENTS_CONTROLLER_LINK + "/{eventId}/removeFromRequested", eventId)
+ .principal(principal))
+ .andExpect(status().isNotFound());
+}📝 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.
| @Test | |
| @SneakyThrows | |
| void removeFromRequestedTest() { | |
| Long eventId = 1L; | |
| mockMvc.perform(delete(EVENTS_CONTROLLER_LINK + "/{eventId}/removeFromRequested", eventId) | |
| .principal(principal)) | |
| .andExpect(status().isOk()); | |
| verify(eventService).removeFromRequested(eventId, principal.getName()); | |
| } | |
| @Test | |
| @SneakyThrows | |
| void removeFromRequestedTest() { | |
| Long eventId = 1L; | |
| mockMvc.perform(delete(EVENTS_CONTROLLER_LINK + "/{eventId}/removeFromRequested", eventId) | |
| .principal(principal)) | |
| .andExpect(status().isOk()); | |
| verify(eventService).removeFromRequested(eventId, principal.getName()); | |
| } | |
| @Test | |
| @SneakyThrows | |
| void removeFromRequestedNotFoundTest() { | |
| Long eventId = 1L; | |
| doThrow(new NotFoundException(ErrorMessage.EVENT_NOT_FOUND)) | |
| .when(eventService) | |
| .removeFromRequested(eventId, principal.getName()); | |
| mockMvc.perform(delete(EVENTS_CONTROLLER_LINK + "/{eventId}/removeFromRequested", eventId) | |
| .principal(principal)) | |
| .andExpect(status().isNotFound()); | |
| } |
GreenCity PR
Issue Link
#6222
Added
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests