Not a bug "A withAlias must be the last relation requested" cripples the feature

[DragonByte Tech]

Affected version

2.1.0 Beta 2

The fact that you decided to impose this arbitrary limitation instead of creating a new method on \XF\Mvc\Entity\Finder absolutely cripples extendability for Repositories. I consider it a bug because it's a design flaw that has an easy solution.

Consider the following scenario:
Let's say you have a Repository method, findLicensesByUser. It normally includes the old forFullView method, does some other stuff based on input, and returns the finder. Before, I would simply call $finder->forFullView() and let method chaining take it from there. That won't work if I'm using aliases.

Let's now say someone creates an add-on for my product, that adds a new database table and a new entity that relates to the License entity, so they add the relation via a class extension / event listener and they go to extend findLicensesByUser. Uh-oh, they can't add a new ->with() call because aliases must be the last relations requested.

You might think "you should just add this relation to the withAliases method that replaces forFullView instead" and sure, that works, if you need that relation at all times where forFullView was normally used. But what if you don't? Why force developers to include a relation that may be a waste of time and resources?

Sure, I could copy-paste the ->with('full') everywhere directly below the ->fetch() call, but that violates DRY (Don't Repeat Yourself).

The solution should be quite simple:

• Add a new property, protected $withAliases = [];
• Add a new method, public function withAlias($name) which appends to the $this->withAliases property
• Above foreach ($this->joins AS $join) or wherever else in getQuery you might deem relevant, loop through $this->withAliases and add them to joins as they are just now

You don't even have to make any changes to existing XF2 finder alias calls in order to support this feature for developers, so fixing this design flaw should not require re-testing every part of XF2 that currently uses finder aliases.


Fillip

 

[Mike]

I think you may be misinterpreting what this error means or there's a bug where it's triggered. I'd like to see the code that triggered the error. Because of this, I'm not really following issue you're trying to identify.

You can't do something like this, because it doesn't make sense: $finder->with('Relation.alias.SomethingElse') -- the SomethingElse part is meaningless with an alias because there's no specific relation to move to. That's the only situation where the error should happen.

There is no order dependency on when you join aliases. Indeed, both of these examples work without issue:

\XF::finder('XF:User')->with('api')->with('Reject')->fetchOne();
\XF::finder('XF:User')->with(['api', 'Reject'])->fetchOne();

 

[DragonByte Tech]

I think you may be misinterpreting what this error means or there's a bug where it's triggered.

You're right, it does seem like I misinterpreted the code

Sorry about that, feel free to bin this.


Fillip