yasuflatland-lf / damascus

⚔️ CRUD boilerplate generator for Liferay DXP
https://yasuflatland-lf.github.io/damascus-doc/
GNU Lesser General Public License v3.0
50 stars 35 forks source link

Entities should not all be assets #111

Closed dnebing closed 3 years ago

dnebing commented 4 years ago

if you remove the asset: {} attribute from an application definition in base.json, many of the templates (at least in 7.2 where I've been testing) fail throwing numerous FM errors with the null object.

When building a complex application with multiple entities in a service, there will often times need to be a mix of entities, some entities will be assets, but other entities have no reason having asset functionality.

Take Liferay's MessageBoards service as an example. In the single service.xml file for the service module, MBMessage is absolutely defined as an asset because, well, it is an asset. But there are many other entities in there, including for example MBStatsUser which has no asset fields at all because it should never be treated like an asset.

icarrara commented 4 years ago

Tested with 7.3 templates and the issue is the same.

Used the attached base.json with three entities. Removed the asset: {} attribute from two entities.

Done 'damascus create' and I got 21 errors cause the below fields are null: application.asset.assetTitleFieldName and 'application.asset.assetSummaryFieldName'.

Then I surrounded the statements in templates that refer to the above fields with: `<#if (application.asset.assetSummaryFieldName)??>

` `<#if (application.asset.assetSummaryFieldName)??> ` I added the above code to the below templates: - Portlet_XXXXSVC_LocalServiceImpl.java.ftl - Portlet_XXXXSVC_ModelDocumentContributor.java.ftl - Portlet_XXXXSVC_portlet-model-hints.xml.ftl - Portlet_XXXXSVC_Validator.java.ftl - Portlet_XXXXWEB_abstract.jsp.ftl - Portlet_XXXXWEB_admin_abstract.jsp.ftl - Portlet_XXXXWEB_admin_edit.jsp.ftl - Portlet_XXXXWEB_admin_full_content.jsp.ftl - Portlet_XXXXWEB_admin_view_record.jsp.ftl - Portlet_XXXXWEB_AssetRenderer.java.ftl - Portlet_XXXXWEB_edit.jsp.ftl - Portlet_XXXXWEB_full_content.jsp.ftl - Portlet_XXXXWEB_preview.jsp.ftl - Portlet_XXXXWEB_view_record.jsp.ftl After that 'damascus create' done without errors and code was generated. Unfortunately I made some errors in Portlet_XXXXSVC_LocalServiceImpl.java.ftl template so 'blade deploy' fails to compile service code. Here ends my knowledge to modify further the templates.
dnebing commented 4 years ago

I had an additional error in 7.2 in service.xml.ftl because the title and summary fields cannot be missing, null or empty.

For my changes, instead of wrapping in an additional check, I changed the lines to something like:

<#if application.asset?? && application.asset.assetTitleFieldName?? && application.asset.assetTitleFieldName != "" >

icarrara commented 4 years ago

I'll try your change in 7.3 but I can't find service.xml.ftl template anywhere. I checked both 7.2 and 7.3 templates...

dnebing commented 4 years ago

Sorry, it is actually just service.xml, doesn't have the ftl extension on it.

icarrara commented 4 years ago

Moreover, how you changed code without some IF already in place?

For example in Portlet_XXXXSVC_LocalServiceImpl.java.ftl at line 586:

entry.set${application.asset.assetTitleFieldName?cap_first}(""); entry.set${application.asset.assetSummaryFieldName?cap_first}("");

dnebing commented 4 years ago

I didn't get all the way through the files like you have, Ivano. I got far enough to know it was an issue, just not how wide-ranging it was.

yasuflatland-lf commented 4 years ago

@dnebing As we discussed yesterday on Slack, we may want to create a separate template for the Non-Asset version to avoid complexity.

The reasons why I want to separate the templates for each purpose are:

icarrara commented 4 years ago

@yasuflatland-lf

Will this solution allow you to use a single base.json definition file containing a mix of entities, some entities with asset: {} attribute and other without asset: {} attribute ?

I'm referring to the @dnebing 's example of Liferay's MessageBoards service.

yasuflatland-lf commented 3 years ago

This marks as won't fix due to the reasons as follows