FINERACT-248: Prevent duplicate SMS campaign names - #5439
Conversation
bc4e52c to
a8a8b7a
Compare
| if (this.smsCampaignRepository.existsByCampaignName(campaignName)) { | ||
| throw new SmsCampaignNameAlreadyExistsException(campaignName); | ||
| } |
There was a problem hiding this 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,.
There was a problem hiding this 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")
There was a problem hiding this comment.
@galovics could you take a fresh look on this please
There was a problem hiding this 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.
There was a problem hiding this 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.
adamsaghy
left a comment
adamsaghy
left a comment
There was a problem hiding this comment.
Kindly review my concerns!
6ba5316 to
0730d48
Compare
|
@nickus Please rebase this PR. |
b0cbd6f to
70ed936
Compare
Add validation to check for duplicate campaign names before creating or updating SMS campaigns. Also add a Liquibase migration to create the campaign_name_UNIQUE constraint on sms_campaign table, since the @UniqueConstraint annotation on the entity was just Hibernate metadata with no actual DB constraint behind it.
70ed936 to
0d74f1c
Compare
|
So many flaky tests... |
|
@adamsaghy finally green, could you please take a new look and merge if everything is okay? |
3 participants
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
existsByCampaignName()andexistsByCampaignNameAndIdNot()methods toSmsCampaignRepositorycreate()method before savingupdate()method when name changesSmsCampaignNameAlreadyExistsExceptionfor clear error messagesTest plan