Hi everyone, I finished PR #6403 last week. It centralizes the mobile number regex for Client and Staff behind a single config property instead of the three hardcoded copies.
FINERACT-405 still has two things left: UI-side validation with a default country code, and changing the raw String mobileNo fields to a PhoneNumber value object. I'm not very familiar with the UI side, so I haven't looked into that part yet. I did spend some time looking at what it would take to introduce the PhoneNumber object, and wanted to share what I found. I checked where phone numbers are stored in the schema. They are on m_client and m_staff, both with unique constraints, as well as m_guarantor, m_client_family_members, sms_messages_outbound, and request_audit_table. None of these columns have been changed since the original schema, so the database side seems fairly stable. The main complication I found is on the read side. A lot of these values are read using raw SQL rather than JPA. I found eight different places across seven service classes doing this: StaffReadServiceImpl, ClientReadPlatformServiceImpl, SearchReadPlatformServiceImpl, GuarantorReadPlatformServiceImpl, ClientFamilyMembersReadPlatformServiceImpl, SmsReadPlatformServiceImpl, and InteropServiceImpl. Changing the entity field type alone won't take care of these. Each of those queries would need to be looked at separately. InteropServiceImpl also aliases the column as contactPhone in its SQL, so a simple grep for mobileNo would not find everything. There may be other aliases like this that I haven't found yet. There is also the API side. ClientData and StaffData currently have private String mobileNo fields, without Jackson or Swagger annotations. The JSON response is therefore just a string. fineract-client and fineract-client-feign are generated from the OpenAPI spec, so changing the field type directly could also change what SDK consumers get when they regenerate their clients. Another thing I noticed is ClientWritePlatformServiceJpaRepositoryImpl, where the database unique constraint violation on mobile_no is caught and converted into a proper error message. So the equals/hashCode behaviour of the PhoneNumber object will need to work correctly with this. Because of this, the PhoneNumber part looks like quite a bit more work than I initially expected. I don't feel I would be able to complete this on my own. One possible way of splitting it would be to start with ClientFamilyMembers, then Guarantor, and later Client and Staff. The raw SQL paths could then be handled separately. But I'm not sure whether this is actually the right way to approach it. I'd also leave bulk import/export out of this for now. ClientEntityImportHandler, ClientPersonImportHandler, and StaffImportHandler all read these values as raw strings from templates that people may already be using, so I think that needs a separate discussion. I'd also leave the interop module alone for now. I don't know whether mobile_no there is being used as contact information or as some kind of routing identifier for Mojaloop, so I'd rather have someone familiar with that part confirm it before changing anything. I'm sharing this mainly to get some feedback. There is a good chance I've misunderstood part of the existing implementation or am making the work look more complicated than it actually is. If anyone has worked with this area before and sees a simpler way to approach the PhoneNumber change, I'd definitely like to hear it. And if someone else is interested in taking up the remaining PhoneNumber work, either as a whole or as smaller changes, please feel free to do so. I'm happy to share what I found and point to the places I looked at. For now, I'll leave #6403 as the part I've completed and won't start another PR for the PhoneNumber refactor until there's some clarity on the best approach. Thanks, Ashhar Ahmad Khan [1] https://github.com/apache/fineract/pull/6403 JIRA: https://issues.apache.org/jira/browse/FINERACT-405
