[PHEE-296A] Email api #79

ankita10r · 2024-06-10T09:23:05Z

Description

New email api based on spec

Method: POST
Endpoint: /emails
Description: Sends an email to the specified recipient(s).
Request Header:
Platform tenant id: PH tenant
X-CallbackUrl : Url on which the mail delivered response to be sent
Correlation id
Request Body:
to: List of email addresses of the recipients (array of strings)
subject: Subject of the email (string)
body: Body content of the email (string)

   Responses:
202: Email Accepted to be sent
400: Invalid request
404: Missing fields

The tc design is added in linked pr openMF/ph-ee-integration-test#293

PR title should have jira ticket enclosed in [].

Format: [jira_ticket] description

ex: [phee-123] PR title.
Add a link to the Jira ticket.
Describe the changes made and why they were made.

Attached with this is the screenshot of TC passing locally

and

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

Followed the PR title naming convention mentioned above.
Design related bullet points or design document link related to this PR added in the description above.
Updated corresponding Postman Collection or Api documentation for the changes in this PR.
Created/updated unit or integration tests for verifying the changes made.
Added required Swagger annotation and update API documentation with details of any API changes if applicable
Followed the naming conventions as given in https://docs.google.com/document/d/1Q4vaMSzrTxxh9TS0RILuNkSkYCxotuYk1Xe0CMIkkCU/edit?usp=sharing

fynmanoj · 2024-07-05T11:15:23Z

src/main/java/org/fineract/messagegateway/sms/api/EmailApiResource.java

+            @Valid @RequestBody EmailRequestDTO emailRequest
+    ) {
+        if (platformTenantId == null || platformTenantId.isEmpty()) {
+            return ResponseEntity.status(HttpStatus.NOT_FOUND).body("Platform-Tenant-Id header is missing");


Supported param validations(etc: email regex) and basic field validations are missing.
Supported header validations are missing.
Error message not in the right format.

fynmanoj · 2024-07-05T11:16:38Z

src/main/java/org/fineract/messagegateway/sms/data/EmailRequestDTO.java

+        @NotEmpty
+        private String subject;
+
+        @NotEmpty


These annotation validations to be handled and sent in phee/fineract specific format.

fynmanoj · 2024-07-05T11:16:59Z

src/main/java/org/fineract/messagegateway/sms/data/EmailRequestDTO.java

+
+        // Getters and setters
+
+        public List<String> getTo() {


use Lombok for getters and setters

fynmanoj · 2024-07-05T11:18:40Z

src/main/java/org/fineract/messagegateway/sms/service/EmailService.java

+    @Autowired
+    private JavaMailSender mailSender;
+
+    @Value("${spring.mail.host}")


why do we have to specify 'spring' here? looks like an overhead. we are not using this anywhere else.

fynmanoj · 2024-07-05T11:23:13Z

src/main/java/org/fineract/messagegateway/sms/service/EmailService.java

+        boolean flag = false;
+        String error = null;
+        try{
+        ((JavaMailSenderImpl) mailSender).setHost(smtpHost);


Severe issue:
The values and the mail sender are global( available at startup). Setting those everytime when the API is called is a bad design. This should be created once and reused. My suggestion is to use mailSender bean with these properties use here.

fynmanoj · 2024-07-05T11:37:43Z

src/main/java/org/fineract/messagegateway/sms/service/EmailService.java

+
+
+    }
+    private ClientHttpRequestFactory createTimeoutRequestFactory() {


could not find a usage for this method.
Again, the rest template should be create as a bean with these settings so that we don't need t create it on the fly every time.

As rest template is going to be deprecated, better stick with webclient / retroft. There is already one implementation added to connector common

fynmanoj · 2024-07-05T11:39:03Z

src/main/java/org/fineract/messagegateway/sms/api/EmailApiResource.java

+    EmailService emailService;
+
+    @PostMapping
+    public ResponseEntity<String> sendEmail(


API documentation missing

fynmanoj · 2024-07-05T11:39:55Z

src/main/resources/application.yml

@@ -22,6 +22,18 @@ spring:
    password: ${MYSQL_PASSWORD:password}
    driver-class-name: org.drizzle.jdbc.DrizzleDriver

+  mail:


Can you attach a demo video with Gmail settings to show that the email is sending successfully with Gmail?

fynmanoj · 2024-07-05T11:40:55Z

build.gradle

@@ -54,6 +54,9 @@ ext {
 dependencies {
    implementation("org.springframework.boot:spring-boot-starter-web:2.5.5")
    implementation("org.springframework.boot:spring-boot-starter-actuator:$springBootVersion")
+    implementation ("org.springframework.boot:spring-boot-starter-validation:$springBootVersion")
+    implementation 'org.springframework.boot:spring-boot-starter-mail'
+    testImplementation 'org.springframework.boot:spring-boot-starter-test'


why do we need this dependency, I could not find any tests below

fynmanoj · 2024-07-05T11:41:42Z

build.gradle

@@ -54,6 +54,9 @@ ext {
 dependencies {
    implementation("org.springframework.boot:spring-boot-starter-web:2.5.5")
    implementation("org.springframework.boot:spring-boot-starter-actuator:$springBootVersion")
+    implementation ("org.springframework.boot:spring-boot-starter-validation:$springBootVersion")


these validations may not produce error messages in our custom format. Please be careful with that

fynmanoj · 2024-07-05T14:20:00Z

src/main/java/org/fineract/messagegateway/sms/api/EmailApiResource.java

+        emailService.sendEmail(emailRequest,callbackUrl);
+
+        return ResponseEntity.status(HttpStatus.ACCEPTED).body("Email accepted to be sent");
+    }


send response in JSON format

ankita10r added 7 commits June 10, 2024 14:50

[PHEE-296A] Email api

33ccafc

[PHEE-296A] Email api

9ab6265

[PHEE-296A] Email api

e8295f2

[PHEE-296A] Email api

066385f

[PHEE-296A] Email api

e1b8aa4

[PHEE-296A] Email api

4e5f507

[PHEE-296A] Email api

dd0b1a8

fynmanoj requested changes Jul 5, 2024

View reviewed changes

fynmanoj approved these changes Jul 5, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[PHEE-296A] Email api #79

[PHEE-296A] Email api #79

ankita10r commented Jun 10, 2024 •

edited

Loading

fynmanoj Jul 5, 2024

fynmanoj Jul 5, 2024

fynmanoj Jul 5, 2024

fynmanoj Jul 5, 2024

fynmanoj Jul 5, 2024

fynmanoj Jul 5, 2024

fynmanoj Jul 5, 2024

fynmanoj Jul 5, 2024

fynmanoj Jul 5, 2024

fynmanoj Jul 5, 2024

fynmanoj Jul 5, 2024



		}
		private ClientHttpRequestFactory createTimeoutRequestFactory() {

[PHEE-296A] Email api #79

Are you sure you want to change the base?

[PHEE-296A] Email api #79

Conversation

ankita10r commented Jun 10, 2024 • edited Loading

Description

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ankita10r commented Jun 10, 2024 •

edited

Loading