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

Allow Runtime Table Aliassing #2737

Open
wants to merge 9 commits into
base: 3.x
Choose a base branch
from

Conversation

tylernathanreed
Copy link

@tylernathanreed tylernathanreed commented Mar 11, 2025

Description

One of my packages, reedware/laravel-relation-joins received an issue report (#34) indicating that it was partially incompatible with your project.

Upon further investigation, I saw that the root cause was the getTable method on models being overridden in such a way that setTable is no longer respected.

My package allows you to join on relations by name, and sometimes those joins need to be aliased, like so:

Video::query()->joinRelation('relatableItems as relatable')

When an alias is used in my package, the underlying implementation is to swap out the table name of the model with the aliased counterpart so that any qualified constraints are bound against the aliased table, rather than the original one. This relies on the ability to set a model's table during runtime that doesn't reflect the actual model's table, but rather an alias in a query that's actively being constructed.

However, because your implementation of getTable does not support setTable, the approach in my package falls apart, as it needs to be able to rely on setTable. Thankfully, the fix is quite simple. Functions like this:

public function getTable()
{
    return config('twill.related_table', 'twill_related');
}

Just need to become this:

public function getTable()
{
    return $this->table ?? config('twill.related_table', 'twill_related');
}

This PR updates all overrides of getTable to follow suit, thus resolving the partial incompatibility between our two projects.

@CLAassistant
Copy link

CLAassistant commented Mar 11, 2025

CLA assistant check
All committers have signed the CLA.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants