Add the ability to auto-determine the presenter #26
Conversation
|
Hi, @drjermy Sorry for the late response/review. I was quite busy. This is an interesting PR to merge, could you please elaborate more about the idea with examples? Thanks! |
|
Currently when using Presenters, we setup by:
And then, have to add a $presenters array that defines the Presenters. In most cases, we define a single default presenter. So, rather than doing this, why not automatically look for a Presenter in a standard location (Where php artisan make:presenter puts a presenter) with the name "ModelPresenter" where Model is the model name. If $presenters is set, then it overwrites the defaults. So, if I had a model called User and implemented CanPresent and added UsesPresenters, then the default presenter would autoload UserPresenter.php which I'd created using php artisan make:presenter |
|
@ousid any update? |
Hey, I'm seeing failed tests, can you please take a look and see what's wrong exactly? Thanks! |
src/Concerns/UsesPresenters.php
Outdated
| { | ||
| $modelNameModifier = $type === 'default' ? '' : $type; | ||
|
|
||
| return str(get_class()) |
There was a problem hiding this comment.
It seems the tests are failing, regards, to the str method. Consider using the Illuminate\Support\Str facade, maybe?
| if (array_key_exists($type, $this->presenters)) { | ||
| return new $this->presenters[$type]($this); | ||
| $presenter = $this->getDefaultPresenterName($type); | ||
|
|
There was a problem hiding this comment.
I see a Code smell in here, consider using aggressive condition statement, to clean the code, and understandable.
| if ( | |
| is_array($this->presenters) && | |
| array_key_exists($type, $this->presenters ?? []) | |
| ) { | |
| $presenter =$this->presetners[$type]; | |
| } | |
| if (is_string($this->presenter ?? null)) { | |
| $presenter = $this->presenter; | |
| } | |
| if(class_exists($presenter)) { | |
| return new $presenter($this); | |
| } | |
| throw new PresenterException(); |
tests/Presenters/ItemPresenter.php
Outdated
| { | ||
| public function slug() | ||
| { | ||
| return str($this->model->title)->slug()->toString(); |
| { | ||
| public function caps() | ||
| { | ||
| return str($this->model->title)->upper()->toString(); |
There was a problem hiding this comment.
Same here, use the Str facade, or invoke the str function on the namespaces.
| @@ -111,7 +111,7 @@ If you want to change the directory, you have two options. | |||
| First options is to set the full namespace while you're creating the presenter class | |||
|
|
|||
| ```bash | |||
| php artisna presneter:make App\Models\Presenter\UserPresenter | |||
| php artisan presneter:make App\Models\Presenter\UserPresenter | |||
There was a problem hiding this comment.
Maybe you want to create an alias for the command? Something like php artisan make:presenter, without removing the current?
There was a problem hiding this comment.
This was just a spelling error change.
|
Tests still failing, could you please run them locally, and once all passes, tag me for a review. Thanks! |
|
Closing this PR, because of the failing tests. |
I wonder whether you'd be interested in considering this pull-request.
The idea here is to automatically use a presenter based on naming convention. So, in the User Model, the UserPresenter is loaded without needing to specify it in the $presenters array.
It also guesses the name for a named presenter, e.g. $users->present('certificate')->title would load the UserCertificatePresenter.