Unfortunately comments in github aren't threaded, so this is going to be a large dump. I'm going to number my comments so that you can reference the comments when responding:
1.) You need to run the rat tests. At least three files are missing their headers, and there may be other problems. To do this run: ./gradlew rat 2.) You need to run your tests. SMSConfigurationTest doesn't compile. TestEmailService doesn't pass. TestSMSService doesn't pass. All tests should compile and run. 3.) What is the purpose of the Event domain object? 4.) You probably only need one permittable group for this service. Currently you have three. 5.) Remove IAMATeapotException. You never return this exception from any of your HTTP methods, so you can delete that throws clause without replacement from the feign client. 6.) Good job for adding validation tests cases. Please add one for each of your domain objects. 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. <still adding points, I'll remove this tag when I'm done> [ Full content available at: https://github.com/apache/fineract-cn-notifications/pull/5 ] This message was relayed via gitbox.apache.org for [email protected]
