Checklist for myself
- [x] 1.) Comment - I have added the relevant licenses to the appropriate .java 
files but after running ./gradlew rat files in component-test/target/ and 
component-test/logs/ caused it to fail

- [x] 2.) Comment - Fixed the bug SMSConfigurationTest. Other two tests 
inTestEmailService and TestSMSService pass however the cause error happens when 
storing a config. I Markus has suggested a fix so I will remove for now so and 
add them when figure it out.

- [x] 3.) Comment I want to used this to allow the notification admin to enable 
or disable specific events. This is for future development. But I think I added 
it too early when it's not relevent to other code

- [x] 4.) Comment - Maintained only Self_management: To be honest I don't 
understant the use of permittable groups. Can you shed light on this?

- [x] 16.) This class shouldn't exist. I replaced it with SMSService.

- [x] 17.) Come to think of it. I can't explain. This is how I have been doing 
it since I learn't how to write sql queries. And I generated the script with 
MySQL bench. I will remove it and read about it proper use cases.

- [x] 11.) The test which was using this method was meant to trigger 
customerlistener events so that I can efficiently debug the class however, it 
kept failing. Can you suggest a possible approach for this?

- [x] 5 , 9 ,10, 13, 14

ToDo:
- [ ] 6.) Good job for adding validation tests cases. Please add one for each 
of your domain objects (the classes in the package 
org.apache.fineract.cn.notification.api.v1.domain) . Please test all of the 
constraints in your domain objects, not just the identifier constraints.

- [ ] 7.) The constraints in your domain objects do not match the constraints 
in your sql. They need to match. For example: some of your fields in SQL are 
strings with length 255, but you allow strings with length 512 in through the 
domain object validation.

- [ ] 8.) Your feign client needs proper function names for the notification 
end point. Currently the name is the generic "entity" from the template project.

- [ ] 12.) V2 and V3 is simply to insert gateway credentials for testing 
purposes. Can you please suggest a better approach to achieve this?

- [ ] 15.) This repo and entity is intended to contain message template. This 
is in future development. I introduced it to early i guess.

[ Full content available at: 
https://github.com/apache/fineract-cn-notifications/pull/5 ]
This message was relayed via gitbox.apache.org for [email protected]

Reply via email to