Monica-CodingWorld commented on PR #5346: URL: https://github.com/apache/fineract/pull/5346#issuecomment-3799742055
> Kindly see my concerns + you need to add proper testing (integration test / E2E). Hi @adamsaghy, Thanks for the detailed feedback - really appreciate you taking the time to go through this thoroughly. You're absolutely right on all counts. Looking back, I took the wrong approach with the centerId field - I'll refactor to use entityId + entityType like you suggested. Also totally missed the permission migrations, my bad. I'll add those properly also will add the integration tests. Quick question on the SpotBugs fix - I actually already removed the exclusion and fixed the switch statement with a default case. Hope that's the right way to handle it? Will push the fixes shortly. Thanks again for the guidance. Regards, Monica -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
