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

Fix/improve dataobject block behaviour #15927

Merged
merged 2 commits into from
Nov 16, 2023

Conversation

gjorgic
Copy link
Contributor

@gjorgic gjorgic commented Sep 12, 2023

WHAT

I notice few issues with dataobject block

  • If block is nested into brick, than on the end we do not have information about that block inside context var (for example, if you use OptionProvider for selectoption in that block, you will have information about object, object key, brick name, and final input name, but you would not know what is between brick and that input)
  • Same case, when you have block inside brick, and you have input filed on object with same name; pimcore made wrong assumption and take data from object instead from objectbrick

… like block

When we have block inside brick, we do not have enough information inside nested input field or option provider;
@github-actions
Copy link

Review Checklist

  • Target branch (11.0 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

…not attached to object

In case when block is attached to brick, and there is field with same name on object we got exception or wrong data
@gjorgic gjorgic force-pushed the fix/improve-dataobject-block-behaviour branch from 03e5fd0 to 4dc66d2 Compare September 20, 2023 10:47
@gjorgic gjorgic changed the base branch from 10.6 to 11.0 September 20, 2023 10:48
@gjorgic
Copy link
Contributor Author

gjorgic commented Sep 20, 2023

I'm not sure do you treat this as bugfix or improvement; let me know if i need change target branch

@sonarcloud
Copy link

sonarcloud bot commented Sep 20, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@dvesh3 dvesh3 added the Bug label Oct 10, 2023
@robertSt7 robertSt7 changed the base branch from 11.0 to 11.1 October 24, 2023 06:50
@@ -1513,6 +1513,12 @@ public static function enrichLayoutDefinition(&$layout, $object = null, $context
if (method_exists($layout, 'getChildren')) {
$children = $layout->getChildren();
if (is_array($children)) {
// Send information when we have block or similar element
if ($layout instanceof \Pimcore\Model\DataObject\ClassDefinition\Data && empty($context['subContainerType'])) {
Copy link
Contributor

Choose a reason for hiding this comment

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

How about moving the assignment outside getChildren scope and add condition for Block specific type next to localize fields and classificationstore? as it will eventually call enrichLayoutDefinition on children as well.

    $context['ownerName'] = 'localizedfields';
} elseif ($layout instanceof DataObject\ClassDefinition\Data\Block && isset($context['containerType'])) {    
    $context['subContainerKey'] = $layout->getName();
    $context['subContainerType'] = $layout->getFieldtype();
}

Copy link
Contributor

Choose a reason for hiding this comment

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

@gjorgic please see

@dvesh3
Copy link
Contributor

dvesh3 commented Nov 7, 2023

I'm not sure do you treat this as bugfix or improvement; let me know if i need change target branch

@gjorgic I would treat this as bug fix since the context info is missing so the current branch is fine 😊

@dvesh3 dvesh3 modified the milestone: 11.1.1 Nov 7, 2023
@dvesh3
Copy link
Contributor

dvesh3 commented Nov 14, 2023

@gjorgic friendly reminder 😺

@dvesh3 dvesh3 added this to the 11.1.2 milestone Nov 16, 2023
@dvesh3 dvesh3 merged commit 98d0441 into pimcore:11.1 Nov 16, 2023
25 checks passed
@dvesh3
Copy link
Contributor

dvesh3 commented Nov 16, 2023

@gjorgic thanks! Please feel free to provide a follow up for the requested change #15927 (comment).

pimcore-deployments pushed a commit that referenced this pull request Nov 23, 2023
* update: add more context information in case we have structured input like block
When we have block inside brick, we do not have enough information inside nested input field or option provider;

* fix: do not try access block data on object if we know that input is not attached to object
In case when block is attached to brick, and there is field with same name on object we got exception or wrong data
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants