Create accounts event

[Raj-Shah1]

Summary by CodeRabbit

• New Feature: Enhanced the SalesSparrow API with event, note, and task management capabilities. Users can now create, update, and delete events, notes, and tasks, as well as retrieve detailed information about specific events and tasks.
• New Feature: Introduced event suggestions to improve user experience and productivity.
• Refactor: Improved code efficiency and readability by deduplicating and grouping changesets, renaming classes for clarity, and updating logger variables to be static and final.
• Bug Fix: Removed an extra semicolon in the code to prevent potential syntax errors.

These updates aim to enhance user interaction with the SalesSparrow API, making it more intuitive and user-friendly.

[coderabbitai]

Walkthrough

This pull request introduces significant enhancements to the SalesSparrow API, focusing on event, note, and task management. It includes new DTOs, formatters, and request mappers, along with updates to existing ones. The changes also involve the addition of new controller methods and service classes, and updates to the existing ones. The codebase now supports additional CRM actions such as creating, updating, and deleting events, and getting event details and lists.

Changes

Files	Summary
.../api/controllers/AccountEventController.java .../api/controllers/AccountNoteController.java .../api/controllers/AccountTaskController.java	Introduced and updated methods for event, note, and task management.
.../api/dto/entities/... .../api/dto/formatter/... .../api/dto/requestMapper/...	Added and modified DTOs, formatters, and request mappers for event, note, and task management.
.../api/lib/GetCrmActionSuggestions.java .../api/lib/Util.java	Updated to handle add_event suggestions and added new method for string trimming.
.../api/lib/crmActions/...	Added and updated classes for CRM actions such as creating, updating, and deleting events, notes, and tasks.
.../api/services/...	Added and updated service classes for event, note, and task management.

"In the land of code, where logic is king, 🤴

Changes are made, new features they bring. 🎁

Events, notes, tasks, oh what a spree, 🎉

SalesSparrow API, as robust as can be!" 🐦

---

Tips

Chat with CodeRabbit Bot (@coderabbitai)

• Mention @coderabbitai in any review comment for bot assistance.
• Note: Review comments are made on code diffs or files, not on the PR overview.

Pause Incremental Reviews

• Insert @coderabbitai: ignore in the PR description to halt the bot's ongoing reviews. Remove the line to resume.
• Additionally, enter @coderabbitai pause to halt the bot's ongoing reviews as any issue or review comment. To
  resume reviews, enter @coderabbitai resume as any issue or review comment.

[coderabbitai]

Review Status

Actionable comments generated: 54

Commits

Files that changed from the base of the PR and between a5f015a and 999b80b.

Files ignored due to filter (24)

• pom.xml
• src/main/resources/config/ParamErrorConfig.json
• src/test/resources/data/controllers/eventController/createEvent.scenarios.json
• src/test/resources/data/functional/controllers/accountEventController/createAccountEvent.scenarios.json
• src/test/resources/data/functional/controllers/accountEventController/deleteAccountEvent.scenarios.json
• src/test/resources/data/functional/controllers/accountEventController/getAccountEventDetails.scenarios.json
• src/test/resources/data/functional/controllers/accountEventController/getAccountEventsList.scenarios.json
• src/test/resources/data/functional/controllers/accountEventController/updateAccountEvent.scenarios.json
• src/test/resources/data/functional/controllers/accountNoteController/updateAccountNote.scenarios.json
• src/test/resources/data/functional/controllers/accountTaskController/deleteAccountTask.scenarios.json
• src/test/resources/data/functional/controllers/accountTaskController/getAccountTaskDetails.scenarios.json
• src/test/resources/data/functional/controllers/accountTaskController/getAccountTasksList.scenarios.json
• src/test/resources/data/functional/controllers/accountTaskController/updateAccountTask.scenarios.json
• src/test/resources/data/functional/controllers/authController/Logout.scenarios.json
• src/test/resources/data/functional/controllers/suggestionsController/crmActionsSuggestions.scenarios.json
• src/test/resources/data/functional/controllers/userController/getCurrentUser.scenarios.json
• src/test/resources/fixtures/functional/controllers/accountEventController/createAccountEvent.fixtures.json
• src/test/resources/fixtures/functional/controllers/accountEventController/deleteAccountEvent.fixtures.json
• src/test/resources/fixtures/functional/controllers/accountEventController/getAccountEventDetails.fixtures.json
• src/test/resources/fixtures/functional/controllers/accountEventController/getAccountEventsList.fixtures.json
• src/test/resources/fixtures/functional/controllers/accountEventController/updateAccountEvent.fixtures.json
• src/test/resources/fixtures/functional/controllers/accountNoteController/updateAccountNote.fixtures.json
• src/test/resources/fixtures/functional/controllers/accountTaskController/getAccountTaskDetails.fixtures.json
• src/test/resources/fixtures/functional/controllers/accountTaskController/updateAccountTask.fixtures.json

Files selected for processing (57)

• repo-docs/CHANGELOG.md (1 hunks)
• src/main/java/com/salessparrow/api/controllers/AccountEventController.java (1 hunks)
• src/main/java/com/salessparrow/api/controllers/AccountNoteController.java (3 hunks)
• src/main/java/com/salessparrow/api/controllers/AccountTaskController.java (5 hunks)
• src/main/java/com/salessparrow/api/dto/entities/AddEventSuggestionEntityDto.java (1 hunks)
• src/main/java/com/salessparrow/api/dto/entities/EventEntity.java (1 hunks)
• src/main/java/com/salessparrow/api/dto/entities/TaskEntity.java (1 hunks)
• src/main/java/com/salessparrow/api/dto/formatter/CreateEventFormatterDto.java (1 hunks)
• src/main/java/com/salessparrow/api/dto/formatter/CrmActionSuggestionsFormatterDto.java (2 hunks)
• src/main/java/com/salessparrow/api/dto/formatter/GetEventDetailsFormatterDto.java (1 hunks)
• src/main/java/com/salessparrow/api/dto/formatter/GetEventsListFormatterDto.java (1 hunks)
• src/main/java/com/salessparrow/api/dto/formatter/GetTaskDetailsFormatterDto.java (1 hunks)
• src/main/java/com/salessparrow/api/dto/requestMapper/AccountNoteDto.java (1 hunks)
• src/main/java/com/salessparrow/api/dto/requestMapper/CreateAccountEventDto.java (1 hunks)
• src/main/java/com/salessparrow/api/dto/requestMapper/UpdateAccountEventDto.java (1 hunks)
• src/main/java/com/salessparrow/api/dto/requestMapper/UpdateAccountTaskDto.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/GetCrmActionSuggestions.java (5 hunks)
• src/main/java/com/salessparrow/api/lib/Util.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountEvent/CreateAccountEventFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountEvent/CreateAccountEventInterface.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountEvent/CreateSalesforceAccountEvent.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountNote/CreateNoteFactory.java (2 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountNote/CreateNoteInterface.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountNote/CreateSalesforceNote.java (3 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountEvent/DeleteAccountEvent.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountEvent/DeleteAccountEventFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountEvent/DeleteSalesforceAccountEvent.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountTask/DeleteSalesforceAccountTask.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventDetails/GetAccountEventDetails.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventDetails/GetAccountEventDetailsFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventDetails/GetSalesforceAccountEventDetails.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventsList/GetAccountEventsListFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventsList/GetAccountEventsListInterface.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventsList/GetSalesforceAccountEventsList.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountTaskDetails/GetAccountTaskDetails.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountTaskDetails/GetAccountTaskDetailsFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountTaskDetails/GetSalesforceAccountTaskDetails.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountEvent/UpdateAccountEventFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountEvent/UpdateAccountEventInterface.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountEvent/UpdateSalesforceAccountEvent.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountNote/UpdateAccountNoteFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountNote/UpdateAccountNoteInterface.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountNote/UpdateSalesforceAccountNote.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountTask/UpdateAccountTaskFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountTask/UpdateAccountTaskInterface.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountTask/UpdateSalesforceAccountTask.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/customAnnotations/ValidDatetimeFormat.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/globalConstants/SalesforceConstants.java (3 hunks)
• src/main/java/com/salessparrow/api/lib/openAi/OpenAiPayloadBuilder.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/salesforce/dto/SalesforceCreateEventDto.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/salesforce/dto/SalesforceGetEventsListDto.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/salesforce/dto/SalesforceGetNoteDetailsDto.java (2 hunks)
• src/main/java/com/salessparrow/api/lib/salesforce/dto/SalesforceGetTasksListDto.java (2 hunks)
• src/main/java/com/salessparrow/api/lib/salesforce/helper/SalesforceQueryBuilder.java (2 hunks)
• src/main/java/com/salessparrow/api/lib/validators/DatetimeFormatValidator.java (1 hunks)
• src/main/java/com/salessparrow/api/services/accountEvents/CreateAccountEventService.java (1 hunks)
• src/main/java/com/salessparrow/api/services/accountEvents/DeleteAccountEventService.java (1 hunks)

Files not processed due to max files limit (17)

• src/main/java/com/salessparrow/api/services/accountEvents/GetAccountEventDetailsService.java
• src/main/java/com/salessparrow/api/services/accountEvents/GetAccountEventsListService.java
• src/main/java/com/salessparrow/api/services/accountEvents/UpdateAccountEventService.java
• src/main/java/com/salessparrow/api/services/accountNotes/CreateAccountNoteService.java
• src/main/java/com/salessparrow/api/services/accountNotes/UpdateAccountNoteService.java
• src/main/java/com/salessparrow/api/services/accountTask/GetAccountTaskDetailsService.java
• src/main/java/com/salessparrow/api/services/accountTask/UpdateAccountTaskService.java
• src/test/java/com/salessparrow/api/functional/controllers/accountEventController/CreateAccountEventTest.java
• src/test/java/com/salessparrow/api/functional/controllers/accountEventController/DeleteAccountEventTest.java
• src/test/java/com/salessparrow/api/functional/controllers/accountEventController/GetAccountEventDetailsTest.java
• src/test/java/com/salessparrow/api/functional/controllers/accountEventController/GetAccountEventsListTest.java
• src/test/java/com/salessparrow/api/functional/controllers/accountEventController/UpdateAccountEventTest.java
• src/test/java/com/salessparrow/api/functional/controllers/accountNoteController/DeleteAccountNoteTest.java
• src/test/java/com/salessparrow/api/functional/controllers/accountNoteController/UpdateAccountNoteTest.java
• src/test/java/com/salessparrow/api/functional/controllers/accountTaskController/GetAccountTaskDetailsTest.java
• src/test/java/com/salessparrow/api/functional/controllers/accountTaskController/UpdateAccountTaskTest.java
• src/test/java/com/salessparrow/api/helper/Constants.java

Files skipped from review due to trivial changes (5)

• src/main/java/com/salessparrow/api/dto/entities/EventEntity.java
• src/main/java/com/salessparrow/api/dto/entities/TaskEntity.java
• src/main/java/com/salessparrow/api/dto/formatter/CreateEventFormatterDto.java
• src/main/java/com/salessparrow/api/dto/requestMapper/AccountNoteDto.java
• src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountTask/DeleteSalesforceAccountTask.java

Additional comments (Suppressed): 59

src/main/java/com/salessparrow/api/dto/formatter/CrmActionSuggestionsFormatterDto.java (2)

• 5-10: The import statements have been updated to include AddEventSuggestionEntityDto. This change aligns with the PR summary, which mentions the addition of event-related functionality.

• 19-24: A new field addEventSuggestions of type List<AddEventSuggestionEntityDto> has been added. This field will hold the event suggestions. Ensure that this field is being populated correctly wherever this DTO is being used.

src/main/java/com/salessparrow/api/lib/crmActions/createAccountEvent/CreateAccountEventInterface.java (1)

• 1-17: The new interface CreateAccountEventInterface is introduced with a single method createEvent. This method takes a User object, an account ID as a String, and a CreateAccountEventDto object as parameters and returns a CreateEventFormatterDto object. This interface is marked with the @Component annotation, which means Spring will automatically detect classes that implement this interface for dependency injection. Ensure that all classes implementing this interface are properly annotated for Spring to manage them.

src/main/java/com/salessparrow/api/dto/requestMapper/UpdateAccountTaskDto.java (2)

• 15-16: Ensure that the crmOrganizationUserId is being validated for its existence in the CRM system. If not, it could lead to inconsistencies.

• 22-23: The dueDate field is a string and is being validated with @ValidDateFormat. Ensure that the date format being used is consistent across the application and is properly documented.

src/main/java/com/salessparrow/api/dto/requestMapper/CreateAccountEventDto.java (1)

• 16-18: Ensure that the maximum size for the description is consistent with the database schema and the CRM system's limitations. If the CRM system allows a larger size, consider increasing the limit here to avoid unnecessary truncation.

repo-docs/CHANGELOG.md (1)

• 3-16: The new features and enhancements listed in the changelog align with the summary of the pull request. Ensure that all the changes mentioned in the pull request are reflected in the changelog.

src/main/java/com/salessparrow/api/dto/formatter/GetTaskDetailsFormatterDto.java (1)

• 1-15: The new GetTaskDetailsFormatterDto class is well-structured and follows the best practices for DTO classes in Java. The use of @Data annotation from Lombok library simplifies the code by generating boilerplate code like getters, setters, equals(), hashCode(), and toString() methods. The @JsonNaming annotation is used to specify the naming convention for JSON serialization, which is set to snake case here. This class encapsulates the TaskEntity object in a field named taskDetail.

src/main/java/com/salessparrow/api/dto/formatter/GetEventDetailsFormatterDto.java (1)

• 1-15: The new GetEventDetailsFormatterDto class is well-structured and follows the standard conventions for a DTO class in Java. The use of @Data from Lombok simplifies the class by auto-generating getters, setters, equals(), hashCode(), and toString() methods. The @JsonNaming annotation ensures that the JSON output will follow the snake_case naming convention, which is a common practice in JSON naming. The class encapsulates an EventEntity object, which presumably holds the details of an event.

src/main/java/com/salessparrow/api/lib/crmActions/createAccountEvent/CreateAccountEventFactory.java (2)

• 14-16: The class CreateAccountEventFactory is well designed following the Factory design pattern. It delegates the creation of account events to the appropriate service based on the user's kind. This design allows for easy extension when new user kinds are introduced.

• 34-40: The switch statement handles the case for UserConstants.SALESFORCE_USER_KIND but throws an exception for all other user kinds. If more user kinds are expected in the future, consider adding their respective cases in this switch statement. Also, ensure that the error message is descriptive enough for debugging purposes.

src/main/java/com/salessparrow/api/lib/crmActions/createAccountNote/CreateNoteFactory.java (2)

• 11-11: The NoteDto class has been replaced with AccountNoteDto. Ensure that all instances of NoteDto have been replaced with AccountNoteDto throughout the codebase.

• 28-28: The method signature for createNote has been changed. Ensure that all calls to this method throughout the codebase have been updated to match the new signature.

src/main/java/com/salessparrow/api/lib/crmActions/createAccountNote/CreateNoteInterface.java (1)

• 15-15: The method signature for createNote has been changed to accept AccountNoteDto instead of NoteDto. Ensure that all calls to this method throughout the codebase have been updated to match the new signature.

src/main/java/com/salessparrow/api/lib/GetCrmActionSuggestions.java (7)

• 16-17: The import of AddEventSuggestionEntityDto is new and suggests that the class now handles event suggestions in addition to task suggestions. Ensure that the class's methods and fields have been updated accordingly.

• 40-40: The DatetimeFormatValidator is a new addition. Ensure that it is used correctly in the class to validate datetime formats.

• 76-76: The formattedEventSuggestionEntityDtos list is a new addition. Ensure that it is populated correctly and used as intended.

• 91-92: The addEventList is a new addition. Ensure that it is populated correctly and used as intended.

• 97-99: The conversion of addEventList to typedAddEventList is a new addition. Ensure that the conversion is done correctly and that the typedAddEventList is used as intended.

• 117-135: The loop for processing typedAddEventList is a new addition. Ensure that the loop correctly processes each AddEventSuggestionEntityDto and that the datetime validation is done correctly.

• 139-139: The call to setAddEventSuggestions is a new addition. Ensure that it is called correctly and that the formattedEventSuggestionEntityDtos list is populated correctly.

src/main/java/com/salessparrow/api/controllers/AccountEventController.java (1)

• 94-102: Ensure that all the service methods handle exceptions properly and return appropriate error messages. This will help in debugging and also provide meaningful error messages to the client.

src/main/java/com/salessparrow/api/controllers/AccountTaskController.java (7)

• 33-34: The base URL path for the endpoints has been changed from "/api/v1/accounts" to "/api/v1/accounts/{account_id}/tasks". Ensure that all calls to these endpoints throughout the codebase have been updated to match the new path.

• 45-52: New services UpdateAccountTaskService and GetAccountTaskDetailsService have been added. Ensure that these services are correctly implemented and tested.

• 54-57: The URL path for the createTask endpoint has been changed from "/{account_id}/tasks" to "". Ensure that all calls to this endpoint throughout the codebase have been updated to match the new path.

• 64-67: The URL path for the getTasksList endpoint has been changed from "/{account_id}/tasks" to "". Ensure that all calls to this endpoint throughout the codebase have been updated to match the new path.

• 74-77: The URL path for the deleteTask endpoint has been changed from "/{account_id}/tasks/{task_id}" to "/{task_id}". Ensure that all calls to this endpoint throughout the codebase have been updated to match the new path.

• 84-92: A new updateTask endpoint has been added. Ensure that this endpoint is correctly implemented and tested.

• 94-102: A new getTaskFromAccount endpoint has been added. Ensure that this endpoint is correctly implemented and tested.

src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventsList/GetAccountEventsListFactory.java (1)

• 1-44: The GetAccountEventsListFactory class is well-structured and follows the Factory design pattern. It delegates the getAccountEventsList operation to the appropriate class based on the user's kind. However, it currently only supports Salesforce users. If more user kinds are to be supported in the future, corresponding classes should be added and handled in the switch-case statement.

		switch (user.getUserKind()) {
			case UserConstants.SALESFORCE_USER_KIND:
				return getSalesforceAccountEventsList.getAccountEventsList(user, accountId);
+			case UserConstants.ANOTHER_USER_KIND:
+				return getAnotherAccountEventsList.getAccountEventsList(user, accountId);
			default:
				throw new CustomException(
						new ErrorObject("l_ca_gatl_gatlf_gtl_1", "something_went_wrong", "Invalid user kind."));
		}

Also, consider using a Map or a strategy pattern for cleaner code and better scalability if the number of user kinds increases significantly.

src/main/java/com/salessparrow/api/controllers/AccountNoteController.java (4)

• 2-28: The import statements have been updated to include new classes and services. Ensure that these new classes and services are correctly implemented and tested.

• 46-50: A new service UpdateAccountNoteService has been added. Make sure that this service is correctly implemented and that it handles all edge cases.

• 52-57: The addNoteToAccount method now uses AccountNoteDto instead of NoteDto. Ensure that all calls to this method throughout the codebase have been updated to match the new signature.

• 92-100: A new method updateNote has been added. This method updates a note associated with an account. Ensure that this method is correctly implemented and that it handles all edge cases.

src/main/java/com/salessparrow/api/lib/crmActions/updateAccountNote/UpdateAccountNoteInterface.java (1)

• 14-14: Ensure that all implementations of this interface have been updated to match the new method signature. Also, verify that all calls to updateNote throughout the codebase have been updated to pass the correct arguments.

src/main/java/com/salessparrow/api/lib/crmActions/updateAccountTask/UpdateAccountTaskInterface.java (1)

• 1-16: The UpdateAccountTaskInterface has been introduced to define the contract for updating tasks in the CRM. It has a single method updateTask which takes a User, accountId, taskId, and UpdateAccountTaskDto as parameters. This method does not return any value, implying that it might be performing the update operation and handling any errors internally.

Ensure that the implementations of this interface handle all possible error scenarios and communicate them appropriately to the caller. Also, verify that the updateTask method is implemented in a service class and is being used correctly in the controller layer.

src/main/java/com/salessparrow/api/lib/globalConstants/SalesforceConstants.java (8)

• 41-43: New method salesforceCreateEventUrl() has been added to generate the URL for creating a Salesforce Event. This is consistent with the other URL generation methods in the class.

• 45-47: New method salesforceUpdateEventUrl(String eventId) has been added to generate the URL for updating a Salesforce Event. Ensure that the eventId parameter is always a valid Salesforce Event ID.

• 49-51: New method salesforceUpdateTaskUrl(String taskId) has been added to generate the URL for updating a Salesforce Task. Ensure that the taskId parameter is always a valid Salesforce Task ID.

• 53-55: New method salesforceUpdateNoteUrl(String noteId) has been added to generate the URL for updating a Salesforce ContentNote. Ensure that the noteId parameter is always a valid Salesforce ContentNote ID.

• 85-87: New method salesforceEventSubjectLength() has been added to return the maximum length of a Salesforce Event subject. Ensure that this value is consistent with Salesforce's API documentation.

• 89-91: New method salesforceTaskSubjectLength() has been added to return the maximum length of a Salesforce Task subject. Ensure that this value is consistent with Salesforce's API documentation.

• 93-95: New method salesforceContentNoteTitleLength() has been added to return the maximum length of a Salesforce ContentNote title. Ensure that this value is consistent with Salesforce's API documentation.

• 109-111: New method salesforceDeleteAccountEventUrl(String eventId) has been added to generate the URL for deleting a Salesforce Event. Ensure that the eventId parameter is always a valid Salesforce Event ID.

src/main/java/com/salessparrow/api/lib/salesforce/dto/SalesforceGetNoteDetailsDto.java (2)

• 5-5: The import of HtmlUtils is new. Ensure that this library is included in your project dependencies.

• 29-36: The noteContentResponse is now being HTML unescaped before being used to create a NoteDetailEntity. This is a good practice to prevent HTML injection attacks. However, ensure that this does not introduce any breaking changes where the escaped HTML content is expected.

- NoteDetailEntity noteDetailEntity = new NoteDetailEntity(this.id, this.createdBy.name, noteContentResponse, this.lastModifiedDate);
+ String decodedNoteContentResponse = HtmlUtils.htmlUnescape(noteContentResponse);
+ NoteDetailEntity noteDetailEntity = new NoteDetailEntity(this.id, this.createdBy.name, decodedNoteContentResponse, this.lastModifiedDate);

src/main/java/com/salessparrow/api/lib/salesforce/dto/SalesforceGetTasksListDto.java (2)

• 39-44: The id field has been added to the Owner inner class. Ensure that this field is being populated correctly and that it is being used as intended in the rest of the codebase.

• 54-54: The CrmOrganizationUserId is now being set in the TaskEntity. Ensure that this change is reflected in all places where TaskEntity is being used, and that the id field in the Owner class is being populated correctly.

-		taskEntity.setCrmOrganizationUserName(this.owner.name);
+		taskEntity.setCrmOrganizationUserName(this.owner.name);
+		taskEntity.setCrmOrganizationUserId(this.owner.id);

src/main/java/com/salessparrow/api/services/accountEvents/CreateAccountEventService.java (1)

• 1-42: The CreateAccountEventService class is well-structured and follows the Single Responsibility Principle. It uses dependency injection for the CreateAccountEventFactory and encapsulates the logic for creating an account event. The method createEvent takes in a HttpServletRequest, accountId, and CreateAccountEventDto and returns a CreateEventFormatterDto. The method retrieves the current user from the request and passes it along with the accountId and CreateAccountEventDto to the createEvent method of the CreateAccountEventFactory. This is a good example of delegation, which improves code readability and maintainability.

src/main/java/com/salessparrow/api/lib/crmActions/createAccountNote/CreateSalesforceNote.java (2)

• 19-19: The NoteDto class has been renamed to AccountNoteDto. Ensure that all references to NoteDto in the codebase have been updated to AccountNoteDto.

• 130-130: The getNoteTitleFromContent method has been removed. Ensure that this method is not being used elsewhere in the codebase.

src/main/java/com/salessparrow/api/lib/customAnnotations/ValidDatetimeFormat.java (1)

• 1-20: The new ValidDatetimeFormat annotation is well implemented. It uses the DatetimeFormatValidator for validation, which should ensure that datetime values are in the correct format. The default message, groups, and payload are also appropriately defined.

src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventDetails/GetAccountEventDetails.java (1)

• 1-16: The interface GetAccountEventDetails has been introduced with a single method getEventDetails(User user, String eventId). Ensure that all implementations of this interface correctly implement this method. Also, verify that the @Component annotation is necessary here, as it's unusual to annotate interfaces with @Component. Typically, this annotation is used on the implementation classes.

src/main/java/com/salessparrow/api/lib/crmActions/getAccountTaskDetails/GetAccountTaskDetails.java (1)

• 1-16: The interface GetAccountTaskDetails is well defined and follows the single responsibility principle. It has a single method getTaskDetails which takes a User object and a taskId as parameters and returns a GetTaskDetailsFormatterDto object. This interface can be implemented by different CRM providers to provide their own implementation of the getTaskDetails method.

src/main/java/com/salessparrow/api/lib/validators/DatetimeFormatValidator.java (1)

• 1-54: The DatetimeFormatValidator class is well implemented. It checks if the input string is in the correct datetime format and returns a boolean value accordingly. The use of SimpleDateFormat with setLenient(false) ensures strict adherence to the datetime format. The exception handling is also done correctly, returning false in case of any exception during parsing.

However, consider refining the exception handling to log the exception message for debugging purposes. This will help in identifying the exact issue when the date parsing fails.

		catch (Exception ex) {
+			// Log the exception message for debugging purposes
+			System.out.println("Date parsing failed: " + ex.getMessage());
			return false;
		}

src/main/java/com/salessparrow/api/lib/salesforce/helper/SalesforceQueryBuilder.java (4)

• 54-59: The query for retrieving tasks has been updated to include the Owner.Id field. Ensure that this additional field is handled correctly in the code that processes the query results.

• 123-129: A new method getAccountEventsQuery has been added to retrieve a list of events for a given account. The query includes the event's Id, Description, CreatedBy.Name, StartDateTime, EndDateTime, and LastModifiedDate. The events are ordered by LastModifiedDate in descending order with a limit of 5. Ensure that this new method is used correctly throughout the codebase.

• 131-137: A new method getAccountEventDetailsUrl has been added to retrieve the details of a specific event based on its Id. The query includes the event's Id, Description, CreatedBy.Name, StartDateTime, EndDateTime, and LastModifiedDate. Ensure that this new method is used correctly throughout the codebase.

• 139-145: The method getAccountTaskDetailsUrl has been updated to include the Owner.Id field in the query. Ensure that this additional field is handled correctly in the code that processes the query results.

[coderabbitai]

Review Status

Actionable comments generated: 1

Commits

Files that changed from the base of the PR and between 999b80b and 1504725.

Files ignored due to filter (1)

• src/test/resources/data/functional/controllers/accountController/getNoteDetails.scenarios.json

Files selected for processing (1)

• src/main/java/com/salessparrow/api/lib/openAi/OpenAiPayloadBuilder.java (1 hunks)

[coderabbitai]

Review Status

Actionable comments generated: 14

Commits

Files that changed from the base of the PR and between 1504725 and 1bcc072.

Files selected for processing (7)

• src/main/java/com/salessparrow/api/lib/Util.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountEvent/CreateSalesforceAccountEvent.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountNote/CreateSalesforceNote.java (3 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountEvent/UpdateSalesforceAccountEvent.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountNote/UpdateSalesforceAccountNote.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountTask/UpdateSalesforceAccountTask.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/openAi/OpenAiPayloadBuilder.java (1 hunks)

Additional comments (Suppressed): 5

src/main/java/com/salessparrow/api/lib/crmActions/createAccountNote/CreateSalesforceNote.java (3)

• 19-19: The NoteDto has been renamed to AccountNoteDto. Ensure that all references to NoteDto in the codebase have been updated to AccountNoteDto.

• 54-65: The method getNoteTitleFromContent() has been removed and its logic has been incorporated into the createNote() method. The new logic uses the Util.getTrimmedString() method to get the note title. This change simplifies the code and removes the need for a separate method to get the note title. However, ensure that the Util.getTrimmedString() method behaves as expected and that the note title is correctly generated.

• 130-130: The getNoteTitleFromContent() method has been removed. If this method was used elsewhere in the codebase, ensure that those references have been updated or removed.

src/main/java/com/salessparrow/api/lib/crmActions/updateAccountTask/UpdateSalesforceAccountTask.java (1)

• 78-100: The parseResponse method is responsible for parsing the response from Salesforce. It checks the status code and throws a CustomException if the status code indicates an error. However, it doesn't handle the case where the response body is null or empty, which could lead to a NullPointerException. Consider adding a null check for the response body.

	private void parseResponse(String updateTaskResponse) {
		logger.info("Parsing the response from Salesforce");

+       if (updateTaskResponse == null || updateTaskResponse.isEmpty()) {
+           throw new CustomException(new ErrorObject("l_ua_uat_usat_pr_4", "empty_response", "The response from Salesforce is empty"));
+       }
		Util util = new Util();
		JsonNode rootNode = util.getJsonNode(updateTaskResponse);

		JsonNode compositeResponse = rootNode.get("compositeResponse").get(0);
		Integer statusCode = compositeResponse.get("httpStatusCode").asInt();

		if (statusCode != 200 && statusCode != 201 && statusCode != 204) {
			String errorBody = compositeResponse.get("body").asText();

			// MALFORMED_ID or NOT_FOUND
			if (statusCode == 400 || statusCode == 404) {

				throw new CustomException(
						new ParamErrorObject("l_ua_uat_usat_pr_1", errorBody, Arrays.asList("invalid_task_id")));
			}
			else {
				throw new CustomException(new ErrorObject("l_ua_uat_usat_pr_2", "something_went_wrong", errorBody));
			}
		}
	}

src/main/java/com/salessparrow/api/lib/crmActions/updateAccountEvent/UpdateSalesforceAccountEvent.java (1)

• 1-103: The class UpdateSalesforceAccountEvent is responsible for updating an event for an account in Salesforce. It uses the SalesforceConstants and MakeCompositeRequest classes to make a PATCH request to the Salesforce API. The response from the API is then parsed and any errors are handled appropriately. The class is well-structured and follows the single responsibility principle. However, there are a few areas that could be improved for better maintainability and performance.

[coderabbitai]

Review Status

Actionable comments generated: 15

Commits

Files that changed from the base of the PR and between 1bcc072 and 491cff0.

Files selected for processing (29)

• src/main/java/com/salessparrow/api/controllers/AccountController.java (1 hunks)
• src/main/java/com/salessparrow/api/controllers/AccountEventController.java (1 hunks)
• src/main/java/com/salessparrow/api/controllers/AccountNoteController.java (4 hunks)
• src/main/java/com/salessparrow/api/controllers/AccountTaskController.java (5 hunks)
• src/main/java/com/salessparrow/api/controllers/SuggestionsController.java (1 hunks)
• src/main/java/com/salessparrow/api/exception/GlobalExceptionHandler.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/GetCrmActionSuggestions.java (5 hunks)
• src/main/java/com/salessparrow/api/lib/Util.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountEvent/CreateAccountEventFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountEvent/CreateSalesforceAccountEvent.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountNote/CreateSalesforceNote.java (3 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountTask/CreateAccountTaskInterface.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountTask/CreateSalesforceAccountTask.java (2 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountEvent/DeleteAccountEventInterface.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountEvent/DeleteSalesforceAccountEvent.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountNote/DeleteAccountNoteFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountNote/DeleteSalesforceAccountNote.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountTaskDetails/GetSalesforceAccountTaskDetails.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountEvent/UpdateAccountEventFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountEvent/UpdateSalesforceAccountEvent.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountNote/UpdateAccountNoteFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountNote/UpdateSalesforceAccountNote.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountTask/UpdateAccountTaskFactory.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/updateAccountTask/UpdateSalesforceAccountTask.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/openAi/OpenAiRequest.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/salesforce/dto/SalesforceGetEventsListDto.java (1 hunks)
• src/main/java/com/salessparrow/api/services/accountEvents/CreateAccountEventService.java (1 hunks)
• src/main/java/com/salessparrow/api/services/accountNotes/DeleteAccountNoteService.java (1 hunks)
• src/main/java/com/salessparrow/api/services/suggestions/CrmActionsSuggestionsService.java (1 hunks)

Files skipped from review due to trivial changes (9)

• src/main/java/com/salessparrow/api/controllers/AccountController.java
• src/main/java/com/salessparrow/api/controllers/SuggestionsController.java
• src/main/java/com/salessparrow/api/exception/GlobalExceptionHandler.java
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountTask/CreateAccountTaskInterface.java
• src/main/java/com/salessparrow/api/lib/crmActions/createAccountTask/CreateSalesforceAccountTask.java
• src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountNote/DeleteSalesforceAccountNote.java
• src/main/java/com/salessparrow/api/lib/openAi/OpenAiRequest.java
• src/main/java/com/salessparrow/api/services/accountNotes/DeleteAccountNoteService.java
• src/main/java/com/salessparrow/api/services/suggestions/CrmActionsSuggestionsService.java

Additional comments (Suppressed): 48

src/main/java/com/salessparrow/api/lib/crmActions/updateAccountEvent/UpdateAccountEventFactory.java (1)

• 30-41: The previous comments on this section have been addressed and the user has clarified their reasoning for the current implementation. No further action is needed.

src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountEvent/DeleteAccountEventInterface.java (1)

• 1-15: The interface DeleteAccountEventInterface is well defined and follows the single responsibility principle. It has a single method deleteAccountEvent which takes a User, accountId, and eventId as parameters. This method is expected to be implemented by any class that needs to delete an account event. The use of the @Component annotation suggests that this interface will be used for dependency injection in Spring, which is a good practice for modularity and testability.

src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountNote/DeleteAccountNoteFactory.java (1)

• 19-19: Good job making the logger variable static final. This is a best practice as it improves performance by creating only one logger instance per class and ensures thread safety.

src/main/java/com/salessparrow/api/lib/crmActions/updateAccountNote/UpdateAccountNoteFactory.java (1)

• 30-41: The updateNote method lacks error handling for potential exceptions thrown by updateSalesforceNote.updateNote. Consider wrapping the call in a try-catch block to handle any exceptions and provide a more meaningful error message to the user.

	public void updateNote(User user, String accountId, String noteId, AccountNoteDto accountNoteDto) {
		logger.info("Update Note Factory started");

		switch (user.getUserKind()) {
			case UserConstants.SALESFORCE_USER_KIND:
+                           try {
				updateSalesforceNote.updateNote(user, accountId, noteId, accountNoteDto);
+                           } catch (Exception e) {
+                               logger.error("Failed to update note", e);
+                               throw new CustomException(
+                                   new ErrorObject("l_ua_uan_uanf_uan_2", "update_note_failed", "Failed to update note."));
+                           }
				return;
			default:
				throw new CustomException(
						new ErrorObject("l_ua_uan_uanf_uan_1", "something_went_wrong", "Invalid user kind."));
		}
	}

src/main/java/com/salessparrow/api/lib/crmActions/createAccountEvent/CreateAccountEventFactory.java (1)

• 22-23: The CreateSalesforceAccountEvent bean is still being injected using field injection. As mentioned in the previous review, constructor injection is generally recommended for mandatory dependencies as it allows for easier unit testing and ensures that the class is always in a valid state. However, if you have a specific reason for using field injection, please disregard this comment.

- @Autowired
- private CreateSalesforceAccountEvent createSalesforceEvent;
+ private final CreateSalesforceAccountEvent createSalesforceEvent;
+ 
+ @Autowired
+ public CreateAccountEventFactory(CreateSalesforceAccountEvent createSalesforceEvent) {
+     this.createSalesforceEvent = createSalesforceEvent;
+ }

src/main/java/com/salessparrow/api/lib/crmActions/updateAccountTask/UpdateAccountTaskFactory.java (1)

• 1-43: The UpdateAccountTaskFactory class has been introduced to handle the update task action for the CRM. It uses the UpdateSalesforceAccountTask class to update tasks for Salesforce users. The updateTask method takes a User object, an account ID, a task ID, and an UpdateAccountTaskDto object as parameters. It checks the user kind and if it's a Salesforce user, it updates the task. If the user kind is not recognized, it throws a CustomException.

The code looks good overall, but there's a potential issue with the updateTask method. It only handles Salesforce users and throws an exception for all other user kinds. If there are other user kinds that need to handle task updates, they should be added to the switch statement.

Also, the error message "Invalid user kind." might not be very helpful for debugging. It would be better to include the actual user kind in the error message.

Here's a suggested change:

38,39c38,39
< 				throw new CustomException(
< 						new ErrorObject("l_ua_uat_uatf_uat_1", "something_went_wrong", "Invalid user kind."));


</blockquote></details>
<details><summary>src/main/java/com/salessparrow/api/lib/crmActions/updateAccountEvent/UpdateSalesforceAccountEvent.java (2)</summary><blockquote>

* 78-100: The `parseResponse` method is responsible for parsing the response from the Salesforce API. It checks the status code of the response and throws a `CustomException` if the status code indicates an error. The method is well-structured and follows the single responsibility principle. However, there is no error handling for potential exceptions that could be thrown during the execution of this method. Consider adding a try-catch block to handle potential exceptions and provide more informative error messages to the user.



* 81-81: Creating a new instance of `Util` class every time the `parseResponse` method is called can be inefficient. Consider making the methods in the `Util` class `static` or autowiring an instance of `Util` at the class level.



</blockquote></details>
<details><summary>src/main/java/com/salessparrow/api/controllers/AccountEventController.java (3)</summary><blockquote>

* 32-33: The `@Validated` annotation is still present in the code. As mentioned in the previous review, this annotation is redundant in this context as all the validation is done on the request body using the `@Valid` annotation. You can safely remove it.
```diff
- @Validated

• 38-51: The services are still being injected using field injection. As mentioned in the previous review, constructor injection is recommended over field injection for better testability and to avoid potential circular dependencies.

- @Autowired
- private CreateAccountEventService createEventService;
- 
- @Autowired
- private GetAccountEventsListService getAccountEventsListService;
- 
- @Autowired
- private DeleteAccountEventService deleteEventService;
- 
- @Autowired
- private UpdateAccountEventService updateEventService;
- 
- @Autowired
- private GetAccountEventDetailsService getAccountEventDetailsService;
+ 
+ private final CreateAccountEventService createEventService;
+ private final GetAccountEventsListService getAccountEventsListService;
+ private final DeleteAccountEventService deleteEventService;
+ private final UpdateAccountEventService updateEventService;
+ private final GetAccountEventDetailsService getAccountEventDetailsService;
+ 
+ public AccountEventController(CreateAccountEventService createEventService, GetAccountEventsListService getAccountEventsListService, DeleteAccountEventService deleteEventService, UpdateAccountEventService updateEventService, GetAccountEventDetailsService getAccountEventDetailsService) {
+     this.createEventService = createEventService;
+     this.getAccountEventsListService = getAccountEventsListService;
+     this.deleteEventService = deleteEventService;
+     this.updateEventService = updateEventService;
+     this.getAccountEventDetailsService = getAccountEventDetailsService;
+ }

• 94-102: The logging level in the getEventFromAccount method is still basic. As mentioned in the previous review, consider adding more detailed logging, such as logging the accountId and eventId where applicable. This will help in tracing the flow of requests and responses in case of issues.

src/main/java/com/salessparrow/api/lib/crmActions/updateAccountNote/UpdateSalesforceAccountNote.java (2)

• 84-106: The parseResponse method is tightly coupled with the HTTP response status codes. Consider using a strategy pattern or a map of handlers for each status code. This would make the code more maintainable and easier to extend in the future. For example, you could have a map where the keys are status codes and the values are handler functions. This way, when you get a response, you can simply look up the handler for the status code and call it.

• 64-66: The updateNoteBody map is being populated manually. Consider creating a DTO for this and using a library like Jackson to convert it to a map. This would make the code more maintainable and less error-prone. For example, you could have a UpdateNoteBodyDto class with title and content fields, and then use Jackson's ObjectMapper to convert an instance of this class to a map.

src/main/java/com/salessparrow/api/lib/crmActions/createAccountNote/CreateSalesforceNote.java (3)

• 19-19: The import statement has been changed from NoteDto to AccountNoteDto. Ensure that all instances of NoteDto have been replaced with AccountNoteDto in the codebase and that the new DTO has all the necessary fields and methods required by the CreateSalesforceNote class.

• 54-65: The method createNote() now takes an AccountNoteDto object instead of a NoteDto object. The method getNoteTitleFromContent() has been removed and its functionality has been incorporated into createNote(). The note title is now obtained by calling Util.getTrimmedString() instead of getNoteTitleFromContent(). The note content is unescaped before the title is extracted, which is a change from the previous behavior where the title was extracted from the escaped content. Ensure that this change does not introduce any issues with special characters in the note title.

• 126-129: The method getNoteTitleFromContent() has been removed. If this method is not used elsewhere in the codebase, this change is fine. However, if it is used elsewhere, ensure that those instances have been updated to use Util.getTrimmedString() instead.

src/main/java/com/salessparrow/api/lib/GetCrmActionSuggestions.java (7)

• 16-24: The new imports AddEventSuggestionEntityDto and DatetimeFormatValidator are introduced. Ensure that these classes exist and are accessible from this package.

• 38-42: The DatetimeFormatValidator instance is added and the Logger instance is now a static final variable. This is a good practice for logger instances as they should be constant and not change throughout the execution of the program.

• 73-78: The formattedEventSuggestionEntityDtos list is added to handle event suggestions. This is in line with the PR summary which mentions the addition of event management functionality.

• 83-100: The code now handles both "add_task" and "add_event" arguments from the API response. This is a significant change from the old hunk which only handled "add_task". Ensure that the external API can return "add_event" in its response.

• 83-105: > Note: This review was outside of the patch, so it was mapped to the patch with the greatest overlap. Original lines [101-115]

The code for handling "add_task" remains largely the same, with the addition of a null check for typedAddTaskList. This is a good practice as it prevents potential NullPointerExceptions.

• 117-135: This new block of code handles "add_event" arguments. It creates AddEventSuggestionEntityDto objects from the API response and validates the datetime format of the start and end times. This is in line with the PR summary which mentions the addition of event management functionality.

• 138-139: The crmActionSuggestionsFormatterDto now sets both task and event suggestions. This is a significant change from the old hunk which only set task suggestions. Ensure that the CrmActionSuggestionsFormatterDto class and all its uses throughout the codebase have been updated to handle event suggestions.

src/main/java/com/salessparrow/api/lib/Util.java (2)

• 184-200: The getTrimmedString method is a good addition to the utility class. However, it doesn't handle the case where length is null or negative. This could lead to unexpected behavior or exceptions. Consider adding a check for these cases.

	public static String getTrimmedString(String input, Integer length) {
		if (input == null || length == null || length < 0) {
			return null;
		}

		if (input.length() < length) {
			return input;
		}

		return input.substring(0, length);
	}

• 176-182: The methods unEscapeSpecialCharactersForPlainText and replaceNewLineWithBreak have been changed from instance methods to static methods. Ensure that all calls to these methods throughout the codebase have been updated to reflect this change.

src/main/java/com/salessparrow/api/lib/crmActions/getAccountTaskDetails/GetSalesforceAccountTaskDetails.java (2)

• 46-66: The method getTaskDetails makes a network request but does not handle any potential exceptions that might be thrown during this operation, such as IOException. It's recommended to add error handling for these exceptions to prevent the application from crashing in case of network issues.

- HttpClient.HttpResponse response = makeCompositeRequest.makePostRequest(compositeRequests, salesforceUserId);
+ HttpClient.HttpResponse response;
+ try {
+     response = makeCompositeRequest.makePostRequest(compositeRequests, salesforceUserId);
+ } catch (IOException e) {
+     logger.error("Network request failed", e);
+     throw new CustomException(new ErrorObject("l_ca_gatd_gsatd_pr_3", "network_request_failed", e.getMessage()));
+ }

• 81-91: The error handling here is not comprehensive. It only checks for status codes 200, 201, and 400. Other status codes like 404 (Not Found), 500 (Internal Server Error), etc., are not handled. It's recommended to add error handling for these status codes as well.

src/main/java/com/salessparrow/api/controllers/AccountNoteController.java (5)

• 2-28: The import statements have been updated to include new classes and DTOs. Ensure that these new classes and DTOs are correctly implemented and that they adhere to the project's standards and conventions.

• 35-35: The logger variable has been made static final, which is a good practice as it makes the logger instance a constant, reducing the overhead of creating a new logger instance for each object of the class.

- private Logger logger = org.slf4j.LoggerFactory.getLogger(AccountNoteController.class);
+ private static final Logger logger = org.slf4j.LoggerFactory.getLogger(AccountNoteController.class);

• 46-50: The UpdateAccountNoteService has been added as a new dependency. Ensure that this service is correctly implemented and that it is correctly autowired in this class.

• 52-57: The method signature for addNoteToAccount has been updated to use AccountNoteDto instead of NoteDto. Ensure that all calls to this method throughout the codebase have been updated to match the new signature.

- public ResponseEntity<CreateNoteFormatterDto> addNoteToAccount(HttpServletRequest request, @PathVariable("account_id") String accountId, @Valid @RequestBody NoteDto note)
+ public ResponseEntity<CreateNoteFormatterDto> addNoteToAccount(HttpServletRequest request, @PathVariable("account_id") String accountId, @Valid @RequestBody AccountNoteDto note)

• 92-100: A new method updateNote has been added to handle PUT requests for updating notes. Ensure that this method is correctly implemented and that it handles all possible edge cases and error conditions.

src/main/java/com/salessparrow/api/lib/crmActions/deleteAccountEvent/DeleteSalesforceAccountEvent.java (2)

• 47-63: The deleteAccountEvent method is clear and concise. It constructs the request, makes the HTTP call, and parses the response. However, it does not handle any exceptions that might be thrown by the makePostRequest method. It would be a good idea to add a try-catch block around the HTTP call to handle any potential exceptions and provide a more meaningful error message to the user.

+ 57:  try {
- 57:  HttpClient.HttpResponse response = makeCompositeRequest.makePostRequest(compositeRequests, salesforceUserId);
+ 57:  HttpClient.HttpResponse response = makeCompositeRequest.makePostRequest(compositeRequests, salesforceUserId);
+ 58:  } catch (Exception e) {
+ 59:      logger.error("Error making HTTP request", e);
+ 60:      throw new CustomException(new ErrorObject("l_ca_dae_dsae_de_1", "http_request_failed", e.getMessage()));
+ 61:  }

[ERROR_HANDLING]

• 70-91: The parseResponse method is well-structured and handles different HTTP status codes appropriately. However, it assumes that the compositeResponse field is always present in the response body and that it is an array with at least one element. If the compositeResponse field is missing or the array is empty, this could lead to a NullPointerException or IndexOutOfBoundsException. Consider adding checks to handle these edge cases.

+ 74:  if (rootNode.has("compositeResponse") && rootNode.get("compositeResponse").isArray() && rootNode.get("compositeResponse").size() > 0) {
- 74:  JsonNode deleteEventCompositeResponse = rootNode.get("compositeResponse").get(0);
+ 74:  JsonNode deleteEventCompositeResponse = rootNode.get("compositeResponse").get(0);
+ 75:  } else {
+ 76:      logger.error("Invalid response body: {}", responseBody);
+ 77:      throw new CustomException(new ErrorObject("l_ca_dae_dsae_pr_3", "invalid_response_body", responseBody));
+ 78:  }

[ERROR_HANDLING]

src/main/java/com/salessparrow/api/lib/salesforce/dto/SalesforceGetEventsListDto.java (2)

• 21-23: The startDateTime and endDateTime fields are still declared as String. If these fields are meant to represent date/time, consider using a more appropriate data type like java.time.LocalDateTime or java.util.Date for better type safety and date/time operations. However, if the usage of String for these fields is intentional and meets your requirements, please ensure that the date/time strings are consistently formatted and parsed throughout your application to avoid potential issues.

• 35-46: The eventEntity method is converting the DTO to an EventEntity. This logic is better suited in a mapper class or method to separate concerns and improve maintainability. If you have a specific reason for keeping the conversion logic in the DTO, could you please share it? It would help me understand your design decision better.

src/main/java/com/salessparrow/api/controllers/AccountTaskController.java (8)

• 33-33: The base URL for the endpoints has been changed from /api/v1/accounts to /api/v1/accounts/{account_id}/tasks. Ensure that all calls to these endpoints throughout the codebase have been updated to match the new base URL.

• 50-50: The UpdateAccountTaskService is a new service that has been autowired. Ensure that this service is correctly implemented and tested.

• 53-53: The GetAccountTaskDetailsService is a new service that has been autowired. Ensure that this service is correctly implemented and tested.

• 55-58: The createTask method now handles requests to the base URL instead of /api/v1/accounts/{account_id}/tasks. Ensure that all calls to this endpoint throughout the codebase have been updated to match the new URL.

• 65-68: The getTasksList method now handles requests to the base URL instead of /api/v1/accounts/{account_id}/tasks. Ensure that all calls to this endpoint throughout the codebase have been updated to match the new URL.

• 75-78: The deleteTask method now handles requests to /api/v1/accounts/{account_id}/tasks/{task_id} instead of /api/v1/accounts/{account_id}/tasks/{task_id}. Ensure that all calls to this endpoint throughout the codebase have been updated to match the new URL.

• 85-93: The updateTask method is a new method that handles PUT requests to /api/v1/accounts/{account_id}/tasks/{task_id}. Ensure that this method is correctly implemented and tested.

• 95-103: The getTaskFromAccount method is a new method that handles GET requests to /api/v1/accounts/{account_id}/tasks/{task_id}. Ensure that this method is correctly implemented and tested.

src/main/java/com/salessparrow/api/lib/crmActions/createAccountEvent/CreateSalesforceAccountEvent.java (3)

• 34-34: The logger variable is declared as static final which is a good practice for logger variables. This ensures that only one instance of the logger is created per class.

• 81-107: The parseResponse method is responsible for parsing the response from Salesforce. It checks the status code of the response and throws a CustomException if the status code is not 200 or 201. However, it does not handle any exceptions that might occur during the parsing process. Consider adding a try-catch block to handle potential exceptions and provide more informative error messages to the user.

private CreateEventFormatterDto parseResponse(String createEventResponse) {
    logger.info("Parsing the response from Salesforce");

    Util util = new Util();
    JsonNode rootNode = util.getJsonNode(createEventResponse);

    JsonNode createEventCompositeResponse = rootNode.get("compositeResponse").get(0);
    Integer createEventStatusCode = createEventCompositeResponse.get("httpStatusCode").asInt();

    if (createEventStatusCode != 200 && createEventStatusCode != 201) {
        String errorBody = createEventCompositeResponse.get("body").asText();

        throw new CustomException(new ErrorObject("l_ca_cae_csae_pr_1", "internal_server_error", errorBody));
    }

+   try {
        JsonNode createEventNodeResponseBody = rootNode.get("compositeResponse").get(0).get("body");

        ObjectMapper mapper = new ObjectMapper();
        mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
        SalesforceCreateEventDto salesforceCreateEventDto = mapper.convertValue(createEventNodeResponseBody,
                SalesforceCreateEventDto.class);

        CreateEventFormatterDto createEventFormatterDto = new CreateEventFormatterDto();
        createEventFormatterDto.setEventId(salesforceCreateEventDto.getId());

        return createEventFormatterDto;
+   } catch (Exception e) {
+       logger.error("Failed to parse Salesforce event response", e);
+       throw new CustomException(new ErrorObject("l_ca_cae_csae_pr_2", "internal_server_error", "Failed to parse Salesforce event response"));
+   }
}

• 90-94: The error handling here is a bit too generic. It would be better to have more specific error messages based on the status code returned. This would make debugging easier in the future.

- if (createEventStatusCode != 200 && createEventStatusCode != 201) {
-    String errorBody = createEventCompositeResponse.get("body").asText();
-    throw new CustomException(new ErrorObject("l_ca_cae_csae_pr_1", "internal_server_error", errorBody));
- }
+ if (createEventStatusCode != 200 && createEventStatusCode != 201) {
+    String errorBody = createEventCompositeResponse.get("body").asText();
+    String errorCode = "l_ca_cae_csae_pr_1";
+    String errorMessage = "internal_server_error";
+    switch (createEventStatusCode) {
+        case 400:
+            errorMessage = "bad_request";
+            break;
+        case 401:
+            errorMessage = "unauthorized";
+            break;
+        case 403:
+            errorMessage = "forbidden";
+            break;
+        case 404:
+            errorMessage = "not_found";
+


</blockquote></details>
<details><summary>src/main/java/com/salessparrow/api/lib/crmActions/updateAccountTask/UpdateSalesforceAccountTask.java (1)</summary><blockquote>

* 77-99: The `parseResponse` method is responsible for parsing the response from Salesforce. It checks the status code and throws a `CustomException` if the status code indicates an error. However, it doesn't handle the case where the response body is null or empty, which could lead to a `NullPointerException`. Consider adding a null check for the response body.
```diff
	private void parseResponse(String updateTaskResponse) {
		logger.info("Parsing the response from Salesforce");

+       if (updateTaskResponse == null || updateTaskResponse.isEmpty()) {
+           throw new CustomException(new ErrorObject("l_ua_uat_usat_pr_4", "empty_response", "The response from Salesforce is empty"));
+       }
		Util util = new Util();
		JsonNode rootNode = util.getJsonNode(updateTaskResponse);

		JsonNode compositeResponse = rootNode.get("compositeResponse").get(0);
		Integer statusCode = compositeResponse.get("httpStatusCode").asInt();

		if (statusCode != 200 && statusCode != 201 && statusCode != 204) {
			String errorBody = compositeResponse.get("body").asText();

			// MALFORMED_ID or NOT_FOUND
			if (statusCode == 400 || statusCode == 404) {

				throw new CustomException(
						new ParamErrorObject("l_ua_uat_usat_pr_1", errorBody, Arrays.asList("invalid_task_id")));
			}
			else {
				throw new CustomException(new ErrorObject("l_ua_uat_usat_pr_2", "something_went_wrong", errorBody));
			}
		}
	}

[coderabbitai]

Review Status

Actionable comments generated: 6

Commits

Files that changed from the base of the PR and between 491cff0 and 32264e6.

Files selected for processing (4)

• src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventDetails/GetSalesforceAccountEventDetails.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventsList/GetSalesforceAccountEventsList.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountTaskDetails/GetSalesforceAccountTaskDetails.java (1 hunks)
• src/main/java/com/salessparrow/api/lib/crmActions/getAccountTasksList/GetSalesforceAccountTasksList.java (1 hunks)

Files skipped from review due to trivial changes (1)

• src/main/java/com/salessparrow/api/lib/crmActions/getAccountTasksList/GetSalesforceAccountTasksList.java

Additional comments (Suppressed): 3

src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventDetails/GetSalesforceAccountEventDetails.java (1)

• 46-66: The method getEventDetails makes a network request but does not handle any potential exceptions that might be thrown during this operation, such as IOException. Consider adding error handling to manage these exceptions and provide a more robust user experience.

- HttpClient.HttpResponse response = makeCompositeRequest.makePostRequest(compositeRequests, salesforceUserId);
+ HttpClient.HttpResponse response;
+ try {
+     response = makeCompositeRequest.makePostRequest(compositeRequests, salesforceUserId);
+ } catch (IOException e) {
+     logger.error("Failed to make network request", e);
+     throw new CustomException(new ErrorObject("l_ca_gaed_gsaed_pr_3", "network_request_failed", e.getMessage()));
+ }

src/main/java/com/salessparrow/api/lib/crmActions/getAccountEventsList/GetSalesforceAccountEventsList.java (2)

• 52-70: The method getAccountEventsList makes a network request but does not handle any potential exceptions that might be thrown during this operation, such as IOException. Consider adding error handling to manage these exceptions and provide a more robust user experience.

- HttpClient.HttpResponse response = makeCompositeRequest.makePostRequest(compositeRequests, salesforceUserId);
+ HttpClient.HttpResponse response;
+ try {
+     response = makeCompositeRequest.makePostRequest(compositeRequests, salesforceUserId);
+ } catch (IOException e) {
+     logger.error("Error making post request", e);
+     throw new CustomException(new ErrorObject("l_ca_gael_gsael_pr_3", "network_error", e.getMessage()));
+ }

• 88-98: The error handling here is based on the HTTP status code. However, it only handles status codes 400, 200, and 201. Other status codes like 500 (Internal Server Error) or 503 (Service Unavailable) are not handled. Consider adding handling for these and other relevant status codes.

[coderabbitai]

The parseResponse method is public and can be called from anywhere in the code. It assumes that the responseBody is a valid JSON string and does not handle any potential JsonProcessingException that might be thrown if the responseBody is not a valid JSON string. Consider adding error handling to manage these exceptions and provide a more robust user experience.

- JsonNode rootNode = util.getJsonNode(responseBody);
+ JsonNode rootNode;
+ try {
+     rootNode = util.getJsonNode(responseBody);
+ } catch (JsonProcessingException e) {
+     logger.error("Failed to parse response body", e);
+     throw new CustomException(new ErrorObject("l_ca_gaed_gsaed_pr_4", "invalid_response_body", e.getMessage()));
+ }

[coderabbitai]

The logger variable should be declared as static final to ensure that only one instance of the logger is created per class. This is a common practice in Java for performance reasons and code consistency.

- Logger logger = LoggerFactory.getLogger(GetSalesforceAccountTaskDetails.class);
+ private static final Logger logger = LoggerFactory.getLogger(GetSalesforceAccountTaskDetails.class);

[coderabbitai]

The method getTaskDetails makes a network request but does not handle any potential exceptions that might be thrown during this operation, such as IOException. It's recommended to add error handling for these exceptions to prevent the application from crashing in case of network issues.

- HttpClient.HttpResponse response = makeCompositeRequest.makePostRequest(compositeRequests, salesforceUserId);
+ HttpClient.HttpResponse response;
+ try {
+     response = makeCompositeRequest.makePostRequest(compositeRequests, salesforceUserId);
+ } catch (IOException e) {
+     logger.error("Network request failed", e);
+     throw new CustomException(new ErrorObject("l_ca_gatd_gsatd_pr_3", "network_request_failed", e.getMessage()));
+ }

[coderabbitai]

The error handling here is not comprehensive. It only checks for status codes 200, 201, and 400. Other status codes like 404 (Not Found), 500 (Internal Server Error), etc., are not handled. It's recommended to add error handling for these status codes as well.

[coderabbitai]

The loop over recordsNode seems unnecessary as the taskEntity is overwritten in each iteration. If there is only one record expected, consider removing the loop. If multiple records are expected, consider storing them in a list.

[coderabbitai]

The ObjectMapper is being created for each record in the loop. This is inefficient as the creation of an ObjectMapper is expensive. Consider creating a single ObjectMapper instance outside the loop and reusing it.

- for (JsonNode recordNode : recordsNode) {
-     ObjectMapper mapper = new ObjectMapper();
-     mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
-     SalesforceGetEventsListDto salesforceGetEventsListDto = mapper.convertValue(recordNode,
-             SalesforceGetEventsListDto.class);
-     EventEntity eventEntity = salesforceGetEventsListDto.eventEntity();
- 
-     eventIds.add(eventEntity.getId());
-     eventIdToEntityMap.put(eventEntity.getId(), eventEntity);
- }
+ ObjectMapper mapper = new ObjectMapper();
+ mapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES);
+ for (JsonNode recordNode : recordsNode) {
+     SalesforceGetEventsListDto salesforceGetEventsListDto = mapper.convertValue(recordNode,
+             SalesforceGetEventsListDto.class);
+     EventEntity eventEntity = salesforceGetEventsListDto.eventEntity();
+ 
+     eventIds.add(eventEntity.getId());
+     eventIdToEntityMap.put(eventEntity.getId(), eventEntity);
+ }