IDEMPIERE-5796: for several tables in a row #2244

nmicoud · 2024-02-13T14:33:28Z

https://idempiere.atlassian.net/browse/IDEMPIERE-5796

Pull Request Checklist

My code follows the code guidelines of this project
My code follows the best practices of this project
I have performed a self-review of my own code
My code is easy to understand and review.
I have checked my code and corrected any misspellings
In hard-to-understand areas, I have commented my code.
My changes generate no new warnings

Tests

I have tested the direct scenario that my code is solving
I checked all collaterals that can be affected by my changes, and tested other potential affected scenarios
New and existing unit tests pass locally with my changes
I have added unit tests that prove my fix is effective or that my feature works

Documentation

I have made corresponding changes to the documentation as follows:
- New feature (non-breaking change which adds functionality): I have created the New Feature page in the project wiki explaining the functionality and how to use it. If relevant, I have committed sample data to the core seed to have usable examples in GardenWorld.
- Breaking change (fix or feature that would cause existing functionality to not work as expected): I have documented the change in a clear way that everyone in the community can understand the impact of the change.
- Improvement (improves and existing functionality): This documentation is needed if the improvement changes the way the user interacts with the system or the outcome of a process/task changes. If it is just, for instance, a performance improvement, documentation might not be needed.
The changed/added documentation is in the project wiki (not privately-hosted pdf files or links pointing to a company website) and is complete and self-explanatory.

https://idempiere.atlassian.net/browse/IDEMPIERE-5796

CarlosRuiz-globalqss · 2024-02-21T14:13:42Z

@nmicoud - this is unnecessary

Same as the swing model generator the "Table Like" parameter accepts several notations, as the Help of the parameters states:

You can use % or a comma separated list of table names enclosed within single quotes (case sensitive)

CarlosRuiz-globalqss

unnecessary

nmicoud · 2024-02-21T14:16:40Z

Hi @CarlosRuiz-globalqss ,
In my tests, it is not working .

eg:

doesn't generate anything

CarlosRuiz-globalqss · 2024-02-21T14:19:48Z

Hi @CarlosRuiz-globalqss , In my tests, it is not working .
doesn't generate anything

@nmicoud - as the help says "comma separated list of table names enclosed within single quotes (case sensitive)"

So, your example must be written as:
'XXA_NdfLine','XXA_NdF'

nmicoud · 2024-02-21T14:21:30Z

with 'XXA_NdfLine','XXA_NdF'
only the last table (XXA_NdF) is generated :-/

CarlosRuiz-globalqss · 2024-02-21T14:22:41Z

with 'XXA_NdfLine','XXA_NdF' only the last table (XXA_NdF) is generated :-/

is case sensitive - are you sure the table name is XXA_NdfLine (with lowercase f?)

nmicoud · 2024-02-21T14:24:51Z

eagle eye :)
Yep 'f' -> 'F' and is working now, thanks for spotting this.

Anyway, adding single quotes is not userfriendly. Seems easier to just write table names directly, no?

CarlosRuiz-globalqss · 2024-02-21T14:33:07Z

it is the same code that is written from the command line - I would just leave it as is - for example I have scripts that run on release time that use this notation, I think it has a lot of years like that

nmicoud · 2024-02-21T14:40:06Z

ok, so maybe adding some code in GenerateModelParameterListener to add those quotes?
haven't tested, but when TableName parameter loses the focus, we can see if it contains comma(s) and check if single quotes are present ; and if not add them?
Or easier, add them on-the-fly on the GenerateModel.java (if TableName parameter contains a comma).
If still not "interested", I'll do to it locally - no bother

CarlosRuiz-globalqss · 2024-02-21T14:45:40Z

I don't see the value added, that's a developer tool, not an end-user, and that change probably will require also changes on other parts, or make it incompatible with the command line tool.

But anyways - you can try to convince @hengsin or @dpansheriya - but IMO there is no value added there :-(

nmicoud · 2024-02-21T15:18:45Z

Ok, let's wait for their opinion (when I was talking about changes on GenerateModel.java, I was talking about the process - no change require elsewhere).

CarlosRuiz-globalqss · 2024-02-21T15:23:52Z

BTW - at some point I tested if making that parameter ChosenMulti would be good, but it didn't work because you can add LIKE stuff there that are not tablenames

hengsin · 2024-05-09T07:18:38Z

hi @nmicoud ,

The changes should be apply to ModelInterfaceGenerator.generateSource and ModelClassGenerator.generateSource instead.

I think making the following changes should make it more user friendly:

comparision of table name is case insensitive (that should be straightforward, just upper or lower everything).
make the use of single quote (') optional, i.e 'abc','def', abc,def, abc% and 'abc%' should all work.

@hengsin

…terfaceGenerator / ModelClassGenerator Implement suggesion from @hengsin

@hengsin

…ter… …faceGenerator / ModelClassGenerator Implement suggesion from @hengsin

nmicoud · 2024-05-13T10:57:29Z

hi @hengsin ,
I've implemented what you suggested.
Does it sounds ok?
Thanks,

Nicolas

hengsin · 2024-05-14T06:40:38Z

hi @hengsin , I've implemented what you suggested. Does it sounds ok? Thanks,

Nicolas

Looks good to me.
@CarlosRuiz-globalqss , WDYT ?

CarlosRuiz-globalqss · 2024-05-14T17:17:23Z

org.adempiere.base/src/org/adempiere/util/ModelInterfaceGenerator.java

-		StringBuilder tableLike = new StringBuilder().append(tableName.trim());
-		if (!tableLike.toString().startsWith("'") || !tableLike.toString().endsWith("'"))
-			tableLike = new StringBuilder("'").append(tableLike).append("'");
+		StringBuilder tableLike = new StringBuilder().append(tableName.trim().toUpperCase());


@nmicoud

Can you please check?

There are differences between the methods generateSource of ModelClassGenerator vs ModelInterfaceGenerator

In theory these two methods must be the same with exception of the line within the while ... new Model

Ideally, because this is duplicated code, it would be better to have it in just one place.

For example, if we can call ModelClassGenerator.generateSource from ModelInterfaceGenerator.

This line here is very relevant, if you don't remove the quotes, then it will fail on line 848 when receiving a quoted tablename

As we are dealing with tablenames, I think is better also to remove spaces too, with the previous approach spaces in the tableLike were allowed, but with this change, spaces will be problematic.

Hi @CarlosRuiz-globalqss ,

Something like that?
Not sure about the name of the new generateSource method with type parameter (which is not yet in javadoc).

And I don't understand what you suggest with spaces ; can you give an example where it can fail?

Thanks,

And I don't understand what you suggest with spaces ; can you give an example where it can fail?

I haven't tested, but from the code I think before the patch the following will work:
'AD_Table', 'AD_Column' <- (see the space after the comma)
but after the patch I think it will fail because you are removing the quotes and adding them back, but not removing the spaces

Just made a test and both tables were considered

@CarlosRuiz-globalqss

…terfaceGenerator / ModelClassGenerator Implement suggestion from @CarlosRuiz-globalqss removing duplicated code

CarlosRuiz-globalqss · 2024-05-15T08:33:02Z

org.adempiere.base/src/org/adempiere/util/ModelClassGenerator.java

-			DB.close(rs, pstmt);
-			rs = null; pstmt = null;
-		}
+		ModelInterfaceGenerator.generateSource(ModelInterfaceGenerator.GEN_SOURCE_INTERFACE, sourceFolder, packageName, entityType, tableName, columnEntityType);


Here probably is GEN_SOURCE_CLASS ?

Oops, fixed

…nterfaceGenerator / ModelClassGenerator fix error

CarlosRuiz-globalqss

tested

@hengsin

…2244) * IDEMPIERE-5796: for several tables in a row https://idempiere.atlassian.net/browse/IDEMPIERE-5796 * IDEMPIERE-5796: for several tables in a row - changes done in ModelInterfaceGenerator / ModelClassGenerator Implement suggesion from @hengsin * IDEMPIERE-5796: for several tables in a row - changes done in ModelInterfaceGenerator / ModelClassGenerator Implement suggesion from @hengsin * IDEMPIERE-5796: for several tables in a row - changes done in ModelInterfaceGenerator / ModelClassGenerator Implement suggestion from @CarlosRuiz-globalqss removing duplicated code * IDEMPIERE-5796: for several tables in a row - changes done in ModelInterfaceGenerator / ModelClassGenerator fix error

IDEMPIERE-5796: for several tables in a row

809238f

https://idempiere.atlassian.net/browse/IDEMPIERE-5796

CarlosRuiz-globalqss requested changes Feb 21, 2024

View reviewed changes

nmicoud added 2 commits May 13, 2024 12:53

IDEMPIERE-5796: for several tables in a row - changes done in ModelIn…

050f320

…terfaceGenerator / ModelClassGenerator Implement suggesion from @hengsin

IDEMPIERE-5796: for several tables in a row - changes done in ModelIn…

793b03c

…ter… …faceGenerator / ModelClassGenerator Implement suggesion from @hengsin

hengsin requested a review from CarlosRuiz-globalqss May 14, 2024 06:40

CarlosRuiz-globalqss reviewed May 14, 2024

View reviewed changes

IDEMPIERE-5796: for several tables in a row - changes done in ModelIn…

c30a1e6

…terfaceGenerator / ModelClassGenerator Implement suggestion from @CarlosRuiz-globalqss removing duplicated code

CarlosRuiz-globalqss requested changes May 15, 2024

View reviewed changes

IDEMPIERE-5796: for several tables in a row - changes done in ModelI…

f557b6b

…nterfaceGenerator / ModelClassGenerator fix error

CarlosRuiz-globalqss approved these changes May 16, 2024

View reviewed changes

CarlosRuiz-globalqss merged commit 633a3fd into idempiere:master May 16, 2024
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

IDEMPIERE-5796: for several tables in a row #2244

IDEMPIERE-5796: for several tables in a row #2244

nmicoud commented Feb 13, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

CarlosRuiz-globalqss left a comment

nmicoud commented Feb 21, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

nmicoud commented Feb 21, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

nmicoud commented Feb 21, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

nmicoud commented Feb 21, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

nmicoud commented Feb 21, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

hengsin commented May 9, 2024 •

edited

nmicoud commented May 13, 2024

hengsin commented May 14, 2024

CarlosRuiz-globalqss May 14, 2024

nmicoud May 15, 2024

CarlosRuiz-globalqss May 15, 2024

nmicoud May 15, 2024

CarlosRuiz-globalqss May 15, 2024

nmicoud May 15, 2024

CarlosRuiz-globalqss left a comment

IDEMPIERE-5796: for several tables in a row #2244

IDEMPIERE-5796: for several tables in a row #2244

Conversation

nmicoud commented Feb 13, 2024

Pull Request Checklist

Tests

Documentation

CarlosRuiz-globalqss commented Feb 21, 2024

CarlosRuiz-globalqss left a comment

Choose a reason for hiding this comment

nmicoud commented Feb 21, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

nmicoud commented Feb 21, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

nmicoud commented Feb 21, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

nmicoud commented Feb 21, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

nmicoud commented Feb 21, 2024

CarlosRuiz-globalqss commented Feb 21, 2024

hengsin commented May 9, 2024 • edited

nmicoud commented May 13, 2024

hengsin commented May 14, 2024

CarlosRuiz-globalqss May 14, 2024

Choose a reason for hiding this comment

nmicoud May 15, 2024

Choose a reason for hiding this comment

CarlosRuiz-globalqss May 15, 2024

Choose a reason for hiding this comment

nmicoud May 15, 2024

Choose a reason for hiding this comment

CarlosRuiz-globalqss May 15, 2024

Choose a reason for hiding this comment

nmicoud May 15, 2024

Choose a reason for hiding this comment

CarlosRuiz-globalqss left a comment

Choose a reason for hiding this comment

hengsin commented May 9, 2024 •

edited