orchidsoftware / crud

Simplify the process of building CRUD (Create, Read, Update, Delete) functionality in Laravel using the features of Orchid.
https://orchid.software
MIT License
138 stars 34 forks source link

Laravel resource convention #50

Closed bilogic closed 2 years ago

bilogic commented 3 years ago

As discussed here #47 If accepted, next step can be to move the screenMatch macro into https://github.com/orchidsoftware/platform

tabuna commented 3 years ago

I don't really like the recording and what it leads to. I expected that we would be fully compliant with a Laravel resource. But with what we have, I'm not sure if we really need a new macro with an exact enumeration of methods. What will it give us?

As for the code. Then I think we can make a much nicer look:

Route::screen('/match', Example::class)
    ->methods('GET');

Route::screen('/match', Example::class)
    ->methods(['GET', 'POST']);

To do this, you need to create a class:

use Illuminate\Support\Arr;
use Illuminate\Support\HigherOrderTapProxy;

class ScreenRoute extends HigherOrderTapProxy
{
    /**
     * @param string|array $methods
     */
    public function methods($methods): self
    {
        $this->target->methods = Arr::wrap($methods);

        return $this;
    }
}

And in the platform macro, return it:

// ...

$route = $this->any($url.'/{method?}', [$screen, 'handle']);

$methods = $screen::getAvailableMethods();

if (! empty($methods)) {
    $route->where('method', implode('|', $methods));
}

return new ScreenRoute($route);

This will make it less painful to add things to the screen definition.

But I repeat that I completely do not understand what advantages we can get. I expected in the issue that we change some of the names from: /crud/view/{resource?}/{Id} to /crud/{resource?}/{Id}.

Maybe I don't understand you completely. Open to discussion

bilogic commented 3 years ago

@tabuna I gave a reply here https://github.com/orchidsoftware/crud/issues/47#issuecomment-874756351