You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fineract.apache.org by "nickus (via GitHub)" <gi...@apache.org> on 2026/02/03 22:06:44 UTC
[PR] FINERACT-248: Prevent duplicate SMS campaign names [fineract]
nickus opened a new pull request, #5439:
URL: https://github.com/apache/fineract/pull/5439
## Summary
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()` methods to `SmsCampaignRepository`
- Validate campaign name uniqueness in `create()` method before saving
- Validate campaign name uniqueness in `update()` method when name changes
- Add `SmsCampaignNameAlreadyExistsException` for clear error messages
## Test plan
- [x] Integration tests added for:
- Creating a campaign with a duplicate name should fail with appropriate error
- Creating campaigns with unique names should succeed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-248: Prevent duplicate SMS campaign names [fineract]
Posted by "nickus (via GitHub)" <gi...@apache.org>.
nickus commented on code in PR #5439:
URL: https://github.com/apache/fineract/pull/5439#discussion_r2773328390
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/service/SmsCampaignWritePlatformServiceJpaImpl.java:
##########
@@ -104,20 +105,32 @@ public class SmsCampaignWritePlatformServiceJpaImpl implements SmsCampaignWriteP
@Transactional
@Override
public CommandProcessingResult create(JsonCommand command) {
- final AppUser currentUser = this.context.authenticatedUser();
- this.smsCampaignValidator.validateCreate(command.json());
- final Long runReportId = command.longValueOfParameterNamed(SmsCampaignValidator.runReportId);
- Report report = this.reportRepository.findById(runReportId).orElseThrow(() -> new ReportNotFoundException(runReportId));
- LocalDateTime tenantDateTime = DateUtils.getLocalDateTimeOfTenant();
- SmsCampaign smsCampaign = SmsCampaign.instance(currentUser, report, command);
- LocalDateTime recurrenceStartDate = smsCampaign.getRecurrenceStartDate();
- if (recurrenceStartDate != null && DateUtils.isBefore(recurrenceStartDate, tenantDateTime)) {
- throw new GeneralPlatformDomainRuleException("error.msg.campaign.recurrenceStartDate.in.the.past",
- "Recurrence start date cannot be the past date.", recurrenceStartDate);
- }
- this.smsCampaignRepository.saveAndFlush(smsCampaign);
+ try {
+ final AppUser currentUser = this.context.authenticatedUser();
+ this.smsCampaignValidator.validateCreate(command.json());
- return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withEntityId(smsCampaign.getId()).build();
+ final String campaignName = command.stringValueOfParameterNamed(SmsCampaignValidator.campaignName);
+ if (this.smsCampaignRepository.existsByCampaignName(campaignName)) {
+ throw new SmsCampaignNameAlreadyExistsException(campaignName);
+ }
Review Comment:
Actually, there is already a unique key on the DB level. You might see further I catch DataIntegrityViolationException and throw the same exception.
see realCause.getMessage().contains("campaign_name_UNIQUE")
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-248: Prevent duplicate SMS campaign names [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on PR #5439:
URL: https://github.com/apache/fineract/pull/5439#issuecomment-3944746014
@nickus Please rebase this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-248: Prevent duplicate SMS campaign names [fineract]
Posted by "adamsaghy (via GitHub)" <gi...@apache.org>.
adamsaghy commented on code in PR #5439:
URL: https://github.com/apache/fineract/pull/5439#discussion_r2822944168
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/service/SmsCampaignWritePlatformServiceJpaImpl.java:
##########
@@ -104,20 +105,32 @@ public class SmsCampaignWritePlatformServiceJpaImpl implements SmsCampaignWriteP
@Transactional
@Override
public CommandProcessingResult create(JsonCommand command) {
- final AppUser currentUser = this.context.authenticatedUser();
- this.smsCampaignValidator.validateCreate(command.json());
- final Long runReportId = command.longValueOfParameterNamed(SmsCampaignValidator.runReportId);
- Report report = this.reportRepository.findById(runReportId).orElseThrow(() -> new ReportNotFoundException(runReportId));
- LocalDateTime tenantDateTime = DateUtils.getLocalDateTimeOfTenant();
- SmsCampaign smsCampaign = SmsCampaign.instance(currentUser, report, command);
- LocalDateTime recurrenceStartDate = smsCampaign.getRecurrenceStartDate();
- if (recurrenceStartDate != null && DateUtils.isBefore(recurrenceStartDate, tenantDateTime)) {
- throw new GeneralPlatformDomainRuleException("error.msg.campaign.recurrenceStartDate.in.the.past",
- "Recurrence start date cannot be the past date.", recurrenceStartDate);
- }
- this.smsCampaignRepository.saveAndFlush(smsCampaign);
+ try {
+ final AppUser currentUser = this.context.authenticatedUser();
+ this.smsCampaignValidator.validateCreate(command.json());
- return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withEntityId(smsCampaign.getId()).build();
+ final String campaignName = command.stringValueOfParameterNamed(SmsCampaignValidator.campaignName);
+ if (this.smsCampaignRepository.existsByCampaignName(campaignName)) {
+ throw new SmsCampaignNameAlreadyExistsException(campaignName);
+ }
Review Comment:
The mentioned unique constraint is not existing. It's mentioned on the entity, but in reality it does not exist. Please add this missing unique constraint.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-248: Prevent duplicate SMS campaign names [fineract]
Posted by "nickus (via GitHub)" <gi...@apache.org>.
nickus commented on code in PR #5439:
URL: https://github.com/apache/fineract/pull/5439#discussion_r2813307741
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/service/SmsCampaignWritePlatformServiceJpaImpl.java:
##########
@@ -104,20 +105,32 @@ public class SmsCampaignWritePlatformServiceJpaImpl implements SmsCampaignWriteP
@Transactional
@Override
public CommandProcessingResult create(JsonCommand command) {
- final AppUser currentUser = this.context.authenticatedUser();
- this.smsCampaignValidator.validateCreate(command.json());
- final Long runReportId = command.longValueOfParameterNamed(SmsCampaignValidator.runReportId);
- Report report = this.reportRepository.findById(runReportId).orElseThrow(() -> new ReportNotFoundException(runReportId));
- LocalDateTime tenantDateTime = DateUtils.getLocalDateTimeOfTenant();
- SmsCampaign smsCampaign = SmsCampaign.instance(currentUser, report, command);
- LocalDateTime recurrenceStartDate = smsCampaign.getRecurrenceStartDate();
- if (recurrenceStartDate != null && DateUtils.isBefore(recurrenceStartDate, tenantDateTime)) {
- throw new GeneralPlatformDomainRuleException("error.msg.campaign.recurrenceStartDate.in.the.past",
- "Recurrence start date cannot be the past date.", recurrenceStartDate);
- }
- this.smsCampaignRepository.saveAndFlush(smsCampaign);
+ try {
+ final AppUser currentUser = this.context.authenticatedUser();
+ this.smsCampaignValidator.validateCreate(command.json());
- return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withEntityId(smsCampaign.getId()).build();
+ final String campaignName = command.stringValueOfParameterNamed(SmsCampaignValidator.campaignName);
+ if (this.smsCampaignRepository.existsByCampaignName(campaignName)) {
+ throw new SmsCampaignNameAlreadyExistsException(campaignName);
+ }
Review Comment:
@galovics could you take a fresh look on this please
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-248: Prevent duplicate SMS campaign names [fineract]
Posted by "nickus (via GitHub)" <gi...@apache.org>.
nickus commented on code in PR #5439:
URL: https://github.com/apache/fineract/pull/5439#discussion_r2836188783
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/service/SmsCampaignWritePlatformServiceJpaImpl.java:
##########
@@ -104,20 +105,32 @@ public class SmsCampaignWritePlatformServiceJpaImpl implements SmsCampaignWriteP
@Transactional
@Override
public CommandProcessingResult create(JsonCommand command) {
- final AppUser currentUser = this.context.authenticatedUser();
- this.smsCampaignValidator.validateCreate(command.json());
- final Long runReportId = command.longValueOfParameterNamed(SmsCampaignValidator.runReportId);
- Report report = this.reportRepository.findById(runReportId).orElseThrow(() -> new ReportNotFoundException(runReportId));
- LocalDateTime tenantDateTime = DateUtils.getLocalDateTimeOfTenant();
- SmsCampaign smsCampaign = SmsCampaign.instance(currentUser, report, command);
- LocalDateTime recurrenceStartDate = smsCampaign.getRecurrenceStartDate();
- if (recurrenceStartDate != null && DateUtils.isBefore(recurrenceStartDate, tenantDateTime)) {
- throw new GeneralPlatformDomainRuleException("error.msg.campaign.recurrenceStartDate.in.the.past",
- "Recurrence start date cannot be the past date.", recurrenceStartDate);
- }
- this.smsCampaignRepository.saveAndFlush(smsCampaign);
+ try {
+ final AppUser currentUser = this.context.authenticatedUser();
+ this.smsCampaignValidator.validateCreate(command.json());
- return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withEntityId(smsCampaign.getId()).build();
+ final String campaignName = command.stringValueOfParameterNamed(SmsCampaignValidator.campaignName);
+ if (this.smsCampaignRepository.existsByCampaignName(campaignName)) {
+ throw new SmsCampaignNameAlreadyExistsException(campaignName);
+ }
Review Comment:
Good catch @adamsaghy, you're right — the @UniqueConstraint annotation on the entity was just Hibernate metadata with no actual DB constraint behind it (since DDL auto-generation is disabled). I've added a Liquibase migration to create the campaign_name_UNIQUE constraint on the sms_campaign table.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] FINERACT-248: Prevent duplicate SMS campaign names [fineract]
Posted by "galovics (via GitHub)" <gi...@apache.org>.
galovics commented on code in PR #5439:
URL: https://github.com/apache/fineract/pull/5439#discussion_r2772964743
##########
fineract-provider/src/main/java/org/apache/fineract/infrastructure/campaigns/sms/service/SmsCampaignWritePlatformServiceJpaImpl.java:
##########
@@ -104,20 +105,32 @@ public class SmsCampaignWritePlatformServiceJpaImpl implements SmsCampaignWriteP
@Transactional
@Override
public CommandProcessingResult create(JsonCommand command) {
- final AppUser currentUser = this.context.authenticatedUser();
- this.smsCampaignValidator.validateCreate(command.json());
- final Long runReportId = command.longValueOfParameterNamed(SmsCampaignValidator.runReportId);
- Report report = this.reportRepository.findById(runReportId).orElseThrow(() -> new ReportNotFoundException(runReportId));
- LocalDateTime tenantDateTime = DateUtils.getLocalDateTimeOfTenant();
- SmsCampaign smsCampaign = SmsCampaign.instance(currentUser, report, command);
- LocalDateTime recurrenceStartDate = smsCampaign.getRecurrenceStartDate();
- if (recurrenceStartDate != null && DateUtils.isBefore(recurrenceStartDate, tenantDateTime)) {
- throw new GeneralPlatformDomainRuleException("error.msg.campaign.recurrenceStartDate.in.the.past",
- "Recurrence start date cannot be the past date.", recurrenceStartDate);
- }
- this.smsCampaignRepository.saveAndFlush(smsCampaign);
+ try {
+ final AppUser currentUser = this.context.authenticatedUser();
+ this.smsCampaignValidator.validateCreate(command.json());
- return new CommandProcessingResultBuilder().withCommandId(command.commandId()).withEntityId(smsCampaign.getId()).build();
+ final String campaignName = command.stringValueOfParameterNamed(SmsCampaignValidator.campaignName);
+ if (this.smsCampaignRepository.existsByCampaignName(campaignName)) {
+ throw new SmsCampaignNameAlreadyExistsException(campaignName);
+ }
Review Comment:
I think the default isolation level we use is read_committed, meaning only committed things will be revealed by this check. It not guarantees anything.
2 transactions concurrently writing the same campaign name is possible.
If you wanna prevent it, ensure having a unique key on the DB level. This is good for a best-effort check but it's not bullet-proof,.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@fineract.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org