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

Support for builder specific templates #9654

Merged
merged 8 commits into from Feb 13, 2021

Conversation

alanhartless
Copy link
Contributor

@alanhartless alanhartless commented Feb 9, 2021

Q A
Branch? "features"
Bug fix? no
New feature? yes
Deprecations? no
BC breaks? no
Automated tests included? yes
Related user documentation PR URL n/a
Related developer documentation PR URL mautic/developer-documentation#185

Description:

This enables plugins to identify itself as providing a "builder" integration. Mautic will then filter themes based on the builder that is enabled (will only acknowledge the first one it finds if there happens to be multiple for a given feature).

Companion PR for the GrapeJS plugin is here mautic/plugin-grapesjs-builder#42.

Steps to test this PR:

  1. Load up this PR (must be done locally because GrapeJS has to be installed during the steps)
  2. You should see all legacy themes when creating an email or landing page
  3. Install the GrapeJS plugin with Registers the plugin as a "builder" plugin-grapesjs-builder#42 patched into it
  4. Leave the plugin disabled and create a new email. All Legacy themes should be displayed.
  5. Enable the GrapeJS plugin in /s/plugins
  6. Create a new email and note that no themes are displayed (because no themes are available yet)
  7. Go to the config.json file of the "blank" theme and add "builder": "grapesjsbuilder" to the json object
  8. Create an email again and note that the blank theme now shows up (and no others)
  9. Go to /s/themes and you should see Legacy badge for all themes except Blank which will show will GrapesJS.

@cla-bot cla-bot bot added the cla-signed The PR contributors have signed the contributors agreement label Feb 9, 2021
@codecov
Copy link

codecov bot commented Feb 9, 2021

Codecov Report

Merging #9654 (870652f) into features (cd7e257) will increase coverage by 0.01%.
The diff coverage is 84.41%.

Impacted file tree graph

@@              Coverage Diff               @@
##             features    #9654      +/-   ##
==============================================
+ Coverage       38.33%   38.35%   +0.01%     
- Complexity      33939    33957      +18     
==============================================
  Files            1982     1984       +2     
  Lines          104948   104997      +49     
==============================================
+ Hits            40236    40274      +38     
- Misses          64712    64723      +11     
Impacted Files Coverage Δ Complexity Δ
.../bundles/CoreBundle/Controller/ThemeController.php 0.00% <0.00%> (ø) 29.00 <0.00> (ø)
...rationsBundle/Helper/BuilderIntegrationsHelper.php 68.18% <68.18%> (ø) 11.00 <11.00> (?)
...dencyInjection/Compiler/BuilderIntegrationPass.php 80.00% <80.00%> (ø) 2.00 <2.00> (?)
app/bundles/CoreBundle/Helper/ThemeHelper.php 62.10% <95.74%> (+3.10%) 83.00 <11.00> (+5.00)
.../bundles/IntegrationsBundle/IntegrationsBundle.php 100.00% <100.00%> (ø) 1.00 <0.00> (ø)

…e of duplicating internal state results when calling getInstalledThemes multiple times
@alanhartless alanhartless marked this pull request as ready for review February 9, 2021 18:37
@alanhartless alanhartless changed the title WIP Support for builder specific templates Support for builder specific templates Feb 9, 2021
@RCheesley RCheesley added builder-legacy Anything related to the legacy email or landing page builders strategic-initiative T1 Low difficulty to fix (issue) or test (PR) essential This must be done to close the milestone labels Feb 11, 2021
@RCheesley RCheesley added this to the 3.3.0 milestone Feb 11, 2021
Copy link
Member

@kuzmany kuzmany left a comment

Choose a reason for hiding this comment

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

Works as describe.
Good job

found that it looks like when you edit an existing resource, you can make changes and save them, however the wrong template will show in the dropdown because the one that was used originally is not available (assuming they did not add the line in the config file for the template). We need to document this if that is the case and make sure that people are aware, before they start working with the builder.

This is weird, but I think older templates are editable in grapejs in good way.
Still this is better like previous state, and we can improve it later...

@kuzmany kuzmany added the pending-test-confirmation PR's that require one test before they can be merged label Feb 13, 2021
@RCheesley RCheesley merged commit 2d3b108 into mautic:features Feb 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builder-legacy Anything related to the legacy email or landing page builders cla-signed The PR contributors have signed the contributors agreement essential This must be done to close the milestone pending-test-confirmation PR's that require one test before they can be merged strategic-initiative T1 Low difficulty to fix (issue) or test (PR)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants