Clearer exception

[bilogic]

Throws a clearer exception message and guide the user towards adding the correct traits

[bilogic]

Only my last commit is for this PR, not sure why the other 2 got mixed in too

[tabuna]

Hi, if we start checking traits, we need to check more, for example, AsSource andAttachable and others. But it can silently break users' workstations if they haven't used some of the traits and their capabilities.

Therefore, I'm not sure that we should make half-hearted decisions, and full ones can violate. I propose to return to this when we release a new major release.

For the old commits to be missing, you need to rebase

[bilogic]

Hi, I did consider AsSource and Attachable, but the CRUD does not always call them. However, ResourceRequest.php always calls ->filter().

Anyway, my thinking was, the user will most likely miss all 3 together, so this 1 exception will (hopefully) quickly point to the root cause 1 time, if there are other exceptions for AsSource or Attachable, there is a higher chance they may look at the model again.

I found it quite hard to connect the current exception filter() does not exist... to the model.

Perhaps we can change the message to be

Model must use Filterable. Consider adding AsSource and Attachable as well.

Should I rebase? Or are you able to remove it?

Thank you.

[bilogic]

@tabuna

I just ran into this issue again and had to check back on this issue to identify the cause. Haha... I really feel we should make things easier to self-discover and not have to rely on memory and docs all the time.

As mentioned ResourceRequest.php always calls filter() https://github.com/orchidsoftware/crud/blob/e308bdb4a84affac08a9fd4286a1065d7b71ad8c/src/ResourceRequest.php#L76 That's the reason why AsSource and Attachable are not as important. Thank you.

[tabuna]

Let's check the value in the constructor. Also, make a rebase so that the changes affect only this functionality, and I will accept it.

[bilogic]

Ok, let me get it. Just to clarify, what did you mean by check in the constructor? My changes does not affect the constructor at the moment. Please let me know if I should do the checking in a different way. Thanks.

[tabuna]

I meant something like this:

    /**
     * Resource constructor.
     */
    public function __construct()
    {
        $needTraits = collect(trait_uses_recursive(static::$model))->has([
            Filter::class,
        ]);

        abort_unless($needTraits, 500, 'The model must have the required traits.');
    }

[bilogic]

@tabuna i don't we can do it in the constructor as $this->model() is not ready and static::$model does not exist. Ok if I stick to check inside getModelPaginationList()?

The exception I get when calling $this->model() in constructor

Argument 1 passed to Orchid\Crud\Arbitrator::findOrFail() must be of the type string, null given, called in /bilogic/crud/src/ResourceRequest.php on line 142