Skip to content

Commit

Permalink
Swap to the better Laravel allRelatedIds parent logic
Browse files Browse the repository at this point in the history
This is needed since the previous query used with multisite would return duplicate keys, eg: [1,2,2,2,2,2] since it joined in shared relations (scopes and other constraints were not applied to this query). This would cause integrity violations in the database when saving the duplicates. We looked at adding the following additional constraint:

```php
protected function applyMultisiteConstraint()
{
    $model = $this->related;
    if (
        $model &&
        $model->isClassInstanceOf(\October\Contracts\Database\MultisiteInterface::class) &&
        $model->isMultisiteEnabled() &&
        !Site::hasGlobalContext()
    ) {
        $this->where($model->getQualifiedSiteIdColumn(), Site::getSiteIdFromContext());
    }
}
```

However, this is getting too complicated now. The Laravel query just hits the join table and we researched why deferred binding was needed in the first place...

- 58f1b1c: Added deferred binding to simple value lookup since it was needed for validation

- 1828fbb: Removed the use of simple value in validation for nested attribute validation

- b0329f2: Added deferred binding to the validation lookup itself

Yet the unused logic still remains. Internally, we can use the much simpler logic found in Laravel instead. Externally, the equivalent functionality is found with a `withDeferred()->pluck('id')->all()` call.
  • Loading branch information
daftspunk committed Mar 5, 2024
1 parent 845e5d1 commit 67bea34
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 16 deletions.
17 changes: 1 addition & 16 deletions src/Database/Relations/BelongsToMany.php
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,6 @@ public function getSimpleValue()
{
$value = [];
$relationName = $this->relationName;
$sessionKey = $this->parent->sessionKey;

if ($this->parent->relationLoaded($relationName)) {
$value = $this->parent->getRelation($relationName)
Expand All @@ -392,26 +391,12 @@ public function getSimpleValue()
;
}
else {
$value = $this->allRelatedIds($sessionKey)->all();
$value = $this->allRelatedIds()->all();
}

return $value;
}

/**
* allRelatedIds for the related models, with deferred binding support
* @param string $sessionKey
* @return \October\Rain\Support\Collection
*/
public function allRelatedIds($sessionKey = null)
{
$fullKey = $this->getQualifiedRelatedKeyName();

$query = $sessionKey ? $this->withDeferred($sessionKey) : $this->query;

return $query->getQuery()->select($fullKey)->pluck($this->getRelatedKeyName());
}

/**
* @deprecated use getQualifiedForeignPivotKeyName
*/
Expand Down
1 change: 1 addition & 0 deletions src/Support/Facades/Site.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* @method static array listSiteIdsInLocale($siteId)
* @method static iterable listSites()
* @method static int|null getSiteIdFromContext()
* @method static string|null getSiteCodeFromContext()
* @method static mixed getSiteFromContext()
* @method static bool hasGlobalContext()
* @method static void withGlobalContext(callable $callback)
Expand Down

0 comments on commit 67bea34

Please sign in to comment.