fix: Add templateid column in program rule action [DHIS2-17515] #17723
Conversation
| } | ||
|
|
||
| if (pnt == null) { | ||
| log.debug( | ||
| String.format( | ||
| "ProgramNotificationTemplate id: %s for program rule: %s does not exist", | ||
| programRuleAction.getTemplateUid(), rule.getName())); | ||
| programRuleAction.getNotificationTemplate().getUid(), rule.getName())); |
There was a problem hiding this comment.
This is a NullPointerException.
And no failing test pointing this issue means that we are not testing the case where the notification template is not present
There was a problem hiding this comment.
good catch @enricocolasante. After this change, i think we won't be needing this part of the code.
I will add test as well to verify this change.
|
|
||
| @Test | ||
| void testInValidRuleActionSendMessage() { | ||
| ProgramRuleAction programRuleAction = new ProgramRuleAction(); |
There was a problem hiding this comment.
ProgramRuleAction shouldn't be of type SENDMESSAGE?
| new NotificationProgramRuleActionValidator(); | ||
| ProgramRuleActionValidationResult result = | ||
| notificationValidator.validate(programRuleAction, validationContext); | ||
| assertFalse(result.isValid()); |
There was a problem hiding this comment.
Could you also assert the ErrorCode to be sure you are getting the right error
| @@ -134,7 +134,7 @@ public class ProgramRuleAction extends BaseIdentifiableObject implements Metadat | |||
| * <li>sendmessage | |||
| * </ul> | |||
| */ | |||
| private String templateUid; | |||
| private ProgramNotificationTemplate notificationTemplate; | |||
There was a problem hiding this comment.
Will this affect Maintenance App?
They were sending a uid before, how is it working now?
There was a problem hiding this comment.
yes and we do have a ticket for that.
https://dhis2.atlassian.net/browse/DHIS2-17534
There was a problem hiding this comment.
This is a breaking change.
If we want to proceed this way, we need to coordinate with Platform team and we cannot just leave the app broken for the time being.
Why cannot we leave this field as a templateUid and change the underneath logic to validate it and store the actual ProgramNotificationTemplate in the DB?
There was a problem hiding this comment.
this is exactly what I discussed previously and we agreed to have a foreign key instead of a validation.
But foreign key is the right solution to go with. if its breaking change then we need to coordinate with the platform team.
|
DHIS2-17515