Abstract `owner` condition for use alongside NestedElementManager #15882

ryansupercool · 2024-10-11T17:42:58Z

ryansupercool
Oct 11, 2024

The owner condition used in beforePrepare is a bit complicated and is duplicated (with minor variants to the names of the tables) in various element queries in the Craft org.

We're looking at implementing NestedElementManager and the associated owner method in our own Elements and was wondering if this part of the query could be abstracted on Craft's end to save us (and others) having to implement it (copy and paste) into our own code?

cms/src/elements/db/EntryQuery.php

Lines 1188 to 1232 in 68557cb

    
           if (!empty($this->fieldId) || !empty($this->ownerId) || !empty($this->primaryOwnerId)) { 
        
               // Join in the elements_owners table 
        
               $ownersCondition = [ 
        
                   'and', 
        
                   '[[elements_owners.elementId]] = [[elements.id]]', 
        
                   $this->ownerId ? ['elements_owners.ownerId' => $this->ownerId] : '[[elements_owners.ownerId]] = [[entries.primaryOwnerId]]', 
        
               ]; 
        
               $this->query 
        
                   ->addSelect([ 
        
                       'elements_owners.ownerId', 
        
                       'elements_owners.sortOrder', 
        
                   ]) 
        
                   ->innerJoin(['elements_owners' => Table::ELEMENTS_OWNERS], $ownersCondition); 
        
               $this->subQuery->innerJoin(['elements_owners' => Table::ELEMENTS_OWNERS], $ownersCondition); 
        
               if ($this->fieldId) { 
        
                   $this->subQuery->andWhere(['entries.fieldId' => $this->fieldId]); 
        
               } 
        
               if ($this->primaryOwnerId) { 
        
                   $this->subQuery->andWhere(['entries.primaryOwnerId' => $this->primaryOwnerId]); 
        
               } 
        
               // Ignore revision/draft blocks by default 
        
               $allowOwnerDrafts = $this->allowOwnerDrafts ?? ($this->id || $this->primaryOwnerId || $this->ownerId); 
        
               $allowOwnerRevisions = $this->allowOwnerRevisions ?? ($this->id || $this->primaryOwnerId || $this->ownerId); 
        
               if (!$allowOwnerDrafts || !$allowOwnerRevisions) { 
        
                   $this->subQuery->innerJoin( 
        
                       ['owners' => Table::ELEMENTS], 
        
                       $this->ownerId ? '[[owners.id]] = [[elements_owners.ownerId]]' : '[[owners.id]] = [[entries.primaryOwnerId]]' 
        
                   ); 
        
                   if (!$allowOwnerDrafts) { 
        
                       $this->subQuery->andWhere(['owners.draftId' => null]); 
        
                   } 
        
                   if (!$allowOwnerRevisions) { 
        
                       $this->subQuery->andWhere(['owners.revisionId' => null]); 
        
                   } 
        
               } 
        
               $this->defaultOrderBy = ['elements_owners.sortOrder' => SORT_ASC]; 
        
           } else {

Answered by brandonkelly

Oct 15, 2024

Thanks for the suggestions! Just added NestedElementQueryTrait and NestedElementTrait::saveOwnership() for Craft 5.5 (#15894).

View full answer

ryansupercool · 2024-10-11T21:34:02Z

ryansupercool
Oct 11, 2024
Author

Along the same line could NestedElementManager or a helper be provided to help setting ownership in an elements afterSave method? It feels a little unsafe for a plugin to be writing directly to the element owners table. Could this be handle via a service method?

cms/src/elements/Address.php

Lines 672 to 724 in 68557cb

    
           $ownerId = $this->getOwnerId(); 
        
           if (isset($this->fieldId) && $ownerId && $this->saveOwnership) { 
        
               if (!isset($this->sortOrder) && (!$isNew || $this->duplicateOf)) { 
        
                   // figure out if we should proceed this way 
        
                   // if we're dealing with an element that's being duplicated, and it has a draftId 
        
                   // it means we're creating a draft of something 
        
                   // if we're duplicating element via duplicate action - draftId would be empty 
        
                   $elementId = null; 
        
                   if ($this->duplicateOf) { 
        
                       if ($this->draftId) { 
        
                           $elementId = $this->duplicateOf->id; 
        
                       } 
        
                   } else { 
        
                       // if we're not duplicating - use element's id 
        
                       $elementId = $this->id; 
        
                   } 
        
                   if ($elementId) { 
        
                       $this->sortOrder = (new Query()) 
        
                           ->select('sortOrder') 
        
                           ->from(Table::ELEMENTS_OWNERS) 
        
                           ->where([ 
        
                               'elementId' => $elementId, 
        
                               'ownerId' => $ownerId, 
        
                           ]) 
        
                           ->scalar() ?: null; 
        
                   } 
        
               } 
        
               if (!isset($this->sortOrder)) { 
        
                   $max = (new Query()) 
        
                       ->from(['eo' => Table::ELEMENTS_OWNERS]) 
        
                       ->innerJoin(['a' => Table::ADDRESSES], '[[a.id]] = [[eo.elementId]]') 
        
                       ->where([ 
        
                           'eo.ownerId' => $ownerId, 
        
                           'a.fieldId' => $this->fieldId, 
        
                       ]) 
        
                       ->max('[[eo.sortOrder]]'); 
        
                   $this->sortOrder = $max ? $max + 1 : 1; 
        
               } 
        
               if ($isNew) { 
        
                   Db::insert(Table::ELEMENTS_OWNERS, [ 
        
                       'elementId' => $this->id, 
        
                       'ownerId' => $ownerId, 
        
                       'sortOrder' => $this->sortOrder, 
        
                   ]); 
        
               } else { 
        
                   Db::update(Table::ELEMENTS_OWNERS, [ 
        
                       'sortOrder' => $this->sortOrder, 
        
                   ], [ 
        
                       'elementId' => $this->id, 
        
                       'ownerId' => $ownerId, 
        
                   ]); 
        
               } 
        
           }

https://github.com/craftcms/commerce/blob/a156731711ab80f9197a14f8832420908b4a9f4f/src/elements/Variant.php#L937-L989

https://github.com/craftcms/stripe/blob/71bcbee30203f432825e24de9f42702f163c4b99/src/elements/Price.php#L469-L519

0 replies

brandonkelly · 2024-10-15T17:42:39Z

brandonkelly
Oct 15, 2024
Maintainer

Thanks for the suggestions! Just added NestedElementQueryTrait and NestedElementTrait::saveOwnership() for Craft 5.5 (#15894).

3 replies

rynpsc Oct 15, 2024

Amazing. Thanks!

Would this also work for element queries that don't have a fieldId such as in the Stripe and Commerce plugins?

https://github.com/craftcms/commerce/blob/5.x/src/elements/db/VariantQuery.php#L416
https://github.com/craftcms/stripe/blob/1.x/src/elements/db/PriceQuery.php#L500

brandonkelly Oct 15, 2024
Maintainer

Right now it’s specifically built for nested elements that are managed via custom fields, at least optionally. (Addresses go both ways.)

ryansupercool Oct 31, 2024
Author

I've just been looking at this again and the elements we are dealing with work very much like Products/Variants in Commerce and Products/Prices in Stripe. Both of those plugins are also managing owership with almost an identical chunk of code to the now abstracted method with exception of the fieldId. Other plugins appear to be duplicating this bits of this code as well such as Verbb Events.

Do you think there's scope for Craft core abstracting the save owership logic for elements that aren't being used in the context of a field?

https://github.com/craftcms/stripe/blob/f7440e5d920f2daabd445ca6dcb9cf3176c14cac/src/elements/Price.php#L470-L520
https://github.com/craftcms/commerce/blob/272475a8b385896e21282e691ec764387b03b191/src/elements/Variant.php#L937-L989

https://github.com/craftcms/stripe/blob/f7440e5d920f2daabd445ca6dcb9cf3176c14cac/src/elements/db/PriceQuery.php#L546-L589

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Abstract `owner` condition for use alongside NestedElementManager #15882

Uh oh!

{{title}}

Uh oh!

Replies: 2 comments 3 replies

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{title}}

Uh oh!

Uh oh!

{{editor}}'s edit

{{editor}}'s edit

Uh oh!

Select a reply

Uh oh!

Abstract owner condition for use alongside NestedElementManager #15882

Uh oh!

ryansupercool Oct 11, 2024

Replies: 2 comments · 3 replies

Uh oh!

ryansupercool Oct 11, 2024 Author

Uh oh!

brandonkelly Oct 15, 2024 Maintainer

Uh oh!

rynpsc Oct 15, 2024

Uh oh!

brandonkelly Oct 15, 2024 Maintainer

Uh oh!

Uh oh!

ryansupercool Oct 31, 2024 Author

Abstract `owner` condition for use alongside NestedElementManager #15882

ryansupercool
Oct 11, 2024

Replies: 2 comments 3 replies

ryansupercool
Oct 11, 2024
Author

brandonkelly
Oct 15, 2024
Maintainer

brandonkelly Oct 15, 2024
Maintainer

ryansupercool Oct 31, 2024
Author