search

liferay / generator-liferay-fragments

Yeoman generator for creating and maintaining Liferay Fragment projects
https://www.npmjs.com/package/generator-liferay-fragments
MIT License
30 stars 31 forks source link

Correctly identify fragment composition files #232

Closed yuchi closed 2 years ago

yuchi commented 2 years ago

This PR supersedes #231

The problem is pretty simple, and so is the solution.

Currently Fragment Compositions are not retrieved since they are searched with the following glob:

<root>/src/<collection>/*/fragment-composition.json

This is not where they are stored since they are grouped inside a fragment-compositions folder.

This PR follows the lead of all other _get* functions in the get-project-content.ts file by using the following glob (please notice the additional ** in the glob):

<root>/src/<collection>/**/*/fragment-composition.json

Other changes

Tests

As requested, I implemented some testing around Fragment Compositions. Unfortunately the testing infrastructure for getProjectContent was insufficient, since using arrayContaining for both fragments and fragmentCompositions required both arrays to be populated.

Also I introduced the use of Jest’s it.each instead of […].forEach(obj => it('…', () => …))

Editorconfig

In order to not make compression and writing tests frown at me, I had to manually fix new lines in the fixtures and disable Editorconfig to not re-introduce them at every file save.

Types

I introduced the thumbnailPath and description properties to IFragmentCompositionMetadata, which were missing.

yuchi commented 2 years ago

Hi @p2kmgcl, this PR is now ready to be reviewed by the team. Please take some time to read the commit messages, they provide some valuable information on “why” I made some changes. Sorry if I did not sqaush everything, but the changes where a little bit more pervasive than I expected and I personally prefer to leave different explicit messages in git blames and file history.

yuchi commented 2 years ago

That change on .editorconfig has been removed, and I applied the approach discussed (removing the JSON.parse/JSON.stringify processing of composition definitions).

Have a look again at the changes and please run the CI tests.

p2kmgcl commented 2 years ago

Hey again! (I've been on PTO since Wednesday :sweat_smile:)

Just one more thing and I think we are good to go, can you add "generators" directory to the .prettierignore file? It is generated when running yarn run build and it is shouldn't be checked by prettier.

yuchi commented 2 years ago

It should be there already: https://github.com/liferay/generator-liferay-fragments/pull/232/files#diff-b640b344ee7f3f03d2a443795a5d0708ef50e2e6e34214109ab2aad13ad6ba98R1

So it should be ready do be merged! 🎉

yuchi commented 2 years ago

Thank you @p2kmgcl ! Glad to contribute!