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.

Sign up for GitHub

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

Jump to bottom

Entity Web API generation #597

Merged
merged 4 commits into from
Jun 9, 2021
Merged

Entity Web API generation #597

merged 4 commits into from
Jun 9, 2021

Conversation

bohdan-harniuk
Copy link
Collaborator

Description (*)
This PR adds the possibility of generating REST API endpoints for managing entity generated with the entity creator feature.

What was done:

  1. added to entity creator dialog checkbox that indicates if REST API endpoints should be created (by default is disabled)
  2. generates API for the DeleteByIdCommand class, adds declaration to the webapi.xml file, replaces the original class type on the newly created interface whenever it is used
  3. generates API for the SaveCommand class, adds declaration to the webapi.xml file, replaces the original class type on the newly created interface whenever it is used
  4. generates API for the GetListQuery class, adds declaration to the webapi.xml file, replaces the original class type on the newly created interface whenever it is used
  5. edited tests that related to the affected parts of the functionality

Screenshot 2021-06-07 at 09 08 35

Screenshot 2021-06-07 at 09 18 54

Screenshot 2021-06-07 at 09 18 00

Screenshot 2021-06-07 at 09 17 25

Screenshot 2021-06-07 at 09 15 49

Contribution checklist (*)

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with integration/functional tests (if applicable)
  • All automated tests passed successfully (all builds are green)

@eduard13 eduard13 self-assigned this Jun 7, 2021
@eduard13 eduard13 self-requested a review June 7, 2021 06:55
eduard13
eduard13 requested changes Jun 8, 2021
View reviewed changes
Copy link
Contributor

@eduard13 eduard13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @bohdan-harniuk, thank you for this cool improvement. Please check the below suggestions, and additionally I have a concern regarding the checkbox and what happens if you don't select the pointed checkboxes.

image

So, basically we should have them as dependency or make them individually, because the end-user may not need the Admin UiComponents, but needs only the WebAPI required files.

Please let me know if there are any questions.
Thank you.

src/com/magento/idea/magento2plugin/actions/generation/data/DeleteEntityControllerFileData.java Outdated
@@ -26,12 +27,14 @@ public DeleteEntityControllerFileData(
final @NotNull String entityName,
final @NotNull String moduleName,
final @NotNull String acl,
final @NotNull String entityId
final @NotNull String entityId,
final boolean isDeleteCommandHasInterface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's replace this one to a more readable version: hasDeleteCommandInterface or deleteCommandHasInterface.

Suggested change
final boolean isDeleteCommandHasInterface
final boolean hasDeleteCommandInterface

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

src/com/magento/idea/magento2plugin/actions/generation/data/SaveEntityControllerFileData.java Outdated
@@ -35,7 +37,8 @@ public SaveEntityControllerFileData(
final @NotNull String entityId,
final @NotNull String dtoName,
final @NotNull String dtoInterfaceName,
final boolean isDtoWithInterface
final boolean isDtoWithInterface,
final boolean isSaveCommandHasInterface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

src/com/magento/idea/magento2plugin/actions/generation/data/UiComponentDataProviderData.java Outdated
*/
public UiComponentDataProviderData(
final String name,
final String path,
final String entityName,
final String entityIdFieldName
final String entityIdFieldName,
final boolean isQueryHasInterface
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@bohdan-harniuk
Copy link
Collaborator Author

Hello, @eduard13! Thank you for the great ideas in your code review!

What was done:

  1. fixed bug when creating entity without DTO interface (generating was stuck on the data mapper generator)
  2. changed bool parameters names (I had to name methods as IsHas... because of AvoidFieldNameMatchingMethodName PMD rule)
  3. added commands, query and data mapper generation when creating admin UI components checkbox is unchecked

Thank you!!! Now it looks better!

Could you please proceed with the code review?

@bohdan-harniuk bohdan-harniuk requested a review from eduard13 June 9, 2021 09:33
eduard13
eduard13 approved these changes Jun 9, 2021
View reviewed changes
Copy link
Contributor

@eduard13 eduard13 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job 👍

@eduard13 eduard13 merged commit 994c3b3 into magento:4.0.0-develop Jun 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eduard13 eduard13 eduard13 approved these changes

Assignees

@eduard13 eduard13

Labels
Award: test coverage Medium Partner: Atwix partners-contribution Progress: done
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bohdan-harniuk @eduard13 @magento-engcom-team