Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Generate applications without entities among a mixed list in JDL #13280

Merged
merged 2 commits into from
Dec 18, 2020

Conversation

swarajsaaj
Copy link
Contributor

@swarajsaaj swarajsaaj commented Dec 14, 2020

Fixes bug to include applications with or without entities from the
config file, where applications without entities was ignored from a list
of applications with atleast one application with entities

Fix #13141


Please make sure the below checklist is followed for Pull Requests.

When you are still working on the PR, consider converting it to Draft (bellow reviewers) and adding skip-ci label, you can still see CI build result at your branch.

Fixes bug to include applications with or without entities from the
config file, where applications without entities was ignored from a list
of applications with atleast one application with entities

Fix jhipster#13141
@MathieuAA
Copy link
Member

Something bothers me: the need to do this function. Are applications without entities filtered somewhere?
Furthermore, the test doesn't cover the entire flow: testing the JDL importer is also required

@swarajsaaj
Copy link
Contributor Author

swarajsaaj commented Dec 15, 2020

Something bothers me: the need to do this function. Are applications without entities filtered somewhere?
Furthermore, the test doesn't cover the entire flow: testing the JDL importer is also required

@MathieuAA The importState in jdl-importer.js is constructed from entitiesPerApplicationMap through JDLWithApplicationsToJSONConverter (see https://github.com/jhipster/generator-jhipster/blob/main/jdl/jdl-importer.js#L277)

The flow is such that this application-entity map is constructed fromJDLWithApplicationsToJSONConverter.convert, so rather than filtering out, these applications were not at all included in map (see https://github.com/jhipster/generator-jhipster/blob/main/jdl/converters/jdl-to-json/jdl-with-applications-to-json-converter.js#L49)

For test I agree, the test for jdl-importer should be added.

@MathieuAA
Copy link
Member

Once the test is done, I think we should see things more clearly. Moreover, a test for the JDL cli command would be nice too!

@swarajsaaj
Copy link
Contributor Author

Once the test is done, I think we should see things more clearly. Moreover, a test for the JDL cli command would be nice too!

@MathieuAA could you please explain what do you mean by 'JDL cli command' test, is it integration-test? or where are they located?

@MathieuAA
Copy link
Member

In test/cli/jdl/import-jdl.spec.js

….js and import-jdl.js

Add test cases for testing app generation from JDL with two apps (one
with entities and one without)

Fix jhipster#13141
@pascalgrimaud
Copy link
Member

@MathieuAA : if you feel confident, can we merge this for v7 ?

@MathieuAA
Copy link
Member

Yes, let's do this

@MathieuAA MathieuAA merged commit e368c79 into jhipster:main Dec 18, 2020
@pascalgrimaud pascalgrimaud added this to the 7.0.0-beta.0 milestone Dec 18, 2020
@swarajsaaj swarajsaaj deleted the jdl-multiple-apps-entities branch December 19, 2020 03:29
@swarajsaaj
Copy link
Contributor Author

@pascalgrimaud
Copy link
Member

@swarajsaaj : approved

@MathieuAA
Copy link
Member

@swarajsaaj great job :)

@swarajsaaj
Copy link
Contributor Author

swarajsaaj commented Dec 22, 2020

Thanks @MathieuAA :) . Thanks for all the help

coderguy-tech pushed a commit to coderguy-tech/generator-jhipster that referenced this pull request Jun 1, 2021
…pster#13280)

* Generate applications without entities among a mixed list in JDL

Fixes bug to include applications with or without entities from the
config file, where applications without entities was ignored from a list
of applications with atleast one application with entities

Fix jhipster#13141

* Add test cases for multiple apps(entity + non-entity) in jdl-importer.js and import-jdl.js

Add test cases for testing app generation from JDL with two apps (one
with entities and one without)

Fix jhipster#13141
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

JDL: an application is not generated if it doesn't contain entities
3 participants