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

#issue-179 Added email template generation #350

Merged
merged 15 commits into from
Feb 15, 2021
Merged

#issue-179 Added email template generation #350

merged 15 commits into from
Feb 15, 2021

Conversation

serhiyzhovnir
Copy link
Contributor

Description (*)
This PR adds the generation for the email templates.

Screenshot 2020-10-24 at 12 55 15

Screenshot 2020-10-24 at 12 56 07

Screenshot 2020-10-24 at 12 56 35

Screenshot 2020-10-24 at 12 56 49

Fixed Issues (if relevant)

  1. Fixes Action/Code Generation. New email template generation #179

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)

@coderimus coderimus self-assigned this Oct 24, 2020
@coderimus coderimus self-requested a review October 24, 2020 10:26
@VitaliyBoyko
Copy link
Contributor

I believe the possible area should be always fronted, shouldn’t it?

@serhiyzhovnir
Copy link
Contributor Author

Hi @VitaliyBoyko
Thank you for your question.
The adminhtml and frontend areas are allowed.
Here are proofs from Magento core:
Screenshot 2020-10-24 at 13 37 18

coderimus
coderimus suggested changes Oct 24, 2020
View reviewed changes
Copy link
Contributor

@coderimus coderimus left a comment

Choose a reason for hiding this comment

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

Hello @serhiyzhovnir
Thank you for the provided improvement. It will help a lot for the development process improvement.

I made some tests and found failed scenarios. Please, check my report below:

Scenario 1

  1. Create a template for the frontend area. Let's name it custom_template
  2. Start creating a new template for the frontend area. Choose the custom_template name for the template.
  3. When you will try to creat it, you will get an error reported that the file custom_template.html already exists. This is correct. But in the email_templates.xml we will have an entry. Please, change this to the next: when an error appeared during the create template action we should not update the XML config file.
    image

Scenario 2

  1. Try to create templates with the same id
  2. Current result: nodes in the email_templates.xml has the same id. I think this is a bug. We should not allowed create email templates with the same id for the selected area
    image

@serhiyzhovnir serhiyzhovnir changed the title #issue-179 Added email template generation #issue-179 Added email template generation [WIP] Oct 25, 2020
@VitaliyBoyko VitaliyBoyko changed the base branch from 2.1.0-develop to 3.2.0-develop February 3, 2021 15:02
@VitaliyBoyko
Copy link
Contributor

@serhiyzhovnir

Are you up to proceed with this?

@serhiyzhovnir
Copy link
Contributor Author

Hi @VitaliyBoyko
Could you check the adjustments?
Thank you!

@VitaliyBoyko
Copy link
Contributor

Hey @coderimus please proceed with this.

@coderimus
Copy link
Contributor

@VitaliyBoyko will process it before lunch today.

@coderimus coderimus changed the title #issue-179 Added email template generation [WIP] #issue-179 Added email template generation Feb 15, 2021
coderimus
coderimus suggested changes Feb 15, 2021
View reviewed changes
Copy link
Contributor

@coderimus coderimus left a comment

Choose a reason for hiding this comment

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

@serhiyzhovnir
I fetched and pulled the latest changes from the PR branch. When I tried to test the latest revision I faced the next problem: when I open a new email template generation form an error occurred and nothing happened on pressing OK button. Please, validate Null pointer exception. CC @VitaliyBoyko
image

@serhiyzhovnir
Copy link
Contributor Author

Hi @coderimus
The issue with the Null pointer is fixed.
May you review this again?
Thanks.

@coderimus
Copy link
Contributor

@serhiyzhovnir thank you for the quick fix 👍 The problem gone and the issue I reported previously also fixed. QA passed. Approved 👍

@coderimus coderimus merged commit 374ecef into magento:3.2.0-develop Feb 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@coderimus coderimus coderimus approved these changes

Assignees

@coderimus coderimus

Labels
Advanced Award: test coverage Event: MageCONF CD 2020 Expert Partner: Atwix partners-contribution Progress: done
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Action/Code Generation. New email template generation
4 participants
@serhiyzhovnir @VitaliyBoyko @coderimus @magento-engcom-team