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

[BC Break] Sulu 2.6: Possible bug and/or documentation needed? How to migrate to new global blocks? #7409

Closed
spackmat opened this issue May 7, 2024 · 21 comments · Fixed by #7411 or #7423
Assignees
Labels
Bug Error or unexpected behavior of already existing functionality

Comments

@spackmat
Copy link
Contributor

spackmat commented May 7, 2024

Q A
Sulu Version 2.6.1 (since 2.6.0)
PHP Version 8.2 (several versions)
DB Version MariaDB 10.3.39
Browser Version Chrome 124.0.6367.91

Actual Behavior

After upgrading to Sulu 2.6 my included block types lead to different errors (on cache clear or in browser-console):

I have nested blocks that are all on their own xml-file. Like blocks/block_text.xml. Those are type elements with a name attribute like name="block_text" and they are included within the types node of other templates via <xi:include href="blocks/block_text.xml"/>. That worked well until Sulu 2.5:

<?xml version="1.0" ?>
<type xmlns="http://schemas.sulu.io/template/template"
      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      xsi:schemaLocation="http://schemas.sulu.io/template/template http://schemas.sulu.io/template/template-1.0.xsd"
      name="block_text">
  <meta>
    <title lang="en">Textblock</title>
  </meta>
  <properties>
    <property name="text" type="text_area">
      <meta>
        <title lang="en">Content</title>
      </meta>
    </property>
  </properties>
</type>
<!---->
  <types>
    <xi:include href="blocks/block_text.xml"/>
  </types>
<!---->

After upgrading to Sulu 2.6, I get an error on cache clear with that:

[ERROR 1845] Element '{http://schemas.sulu.io/template/template}type': No matching global declaration available for the validation root. (in D:\Projects\sulu.dev.lan/config/templates/blocks\block_text.xml - line 5, column 0)

Interestingly, that error does not raise for elements (instead of blocks) that are defined and included the exact same way and it also does not raise, when I copy the type definition directly into its parent instead of including it via XInclude. This is my workaround for now.

Since the blocks have changed in Sulu 2.6 to global blocks, I then changed their definition according to the current documentation in https://docs.sulu.io/en/2.6/book/templates.html#using-global-blocks to the following code:

<?xml version="1.0" ?>
<template xmlns="http://schemas.sulu.io/template/template"
          xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
          xsi:schemaLocation="http://schemas.sulu.io/template/template http://schemas.sulu.io/template/template-1.0.xsd">
  <key>block_text</key>
  <meta>
    <title lang="en">Textblock</title>
  </meta>
  <properties>
    <property name="text" type="text_area">
      <meta>
        <title lang="en">Content</title>
      </meta>
    </property>
  </properties>
</template>
<!---->
  <types>
    <type ref="block_text"/>
  </types>
<!---->

That removes the error on cache clear, but now, I cannot edit pages containing that block type any more. Instead I get the following error inside the browser console:

uncaught (in promise) Error: can't resolve reference #/definitions/block_text from id #

Adding an name-attribute to the ´type´ node with the ref leads to an xml error, as that ref and name are not allowed together.

Expected Behavior

I expected either the old xi:include way to still work for included block types or the new global block handling to work with my existing content. Or at least a documented way to migrate my existing content to work with the new global blocks.

Steps to Reproduce

Have a Sulu 2.5 instance with nested block based content using xi:include and upgrade to Sulu 2.6. We have two problems here: The first is the one with the new xi:include error for included block types and the other one that the needed migration for existing content that is not documented.

Possible Solutions

I don't know how to solve that, but I guess that the existing content has to be migrated to be compatible to the new global block handling. Some documentation on how to do that would be helpful. Better would be to change the implementation in a way that no migration would be needed at all when the blocks keep their name.

For now, my workaround is to not include my block types via xi:include, but to copy their definitions directly into the templates using them.

@spackmat spackmat added the Bug Error or unexpected behavior of already existing functionality label May 7, 2024
@alexander-schranz
Copy link
Member

Thx for the report maybe we can add a line to the UPGRADE.md. The config/templates/blocks directory is used now for global blocks and so should not contain any none templates files.

If you did use it currently for xi:includes I recommend migrating to config/templates/includes/blocks directory like it is currently done in the Sulu Demo: https://github.com/sulu/sulu-demo/tree/master/config/templates/includes/blocks.

Migrating to global blocks is not a requirement for 2.6 sulu supports still local blocks and local block types. Some projects may not even have unique block names that they can easy migrate to global blocks. But beside the new reserved directory there is no change here required for the upgrade you can change the config/templates/blocks -> config/templates/includes/blocks.

@spackmat
Copy link
Contributor Author

spackmat commented May 7, 2024

Thank you, also for the direct reaction with a Docs PR. That may help others to not spend hours on that surprising BC break.

Is there a way to migrate existing content to the new global blocks? I like the idea behind that new architecture.

@mamazu
Copy link
Contributor

mamazu commented May 7, 2024

The migration must be done manually sadly. Mostly just copy and paste.

@spackmat
Copy link
Contributor Author

spackmat commented May 7, 2024

Sorry to open that again, but I now tried to add a new global block with a new name using the documented implementation and I still get that console error, when I want to edit any page:

Uncaught (in promise) Error: can't resolve reference #/definitions/block_test from id #

Am I missing something here? Must the global blocks be registered or something?

@remdan
Copy link

remdan commented May 8, 2024

I have installed a new clone of sulu/skeleton and followed the example of globel blocks.

I also get the same error message...

@spackmat
Copy link
Contributor Author

spackmat commented May 8, 2024

@alexander-schranz can you open the issue again, as the second described problem seems to be not fixed?

@alexander-schranz
Copy link
Member

alexander-schranz commented May 8, 2024

@spackmat the admin build wasu pdated to the latest version: https://docs.sulu.io/en/2.6/upgrades/upgrade-2.x.html#update-the-admin-javascript-build ? If you normally forget that your browser dev tools should show a warning at the very beginning of the sulu admin start.

If I understand you only get the error in the frontend not in the cache:clear?

@wachterjohannes do you have any idea what is causing the issue?

@spackmat
Copy link
Contributor Author

spackmat commented May 8, 2024

Yes, i updated the js build multiple times. And yes, the error is in the browser-console.

@wachterjohannes
Copy link
Member

@ remdan can you share your test on a fresh skeleton? just push it as it is on a new repo and share it here! then we can take a look at it

@remdan
Copy link

remdan commented May 9, 2024

I have found a difference!

the example works for me

<block name="blocks" default-type="text_block" minOccurs="0">
    <types>
        <!-- using global blocks and only referencing the block -->
        <type ref="text_block" />

        <!-- default definition of a block type without using global blocks -->
        <type name="editor">
            <meta>
                <title lang="en">Editor</title>
                <title lang="de">Editor</title>
            </meta>
            <properties>
                <property name="text_editor" type="text_editor">
                    <meta>
                        <title lang="en">Text Editor</title>
                        <title lang="de">Texteditor</title>
                    </meta>
                </property>
            </properties>
        </type>
    </types>
</block>

but if I put this code in a Section it will break:

<section name="organizationalDetails">
      <!-- ... -->

      <properties>
          <block name="blocks" default-type="text_block" minOccurs="0">
              <types>
                  <!-- using global blocks and only referencing the block -->
                  <type ref="text_block" />

                  <!-- default definition of a block type without using global blocks -->
                  <type name="editor">
                      <meta>
                          <title lang="en">Editor</title>
                          <title lang="de">Editor</title>
                      </meta>
                      <properties>
                          <property name="text_editor" type="text_editor">
                              <meta>
                                  <title lang="en">Text Editor</title>
                                  <title lang="de">Texteditor</title>
                              </meta>
                          </property>
                      </properties>
                  </type>
              </types>
          </block>
      </properties>
  </section>

ref_error.js:6 Uncaught (in promise) Error: can't resolve reference #/definitions/text_block from id #

@mamazu
Copy link
Contributor

mamazu commented May 10, 2024

I think the problem is that type-refs inside of blocks don't work. I'm running into the same issue.

@spackmat
Copy link
Contributor Author

spackmat commented May 13, 2024

I can confirm what @remdan said: In our case, the blocks were also included within a section. When I move them out of that section the error in the backend disappears. I consider this a bug as I expect the new global blocks to also work inside sections. Or maybe I've missed a deprecation for sections.

Edit: Our blocks all had distinct names, so no content migration was needed at all. Just changing the definition to global block templates and using a type reference instead of an include (and not within a section!) already does the job. Maybe that could also be documented as a migration path for cases with already distinct named blocks. Besides fixing or at least documenting the bug with the sections.

Edit2: Seems not to work with nested blocks. In the type list of the parent blocks I can see the referenced global types, but with their key (with its first letter uppercased) and not the name from the meta of the block template (Block_text instead of Textblock). When I switch to one of those, the fields are not shown without an error raised inside the browser console. But the preview shows the correct templates.

@alexander-schranz
Copy link
Member

alexander-schranz commented May 14, 2024

Would be nice if the ones who run into issues give #7423 a try and check if that solves all the issues. @spackmat @remdan @mamazu. Custom JS build of the admin is required.

@spackmat
Copy link
Contributor Author

Hey, I patched my installation with the current version of #7423 (the one with two commits). Then I built the admin js and observed the following:

  • When I put my blocks back into a section, the console error vanishes, but now, all blocks are empty in the backend: I can open them, but they all have no fields. Removing the section again, and the fields come back.
  • The nested blocks still have no fields and their wrong name as described in my last post, so no changes here.

@wachterjohannes
Copy link
Member

@spackmat i have done some changes in the merge-request! can you recheck and update your admin build? That should fix your empty forms in the admin

@spackmat
Copy link
Contributor Author

spackmat commented May 14, 2024

@wachterjohannes I replaced my sulu/sulu dependency with your branch: "sulu/sulu": "dev-fix-global-blocks-edge-cases" (and added your repo) and rebuild the admin build: Still the same: No changes for nested blocks and blocks within a section still have no fields.

Edit: Adding the repo works this way:

    "repositories": {
        "sulu": {
            "type": "vcs",
            "url": "git@github.com:wachterjohannes/sulu.git"
        }
    },

@wachterjohannes
Copy link
Member

@spackmat this is really strange - have you also done a new an npm install in the assets/admin directory after composer update? This is neccesary since the last npm upgrade in 2.6

@spackmat
Copy link
Contributor Author

spackmat commented May 14, 2024

@wachterjohannes yes, npm install and npm run build. Both have run without errors.

Edit: Did it again, still the same.

@spackmat
Copy link
Contributor Author

Both problems are fixed now with the latest commits in #7423. So this issue can be closed, when the PR is merged.

@chirimoya
Copy link
Member

Both problems are fixed now with the latest commits in #7423. So this issue can be closed, when the PR is merged.

Thanks for testing!

@alexander-schranz
Copy link
Member

alexander-schranz commented May 16, 2024

Released in 2.6.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Error or unexpected behavior of already existing functionality
Projects
None yet
6 participants