Fixed missing env options in admin context

[hbugdoll]

What are you changing/introducing

• Fixed the passed "page object" for admin context, so env option 'pageObject' is available
• Added env options 'index', 'itemsCount', 'isFirst', 'isLast', 'isPrevEqual', 'isNextEqual' and 'equalIndex' for admin context in the block iteration in getPlaceholder() analog to renderPlaceholderRecursive()
• Multiple return type for NavItemPage::getBlockItem()

What is the reason for changing/introducing

• See issue Some block env options are missing in admin context #401

QA

Q	A
Is bugfix?	yes
New feature?	no
Breaks BC?	no
Tests pass?	yes
Fixed issues	Please let the PR open!

[hbugdoll]

I can't unterstand the rector error 😳
(all local changes are committed)
Edit: Ah, now I see the problem... git-auto-commit-action hung on the uncommitted rector changes.

[nadar]

We should preload this relations, otherwise this will strongly increase db requests and memory usage.

[hbugdoll]

We should preload this relations, otherwise this will strongly increase db requests and memory usage.

To avoid repeating getNavItem() (with same returned nav item) for every block of a page item?

[hbugdoll]

The NavItem of a NavItemType doesn't change on object lifetime. Is this assumption correct?
So only one database access should be sufficient (73c5072).
Edit: I've reverted that commit.

[nadar]

Thanks for all the work, sorry for late and bad reply. There is no need for private variable in a yii2 relation

if you define $this->hasOne() relation in a method:

public function getNavItem()
    {
        return $this->hasOne(NavItem::class, ['nav_item_type_id' => 'id'])->where(['nav_item_type' => static::getNummericType()]);
    }

then you have to use $object->navItem, this will establish the relation and its already stored, so a second call to $object->navItem will not result in a second query.

what i meant is that this above relation should be preloaded using ->with(['navItem]) at the point where all blocks are iterated, maybe its already the case.

[nadar]

this won't pass the relation, this will pass an active query instance, not an active record. So if you like to pass the nav item object, then you have to use navItemPage->navItem. There is no unit tests for this, then this would fail here :-)

[hbugdoll]

I'm confused because of two reasons.
The guide https://luya.io/guide/cms/blocks.html#env-context-information says

pageObject: Returns the luya\cms\models\NavItem object where you can run luya\cms\models\NavItem->getNav() to retrieve the luya\cms\models\Nav object.

And for the frontend context getNavItem() is already used in NavItemPage::renderPlaceholderRecursive():

luya-module-cms/src/models/NavItemPage.php

Line 244 in a05aac2

$blockObject = Block::createObject($placeholder['class'], $placeholder['block_id'], $placeholder['id'], 'frontend', $this->getNavItem());

The pageObject should anyway be the same in frontend and admin context, so we should use $navItemPage->navItem resp. $this->navItem.

[nadar]

in general, i think we need a unit tests for this huge changeset.

[hbugdoll]

in general, i think we need a unit tests for this huge changeset.

Of course!

then you have to use $object->navItem, this will establish the relation and its already stored, so a second call to $object->navItem will not result in a second query.

what i meant is that this above relation should be preloaded using ->with(['navItem]) at the point where all blocks are iterated, maybe its already the case.

I should have read https://www.yiiframework.com/wiki/834/relational-query-lazy-loading-and-eager-loading-in-yii-2-0#eager-loading before.

But at the point in question (NavItemPage::getPlaceholder()) the query is on NavItemPageBlockItem which doesn't have getNavItem(), so ->with(['navItem']) fails.

Would it be sufficient to store the ActiceRecord in a variable before the block iteration?

luya-module-cms/src/models/NavItemPage.php

Lines 436 to 437 in a05aac2

	foreach ($nav_item_page_block_item_data as $blockItem) {
	$item = self::getBlockItem($blockItem, $navItemPage);

$nav_item_page_block_item_data = NavItemPageBlockItem::find()...

$navItem = $navItemPage->navItem;

foreach ($nav_item_page_block_item_data as $blockItem) {
    $item = self::getBlockItem($blockItem, $navItemPage, $navItem);
    ...

[nadar]

i have quickly created a PR with just the required parts for your code, thanks!

#403

can you check it quickly maybe?

[hbugdoll]

i have quickly created a PR with just the required parts for your code, thanks!

#403

Thank you.

can you check it quickly maybe?

See #403 (review).

[nadar]

lets continue with my PR and we take care of each problem by seperate pr. agree? 👍

[hbugdoll]

lets continue with my PR and we take care of each problem by seperate pr. agree? 👍

Agreed!

[nadar]

Thank again. The PR with the env options have been merged into the master. Problem with page object still exists.