Thanks Marta - this is super helpful to see. Do you think it will require deployments that are using datatables to run any kind of migration script or other process to ensure compatibility with this when they upgrade to the new release?
I would call people's attention to this effort. There are some long standing issues in the code base - we generally call this code debt on a project. Fixing this doesn't "add new functionality" per se but does make the project more secure and functional. Adding additional tests also helps. All together this contribution set is super important for maintainability of the code. I applaud the contribution. In order to make this more understandable to others, I would point to the tickets that prompted this refactoring. https://issues.apache.org/jira/browse/FINERACT-1911 In this ticket, the enhancement is to the feature of the savings accounts (also used as current accounts and share accounts, which is another story...) but this narrow investigation of feature enhancement led to a broader identification of how data tables are used throughout fineract. This set of changes will impact data tables across fineract. Note that this PR was reviewed and was accepted on Aug 10th, and therefore users of fineract should make sure that their specific implementations are compatible with this change prior to updating their deployments. Marta - thank you again and please correct me if I am wrong. On Fri, Aug 11, 2023 at 9:41 AM Márta Jankovics <marta.jankov...@dpc.hu> wrote: > Hi all, > > I’ve raised a PR to enhance datatable behaviour. > > https://github.com/apache/fineract/pull/3364 > > Please review general changes for data tables. > > - > > Make the datatable read/write actions more type safe. > Instead of reading and comparing string literals as database types, eg. > ResultsetColumnHeaderData > public boolean isDateTimeDisplayType() { return > "DATETIME".equalsIgnoreCase(this.columnDisplayType); } > > changed to type safe enums > public boolean isDateTimeDisplayType() { return columnDisplayType == > DisplayType.DATETIME; } > In order to fulfil this, there were new types introduced > org.apache.fineract.infrastructure.core.service.database.JdbcJavaType > org.apache.fineract.infrastructure.core.service.database.JavaType > These enums are able to recognise/compare/parse/format database types for > the currently supported database dialects: mysql, postgresql, and can be > extended to other dialects on need. > Some code could be still more enhanced, like ResultsetColumnHeaderData. > adjustColumnType > > - > > Availability to connect data tables to the application table by a > configurable database field instead of the application table generated > primary key. > Reason: Datatable support was added for Savings Transactions, but the > generated key of the transactions could be invalidated on Savings Account > reschedule. (Old transactions could be replaced by new transactions with > different primary keys). To keep the referential integrity, there is a need > to link the datatables for this entity through an external key (which never > changes). > - > > First step to support this was to add new property to > org.apache.fineract.infrastructure.dataqueries.data.EntityTables > private final String refColumn; // referenced column name on > apptable > - > > Add new property in EntityTables > This way it is possible to introduce several entity types mapped to > the same application table (GROUP/CENTER) > private final String apptableName; > - > > Additionally we made sure that everywhere in the code the string > based logic was replaced by the use of EntityTables > if ("m_center".equalsIgnoreCase(appTable)) to EntityTables.CENTER > == appTable > - > > Utility classes > org.apache.fineract.infrastructure.core.service.MathUtil > org.apache.fineract.infrastructure.core.service.DateUtils > were moved to common packages and extended by new util methods. > Some of the new methods are not used yet, but during the code clean-up > we would like to use these utils more and more. > - > > Validation and sql build support methods added : > org.apache.fineract.infrastructure.core.service.database. > DatabaseSpecificSQLGenerator > - > > Support for generic search: > org.apache.fineract.infrastructure.core.service.database.SqlOperator > org.apache.fineract.portfolio.search.service.SearchUtil > - > > Enhanced existing unit/integration tests: > org.apache.fineract.infrastructure.dataqueries.service > .ReadWriteNonCoreDataServiceImplTest.java > org.apache.fineract.integrationtests.datatable. > DatatableIntegrationTest.java > org.apache.fineract.integrationtests.common.organisation > .EntityDatatableChecksIntegrationTest.java > > Regards, > Marta Jankovics > > >
