12525 move web2print to bundle by mattamon · Pull Request #14175 · pimcore/pimcore

mattamon · 2023-01-30T10:01:11Z

Changes in this pull request

Resolves #12525

Additional info

Moving web to print functionality into PimcoreWebToPrint
Able to install and uninstall via Installer. Permissions are in the Bundle
Moved translations into the bundle
Moved documents printabstract, printcontainer and printpage to bundle

Moving Migration into CoreBundle Starting to add Document class loader

# Conflicts: # .github/ci/files/config/bundles.php # bundles/AdminBundle/templates/admin/index/index.html.twig # bundles/CoreBundle/config/pimcore/default.yaml # bundles/CoreBundle/src/DependencyInjection/PimcoreCoreExtension.php # bundles/Web2PrintBundle/public/js/web2print.js # composer.json # models/DataObject/ClassDefinition/Data/AdvancedManyToManyObjectRelation.php # models/DataObject/ClassDefinition/Data/AdvancedManyToManyRelation.php # models/DataObject/ClassDefinition/Data/Classificationstore.php

Reverting routing changes to document controllers

Changed fixed document routes

github-actions · 2023-01-30T10:01:28Z

Review Checklist

Target branch (10.5 for bug fixes, others 11.x)
Tests (if it's testable code, there should be a test for it - get help)
Docs (every functionality needs to be documented, see here)
Migration incl. install.sql (e.g. if the database schema changes, ...)
Upgrade notes (deprecations, important information, migration hints, ...)
Label
Milestone

mattamon · 2023-01-30T12:39:55Z

bundles/CoreBundle/src/Migrations/Version20220829132224.php

        $configs = $this->loadLegacyConfigs('web2print.php');
-        if (count($configs) > 0) {
-            $web2PrintConfigs = Config::getWeb2PrintConfig();
+        if (count($configs) > 0 && class_exists('Pimcore\Bundle\WebToPrintBundle\Config')) {


I am not to sure about changing this migration

I'd wrap the entire function up() with a IF block that checks if it exists under the SettingStore and in case "skips" everything.
The down() is empty anyways.
Nevermind, i've misunderstood the issue.

I am not sure how this check should look like. Do you have an example?

lib/Perspective/Config.php

kingjia90

Found some minor issues after a quick check, but overall seems working fine

bundles/WebToPrintBundle/translations/admin.en.yaml

kingjia90 · 2023-01-31T14:04:59Z

bundles/CoreBundle/src/Migrations/Version20220829132224.php

        $configs = $this->loadLegacyConfigs('web2print.php');
-        if (count($configs) > 0) {
-            $web2PrintConfigs = Config::getWeb2PrintConfig();
+        if (count($configs) > 0 && class_exists('Pimcore\Bundle\WebToPrintBundle\Config')) {


I'd wrap the entire function up() with a IF block that checks if it exists under the SettingStore and in case "skips" everything.
The down() is empty anyways.
Nevermind, i've misunderstood the issue.

bundles/CoreBundle/src/Migrations/Version20221220152444.php

bundles/AdminBundle/public/js/pimcore/document/document.js

kingjia90 · 2023-01-31T14:39:28Z

One input but unrelated to the changes in the PR is: how should it behave when you remove/deactivate the web2print bundle and you still have printpage and printcontainers? What about a cleanup tool or a "archived/invalid documents" tree node and/or filter to hide them? And the generated PDFs and job file? Just wondering if uninstaller should take care of this or is up to the devs

# Conflicts: # bundles/WebToPrintBundle/src/Controller/Document/PrintpageControllerBase.php

Adding missing translations Adding category to permissions in migration

mattamon · 2023-02-02T12:46:15Z

I was wondering about this too, how we should handle those files, because with this PR we do not even have a fallback for the printpage or printcontainer in the core.

@brusch wdyt? what should happen with printcontainer or printpage documents after the bundle gets uninstalled or disabled?

# Conflicts: # .github/ci/files/config/bundles.php # doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md

kingjia90

Thank you for fixing most of the stuff from the list in the last review.
I've found some other points, but overall looks and works fine to me.

bundles/WebToPrintBundle/src/Installer.php

lib/Perspective/Config.php

lib/Twig/Extension/DocumentHelperExtensions.php

tests/Model/Document/DocumentTest.php

kingjia90

Please remove this checkbox and related translation/docs

pimcore/bundles/WebToPrintBundle/public/js/settings.js

Lines 257 to 266 in 3eca81b

    
               fieldLabel: t('web2print_enable_in_default_view'), 
        
               xtype: "checkbox", 
        
               name: "enableInDefaultView", 
        
               checked: this.getValue("enableInDefaultView") 
        
           }, { 
        
               xtype: "displayfield", 
        
               hideLabel: true, 
        
               width: 600, 
        
               value: t('web2print_enable_in_default_view_txt'), 
        
               cls: "pimcore_extra_label_bottom"

which was used to define if the web2print should be activated or not

pimcore/doc/Development_Documentation/03_Documents/02_Document_Types/15_Print_Documents.md

Line 8 in 3eca81b

    
           By default, web-to-print documents are disabled. To enable them, you need to activate them in the web-to-print settings:

Also in the migration needs to check wheter the checkbox was previously marked by that config

pimcore/bundles/CoreBundle/src/Migrations/Version20221220152444.php

Line 37 in 3eca81b

    
           if(!SettingsStore::get('BUNDLE_INSTALLED__Pimcore\\Bundle\\WebToPrintBundle\\PimcoreWebToPrintBundle', 'pimcore')) {

# Conflicts: # bundles/AdminBundle/src/Controller/Admin/SettingsController.php

Adding cleanup command Adjust migration to remove or add config Adjust installer to only remove permissions Updated documentation

bundles/CoreBundle/src/Migrations/Version20221220152444.php

kingjia90

found some other minor tweak to do while scanning through the last commit

bundles/WebToPrintBundle/src/Installer.php

lib/Twig/Extension/DocumentHelperExtensions.php

# Conflicts: # bundles/WebToPrintBundle/src/Processor/HeadlessChrome.php

Only show enable bundle warning if bundle was marked as installed

Remove/add permissions in migration

bundles/CoreBundle/src/Migrations/Version20221220152444.php

bundles/WebToPrintBundle/src/Installer.php

doc/Development_Documentation/23_Installation_and_Upgrade/07_Updating_Pimcore/12_V10_to_V11.md

Adding deleting queries instead of doing it in the migration Adding consoleoutput on uninstall

kingjia90

I will commit these final suggestions and looks good to be merged

bundles/WebToPrintBundle/src/Installer.php

doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md

mattamon added 9 commits December 21, 2022 15:10

Moving web2print to bundle

8756e93

Renaming Web2PrintBundle to PimcoreWeb2PrintBundle

1023e42

Moving Migration into CoreBundle Starting to add Document class loader

Updating bundle to refactoring tips

3136491

Adding upgrade notes

cc3822e

Reverting routing changes to document controllers

Remove document data loader

b46baa2

Moving messenger

bafc989

Changed fixed document routes

Merge branch '11.x' into 12525-move-web2print-to-bundle

edd93e7

Changing routing for save and remove from session

bc105cd

mattamon added the Task label Jan 30, 2023

mattamon added this to the 11.0.0 milestone Jan 30, 2023

mattamon marked this pull request as draft January 30, 2023 10:01

mattamon added 2 commits January 30, 2023 11:14

Change bundle namespace

c2f0b47

Update tests

9555a2c

dvesh3 assigned kingjia90 Jan 30, 2023

mattamon added 2 commits January 30, 2023 12:35

Fix test errors

8b3f24c

Add type

b47e68e

mattamon commented Jan 30, 2023

View reviewed changes

Change category to web2print

9b4adfe

mattamon commented Jan 30, 2023

View reviewed changes

lib/Perspective/Config.php Outdated Show resolved Hide resolved

Update docs and deleting not used files

9a36587

mattamon marked this pull request as ready for review January 30, 2023 13:12

mattamon requested a review from kingjia90 January 30, 2023 13:12

kingjia90 requested changes Jan 31, 2023

View reviewed changes

kingjia90 mentioned this pull request Feb 2, 2023

[Task]: Add Gotenberg support in Web2Print and Asset Document Preview #14212

Merged

mattamon added 2 commits February 2, 2023 11:21

Merge branch '11.x' into 12525-move-web2print-to-bundle

2bf123a

# Conflicts: # bundles/WebToPrintBundle/src/Controller/Document/PrintpageControllerBase.php

Adding task param

9f8b635

Adding missing translations Adding category to permissions in migration

mattamon added 2 commits February 8, 2023 15:52

Adding doctrine compatibility layer

0e0b21e

Merge branch '11.x' into 12525-move-web2print-to-bundle

3eca81b

# Conflicts: # .github/ci/files/config/bundles.php # doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md

mattamon requested a review from kingjia90 February 10, 2023 10:02

kingjia90 requested changes Feb 10, 2023

View reviewed changes

bundles/WebToPrintBundle/src/Installer.php Show resolved Hide resolved

lib/Perspective/Config.php Outdated Show resolved Hide resolved

lib/Twig/Extension/DocumentHelperExtensions.php Outdated Show resolved Hide resolved

tests/Model/Document/DocumentTest.php Show resolved Hide resolved

kingjia90 reviewed Feb 10, 2023

View reviewed changes

This comment was marked as resolved.

Sign in to view

mattamon added 2 commits February 13, 2023 08:10

Merge branch '11.x' into 12525-move-web2print-to-bundle

60d1933

# Conflicts: # bundles/AdminBundle/src/Controller/Admin/SettingsController.php

Move translations

7fe2f54

Adding cleanup command Adjust migration to remove or add config Adjust installer to only remove permissions Updated documentation

mattamon requested a review from kingjia90 February 14, 2023 11:11

kingjia90 reviewed Feb 14, 2023

View reviewed changes

bundles/CoreBundle/src/Migrations/Version20221220152444.php Outdated Show resolved Hide resolved

kingjia90 requested changes Feb 14, 2023

View reviewed changes

bundles/WebToPrintBundle/src/Installer.php Outdated Show resolved Hide resolved

lib/Twig/Extension/DocumentHelperExtensions.php Outdated Show resolved Hide resolved

mattamon added 6 commits February 15, 2023 09:40

Change uninstall info and remove TwigTest

0bf876a

Merge branch '11.x' into 12525-move-web2print-to-bundle

cbe16a4

# Conflicts: # bundles/WebToPrintBundle/src/Processor/HeadlessChrome.php

Add use for Pimcore\Tool\Console

ae76d24

Make installer idempotent and enums unique

6163bca

Make migration idempotent. Remove settings store setting in down

fe2eee2

Only show enable bundle warning if bundle was marked as installed

Add Upgrading notes and warnings

a414312

Remove/add permissions in migration

kingjia90 requested changes Feb 16, 2023

View reviewed changes

mattamon added 2 commits February 16, 2023 14:20

Adding command documentation

dcb0874

Adding deleting queries instead of doing it in the migration Adding consoleoutput on uninstall

Adding doc

da3c592

mattamon requested a review from kingjia90 February 16, 2023 13:26

kingjia90 reviewed Feb 16, 2023

View reviewed changes

bundles/WebToPrintBundle/src/Installer.php Outdated Show resolved Hide resolved

doc/Development_Documentation/23_Installation_and_Upgrade/09_Upgrade_Notes/README.md Outdated Show resolved Hide resolved

Apply suggestions from code review

2323763

kingjia90 merged commit 5970400 into 11.x Feb 16, 2023

kingjia90 deleted the 12525-move-web2print-to-bundle branch February 16, 2023 16:59

This was referenced Feb 17, 2023

[Task]: Move print related events in DocumentEvents to Web2PrintBundle #14377

Closed

[Task]: Enable PimcoreWebToPrintBundle in bundles.php pimcore/demo#419

Closed

brusch mentioned this pull request Mar 6, 2023

Unrelated code introduced by #14175 #14542

Closed

kingjia90 mentioned this pull request Mar 7, 2023

[Task][Web2PrintBundle]: Fix Add PrintPage position in right-click menu #14554

Merged

mattamon mentioned this pull request Apr 4, 2023

Readd the document types to context menu for web2print #14852

Merged

	fieldLabel: t('web2print_enable_in_default_view'),
	xtype: "checkbox",
	name: "enableInDefaultView",
	checked: this.getValue("enableInDefaultView")
	}, {
	xtype: "displayfield",
	hideLabel: true,
	width: 600,
	value: t('web2print_enable_in_default_view_txt'),
	cls: "pimcore_extra_label_bottom"

Conversation

mattamon commented Jan 30, 2023

Changes in this pull request

Additional info

Uh oh!

github-actions bot commented Jan 30, 2023 • edited by kingjia90 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review Checklist

Uh oh!

mattamon Jan 30, 2023

Choose a reason for hiding this comment

Uh oh!

kingjia90 Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mattamon Feb 2, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kingjia90 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kingjia90 Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

kingjia90 commented Jan 31, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattamon commented Feb 2, 2023

Uh oh!

kingjia90 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kingjia90 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

kingjia90 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

kingjia90 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Jan 30, 2023 •

edited by kingjia90

Loading

kingjia90 Jan 31, 2023 •

edited

Loading

kingjia90 Jan 31, 2023 •

edited

Loading

kingjia90 commented Jan 31, 2023 •

edited

Loading

kingjia90 left a comment •

edited

Loading

kingjia90 left a comment •

edited

Loading