Skip to content

Commit a8a8b7a

Browse files
nickusnickus
committed
FINERACT-248: Prevent duplicate SMS campaign names
Add validation to check for duplicate campaign names before creating or updating SMS campaigns. This provides a user-friendly error message instead of relying on database constraint violations. Changes: - Add existsByCampaignName() and existsByCampaignNameAndIdNot() to repository - Validate campaign name uniqueness in create() method - Validate campaign name uniqueness in update() when name changes - Add SmsCampaignNameAlreadyExistsException for clear error messages - Add integration tests for duplicate name validation - Update CampaignsHelper with methods for testing specific campaign names
1 parent ef12056 commit a8a8b7a

File tree

5 files changed

+204
-14
lines changed

5 files changed

+204
-14
lines changed

‎fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/domain/SmsCampaignRepository.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,4 +37,8 @@ public interface SmsCampaignRepository extends JpaRepository<SmsCampaign, Long>,
3737

3838
@Query("SELECT campaign FROM SmsCampaign campaign WHERE campaign.paramValue LIKE :reportPattern AND campaign.triggerType=:triggerType AND campaign.status=300")
3939
List<SmsCampaign> findActiveSmsCampaigns(@Param("reportPattern") String reportPattern, @Param("triggerType") Integer triggerType);
40+
41+
boolean existsByCampaignName(String campaignName);
42+
43+
boolean existsByCampaignNameAndIdNot(String campaignName, Long id);
4044
}

‎fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/exception/SmsCampaignNameAlreadyExistsException.java‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.fineract.infrastructure.campaigns.sms.exception;
20+
21+
import org.apache.fineract.infrastructure.core.exception.AbstractPlatformDomainRuleException;
22+
23+
public class SmsCampaignNameAlreadyExistsException extends AbstractPlatformDomainRuleException {
24+
25+
public SmsCampaignNameAlreadyExistsException(final String name) {
26+
super("error.msg.sms.campaign.duplicate.name", "An SMS campaign with name '" + name + "' already exists", name);
27+
}
28+
}

‎fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/service/SmsCampaignWritePlatformServiceJpaImpl.java‎

Lines changed: 37 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import org.apache.fineract.infrastructure.campaigns.sms.domain.SmsCampaignRepository;
4848
import org.apache.fineract.infrastructure.campaigns.sms.exception.SmsCampaignMustBeClosedToBeDeletedException;
4949
import org.apache.fineract.infrastructure.campaigns.sms.exception.SmsCampaignMustBeClosedToEditException;
50+
import org.apache.fineract.infrastructure.campaigns.sms.exception.SmsCampaignNameAlreadyExistsException;
5051
import org.apache.fineract.infrastructure.campaigns.sms.exception.SmsCampaignNotFound;
5152
import org.apache.fineract.infrastructure.campaigns.sms.serialization.SmsCampaignValidator;
5253
import org.apache.fineract.infrastructure.core.api.JsonCommand;
@@ -104,20 +105,32 @@ public class SmsCampaignWritePlatformServiceJpaImpl implements SmsCampaignWriteP
104105
@Transactional
105106
@Override
106107
public CommandProcessingResult create(JsonCommand command) {
107-
final AppUser currentUser = this.context.authenticatedUser();
108-
this.smsCampaignValidator.validateCreate(command.json());
109-
final Long runReportId = command.longValueOfParameterNamed(SmsCampaignValidator.runReportId);
110-
Report report = this.reportRepository.findById(runReportId).orElseThrow(() -> new ReportNotFoundException(runReportId));
111-
LocalDateTime tenantDateTime = DateUtils.getLocalDateTimeOfTenant();
112-
SmsCampaign smsCampaign = SmsCampaign.instance(currentUser, report, command);
113-
LocalDateTime recurrenceStartDate = smsCampaign.getRecurrenceStartDate();
114-
if (recurrenceStartDate != null && DateUtils.isBefore(recurrenceStartDate, tenantDateTime)) {
115-
throw new GeneralPlatformDomainRuleException("error.msg.campaign.recurrenceStartDate.in.the.past",
116-
"Recurrence start date cannot be the past date.", recurrenceStartDate);
117-
}
118-
this.smsCampaignRepository.saveAndFlush(smsCampaign);
108+
try {
109+
final AppUser currentUser = this.context.authenticatedUser();
110+
this.smsCampaignValidator.validateCreate(command.json());
119111

120-
return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withEntityId(smsCampaign.getId()).build();
112+
final String campaignName = command.stringValueOfParameterNamed(SmsCampaignValidator.campaignName);
113+
if (this.smsCampaignRepository.existsByCampaignName(campaignName)) {
114+
throw new SmsCampaignNameAlreadyExistsException(campaignName);
115+
}
116+
117+
final Long runReportId = command.longValueOfParameterNamed(SmsCampaignValidator.runReportId);
118+
Report report = this.reportRepository.findById(runReportId).orElseThrow(() -> new ReportNotFoundException(runReportId));
119+
LocalDateTime tenantDateTime = DateUtils.getLocalDateTimeOfTenant();
120+
SmsCampaign smsCampaign = SmsCampaign.instance(currentUser, report, command);
121+
LocalDateTime recurrenceStartDate = smsCampaign.getRecurrenceStartDate();
122+
if (recurrenceStartDate != null && DateUtils.isBefore(recurrenceStartDate, tenantDateTime)) {
123+
throw new GeneralPlatformDomainRuleException("error.msg.campaign.recurrenceStartDate.in.the.past",
124+
"Recurrence start date cannot be the past date.", recurrenceStartDate);
125+
}
126+
this.smsCampaignRepository.saveAndFlush(smsCampaign);
127+
128+
return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withEntityId(smsCampaign.getId()).build();
129+
} catch (final JpaSystemException | DataIntegrityViolationException dve) {
130+
final Throwable throwable = dve.getMostSpecificCause();
131+
handleDataIntegrityIssues(command, throwable);
132+
return CommandProcessingResult.empty();
133+
}
121134
}
122135

123136
@Transactional
@@ -135,6 +148,13 @@ public CommandProcessingResult update(final Long resourceId, final JsonCommand c
135148
}
136149
final Map<String, Object> changes = smsCampaign.update(command);
137150

151+
if (changes.containsKey(SmsCampaignValidator.campaignName)) {
152+
final String newName = (String) changes.get(SmsCampaignValidator.campaignName);
153+
if (this.smsCampaignRepository.existsByCampaignNameAndIdNot(newName, resourceId)) {
154+
throw new SmsCampaignNameAlreadyExistsException(newName);
155+
}
156+
}
157+
138158
if (changes.containsKey(SmsCampaignValidator.runReportId)) {
139159
final Long newValue = command.longValueOfParameterNamed(SmsCampaignValidator.runReportId);
140160
final Report reportId = this.reportRepository.findById(newValue).orElseThrow(() -> new ReportNotFoundException(newValue));
@@ -538,6 +558,10 @@ public CommandProcessingResult reactivateSmsCampaign(final Long campaignId, Json
538558
}
539559

540560
private void handleDataIntegrityIssues(final JsonCommand command, final Throwable realCause) {
561+
if (realCause.getMessage().contains("campaign_name_UNIQUE")) {
562+
final String name = command.stringValueOfParameterNamed(SmsCampaignValidator.campaignName);
563+
throw new SmsCampaignNameAlreadyExistsException(name);
564+
}
541565
throw ErrorHandler.getMappable(realCause, "error.msg.sms.campaign.unknown.data.integrity.issue",
542566
"Unknown data integrity issue with resource: " + realCause.getMessage());
543567
}

‎integration-tests/src/test/java/org/apache/fineract/integrationtests/SmsCampaignIntegrationTest.java‎

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
/**
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
package org.apache.fineract.integrationtests;
20+
21+
import static org.junit.jupiter.api.Assertions.assertEquals;
22+
import static org.junit.jupiter.api.Assertions.assertNotNull;
23+
24+
import io.restassured.builder.RequestSpecBuilder;
25+
import io.restassured.builder.ResponseSpecBuilder;
26+
import io.restassured.http.ContentType;
27+
import io.restassured.specification.RequestSpecification;
28+
import io.restassured.specification.ResponseSpecification;
29+
import java.util.HashMap;
30+
import java.util.List;
31+
import org.apache.fineract.integrationtests.common.CommonConstants;
32+
import org.apache.fineract.integrationtests.common.Utils;
33+
import org.apache.fineract.integrationtests.common.organisation.CampaignsHelper;
34+
import org.junit.jupiter.api.BeforeEach;
35+
import org.junit.jupiter.api.Test;
36+
import org.junit.jupiter.api.extension.ExtendWith;
37+
import org.mockserver.integration.ClientAndServer;
38+
import org.mockserver.junit.jupiter.MockServerExtension;
39+
import org.mockserver.junit.jupiter.MockServerSettings;
40+
import org.mockserver.model.HttpRequest;
41+
import org.mockserver.model.HttpResponse;
42+
import org.mockserver.model.MediaType;
43+
44+
/**
45+
* Integration tests for SMS Campaign duplicate name validation.
46+
*/
47+
@ExtendWith(MockServerExtension.class)
48+
@MockServerSettings(ports = { 9191 })
49+
public class SmsCampaignIntegrationTest {
50+
51+
private RequestSpecification requestSpec;
52+
private ResponseSpecification responseSpec;
53+
private ResponseSpecification errorResponseSpec;
54+
private CampaignsHelper campaignsHelper;
55+
private final ClientAndServer client;
56+
57+
public SmsCampaignIntegrationTest(ClientAndServer client) {
58+
this.client = client;
59+
this.client.when(HttpRequest.request().withMethod("GET").withPath("/smsbridges"))
60+
.respond(HttpResponse.response().withContentType(MediaType.APPLICATION_JSON).withBody(
61+
"[{\"id\":1,\"tenantId\":1,\"phoneNo\":\"+1234567890\",\"providerName\":\"Dummy SMS Provider - Testing\",\"providerDescription\":\"Dummy, just for testing\"}]"));
62+
}
63+
64+
@BeforeEach
65+
public void setup() {
66+
Utils.initializeRESTAssured();
67+
this.requestSpec = new RequestSpecBuilder().setContentType(ContentType.JSON).build();
68+
this.requestSpec.header("Authorization", "Basic " + Utils.loginIntoServerAndGetBase64EncodedAuthenticationKey());
69+
this.requestSpec.header("Fineract-Platform-TenantId", "default");
70+
this.responseSpec = new ResponseSpecBuilder().expectStatusCode(200).build();
71+
this.errorResponseSpec = new ResponseSpecBuilder().expectStatusCode(403).build();
72+
this.campaignsHelper = new CampaignsHelper(this.requestSpec, this.responseSpec);
73+
}
74+
75+
@Test
76+
public void testCreateCampaignWithDuplicateNameShouldFail() {
77+
String reportName = "Prospective Clients";
78+
int triggerType = 1;
79+
String campaignName = "Duplicate_Test_Campaign_" + System.currentTimeMillis();
80+
81+
// Create first campaign with specific name
82+
Integer firstCampaignId = campaignsHelper.createCampaignWithName(reportName, triggerType, campaignName);
83+
assertNotNull(firstCampaignId, "First campaign should be created successfully");
84+
campaignsHelper.verifyCampaignCreatedOnServer(requestSpec, responseSpec, firstCampaignId);
85+
86+
// Attempt to create second campaign with the same name - should fail
87+
List<HashMap> errors = campaignsHelper.createCampaignWithNameExpectingError(errorResponseSpec, reportName, triggerType,
88+
campaignName);
89+
90+
assertNotNull(errors, "Error response should not be null");
91+
assertEquals(1, errors.size(), "Should have exactly one error");
92+
assertEquals("error.msg.sms.campaign.duplicate.name", errors.get(0).get(CommonConstants.RESPONSE_ERROR_MESSAGE_CODE),
93+
"Error code should indicate duplicate campaign name");
94+
}
95+
96+
@Test
97+
public void testCreateCampaignWithUniqueNameShouldSucceed() {
98+
String reportName = "Prospective Clients";
99+
int triggerType = 1;
100+
String campaignName1 = "Unique_Campaign_1_" + System.currentTimeMillis();
101+
String campaignName2 = "Unique_Campaign_2_" + System.currentTimeMillis();
102+
103+
// Create first campaign
104+
Integer firstCampaignId = campaignsHelper.createCampaignWithName(reportName, triggerType, campaignName1);
105+
assertNotNull(firstCampaignId, "First campaign should be created successfully");
106+
107+
// Create second campaign with different name - should succeed
108+
Integer secondCampaignId = campaignsHelper.createCampaignWithName(reportName, triggerType, campaignName2);
109+
assertNotNull(secondCampaignId, "Second campaign with different name should be created successfully");
110+
111+
// Verify both campaigns exist
112+
campaignsHelper.verifyCampaignCreatedOnServer(requestSpec, responseSpec, firstCampaignId);
113+
campaignsHelper.verifyCampaignCreatedOnServer(requestSpec, responseSpec, secondCampaignId);
114+
}
115+
}

‎integration-tests/src/test/java/org/apache/fineract/integrationtests/common/organisation/CampaignsHelper.java‎

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,21 @@ public Integer createCampaign(String reportName, Integer triggerType) {
6565
"resourceId");
6666
}
6767

68+
public Integer createCampaignWithName(String reportName, Integer triggerType, String campaignName) {
69+
log.info("---------------------------------CREATING A CAMPAIGN WITH NAME---------------------------------------------");
70+
final String CREATE_SMS_CAMPAIGNS_URL = SMS_CAMPAIGNS_URL + "?" + Utils.TENANT_IDENTIFIER;
71+
return Utils.performServerPost(requestSpec, responseSpec, CREATE_SMS_CAMPAIGNS_URL,
72+
getCreateCampaignJSONWithName(reportName, triggerType, campaignName), "resourceId");
73+
}
74+
75+
public List<HashMap> createCampaignWithNameExpectingError(ResponseSpecification errorResponseSpec, String reportName,
76+
Integer triggerType, String campaignName) {
77+
log.info("---------------------------------CREATING A CAMPAIGN WITH NAME (EXPECTING ERROR)---------------------");
78+
final String CREATE_SMS_CAMPAIGNS_URL = SMS_CAMPAIGNS_URL + "?" + Utils.TENANT_IDENTIFIER;
79+
return Utils.performServerPost(requestSpec, errorResponseSpec, CREATE_SMS_CAMPAIGNS_URL,
80+
getCreateCampaignJSONWithName(reportName, triggerType, campaignName), "errors");
81+
}
82+
6883
// TODO: Rewrite to use fineract-client instead!
6984
// Example: org.apache.fineract.integrationtests.common.loans.LoanTransactionHelper.disburseLoan(java.lang.Long,
7085
// org.apache.fineract.client.models.PostLoansLoanIdRequest)
@@ -132,6 +147,10 @@ public Object performActionsOnCampaignWithFailure(final Integer generatedCampaig
132147
// org.apache.fineract.client.models.PostLoansLoanIdRequest)
133148
@Deprecated(forRemoval = true)
134149
public String getCreateCampaignJSON(String reportName, Integer triggerType) {
150+
return getCreateCampaignJSONWithName(reportName, triggerType, Utils.randomStringGenerator("Campaign_Name_", 5));
151+
}
152+
153+
public String getCreateCampaignJSONWithName(String reportName, Integer triggerType, String campaignName) {
135154
final HashMap<String, Object> map = new HashMap<>();
136155
final HashMap<String, Object> paramValueMap = new HashMap<>();
137156
Long reportId = getSelectedReportId(reportName);
@@ -143,7 +162,7 @@ public String getCreateCampaignJSON(String reportName, Integer triggerType) {
143162
map.put("frequency", 1);
144163
map.put("interval", "1");
145164
}
146-
map.put("campaignName", Utils.randomStringGenerator("Campaign_Name_", 5));
165+
map.put("campaignName", campaignName);
147166
map.put("campaignType", 1);
148167
map.put("message", "Hi, this is from integtration tests runner");
149168
map.put("locale", "en");

0 commit comments

Comments
 (0)