yajra / laravel-datatables-editor

Laravel DataTables Editor Integration.
https://yajrabox.com/docs/laravel-datatables/editor-installation
MIT License
115 stars 14 forks source link

Massive Improvement Options #79

Closed OzanKurt closed 5 months ago

OzanKurt commented 1 year ago

Hello there,

I've been recently learning how to use the datatable editor. From what I've seen datatables editor doesn't have any autorization checks.

Wouldn't it be nice to implement it inside the editor actions so that it checks a certain Policy automatically in case it was not configured.

I have seen a great API example by a package providing generalized API endpoints.

https://github.com/tailflow/laravel-orion/blob/main/src/Concerns/HandlesStandardOperations.php

We only care about the create, update and delete portions of this.


    /**
     * Creates new resource in a transaction-safe way.
     *
     * @param Request $request
     * @return Resource
     * @throws Exception
     */
    public function store(Request $request)
    {
        try {
            $this->startTransaction();
            $result = $this->storeWithTransaction($request);
            $this->commitTransaction();
            return $result;
        } catch (Exception $exception) {
            $this->rollbackTransactionAndRaise($exception);
        }
    }

    /**
     * Creates new resource.
     *
     * @param Request $request
     * @return Resource
     * @throws AuthorizationException
     * @throws BindingResolutionException
     */
    protected function storeWithTransaction(Request $request)
    {
        $resourceModelClass = $this->resolveResourceModelClass();

        $this->authorize($this->resolveAbility('create'), $resourceModelClass);

        /**
         * @var Model $entity
         */
        $entity = new $resourceModelClass;

        $beforeHookResult = $this->beforeStore($request, $entity);
        if ($this->hookResponds($beforeHookResult)) {
            return $beforeHookResult;
        }

        $beforeSaveHookResult = $this->beforeSave($request, $entity);
        if ($this->hookResponds($beforeSaveHookResult)) {
            return $beforeSaveHookResult;
        }

        $requestedRelations = $this->relationsResolver->requestedRelations($request);

        $this->performStore(
            $request,
            $entity,
            $this->retrieve($request)
        );

        $beforeStoreFreshResult = $this->beforeStoreFresh($request, $entity);
        if ($this->hookResponds($beforeStoreFreshResult)) {
            return $beforeStoreFreshResult;
        }

        $query = $this->buildStoreFetchQuery($request, $requestedRelations);

        $entity = $this->runStoreFetchQuery($request, $query, $entity->{$this->keyName()});
        $entity->wasRecentlyCreated = true;

        $afterSaveHookResult = $this->afterSave($request, $entity);
        if ($this->hookResponds($afterSaveHookResult)) {
            return $afterSaveHookResult;
        }

        $afterHookResult = $this->afterStore($request, $entity);
        if ($this->hookResponds($afterHookResult)) {
            return $afterHookResult;
        }

        $entity = $this->relationsResolver->guardRelations($entity, $requestedRelations);

        return $this->entityResponse($entity);
    }

A similar adaptation of DataTablesEditor would be something like this:

I know this is a huge addition but it will have its use cases.

Lets go over the additions 1 by 1:

  1. $this->authorize('create', $this->resolvePolicy($instance)); Allows user to properly use Laravels authorization functionalities for the editor such as create a Post
  2. $data = $this->prepareForValidation($data); Allows the developer to manipulate each "data" from the request before they are sent to validation.
  3. $errorsToAppend = $this->validateRequest($data); handles the validation related stuff and returns an array of errors. The processing of the "data" in that foreach loop will halt and the next data will be processed.
  4. Added new hooks to allow developer to actually "respond" to the editor request. beforeStore, beforeSave, afterSave, afterStore This will allow developers to return custom response information and maybe display an alert message unrelated to the editor defaults.
    public function create(Request $request)
    {
        $model = $this->resolveModel();
        $connection = $model->getConnection();
        $affected = [];
        $errors = [];

        $connection->beginTransaction();
        foreach ($request->get('data') as $data) {
            $this->currentData = $data;

            $instance = $model->newInstance();

            if ($this->authorizationEnabled) {
                $this->authorize('create', $this->resolvePolicy($instance));
            }

            $data = $this->prepareForValidation($data);

            $errorsToAppend = $this->validateRequest($data);

            if (! empty($errorsToAppend)) {
                $errors = array_merge($errors, $errorsToAppend);
                continue;
            }

            $beforeHookResult = $this->beforeStore($instance, $data);
            if ($this->hookResponds($beforeHookResult)) {
                return $beforeHookResult;
            }

            $beforeSaveHookResult = $this->beforeSave($instance, $data);
            if ($this->hookResponds($beforeSaveHookResult)) {
                return $beforeSaveHookResult;
            }

            $instance = $this->performStore();

            $afterSaveHookResult = $this->afterSave($instance, $data);
            if ($this->hookResponds($afterSaveHookResult)) {
                return $afterSaveHookResult;
            }

            $afterHookResult = $this->afterStore($instance, $data);
            if ($this->hookResponds($afterHookResult)) {
                return $afterHookResult;
            }

            $instance->setAttribute('DT_RowId', $instance->getKey());
            $affected[] = $instance;
        }

        if (! $errors) {
            $connection->commit();
        } else {
            $connection->rollBack();
        }

        return $this->toJson($affected, $errors);
    }

    protected bool $authorizationEnabled = true;

    protected ?string $policyClass;

    use Illuminate\Contracts\Auth\Access\Gate;

    public function resolvePolicy(Model $model)
    {
        if (! is_null($policyClass)) {
            return $policyClass;
        }

        return app(Gate::class)->getPolicyFor($model);
    }

    public function performStore($instance, $data)
    {
        if (method_exists($this, 'creating')) {
            $data = $this->creating($instance, $data);
        }

        if (method_exists($this, 'saving')) {
            $data = $this->saving($instance, $data);
        }

        $instance->fill($data)->save();

        if (method_exists($this, 'created')) {
            $instance = $this->created($instance, $data);
        }

        if (method_exists($this, 'saved')) {
            $instance = $this->saved($instance, $data);
        }
    }

    public function validateRequest(array $data): array
    {
        $instance = $this->model->newInstance();
        $validator = $this->getValidationFactory()
            ->make($data, $this->createRules(), $this->messages() + $this->createMessages(), $this->attributes());

        if ($validator->passes()) {
            return [];
        }

        $errors = [];

        foreach ($this->formatErrors($validator) as $error) {
            $errors[] = $error;
        }

        return $errors;
    }

    public function prepareForValidation(array $data): array
    {
        return $data;
    }

Here is an example for the hookResponds implementation.

trait InteractsWithHooks
{
    /**
     * Determine whether hook returns a response or not.
     *
     * @param mixed $hookResult
     * @return bool
     */
    protected function hookResponds($hookResult): bool
    {
        return $hookResult instanceof Response;
    }
}

Let me know what you think about this. Since we did not remove any actual code from the editor it should NOT be a breaking change.

I've installed the editor standalone and it choose to install Laravel components of version 6.*. Which caused A LOT of depreciation errors, maybe its time to add a minimum version requirement for the composer packages. I've also tried running the tests via PHPUnit after upgrading the Laravel components and orchestra/testbench, it didn't go that well... It cannot locate the classes.

Here is a screenshot of it: image

yajra commented 1 year ago

I agree with the missing authorization and have been wondering how to do it with minimal or no breaking change for quite some time.

Atm, this is how I handle authorization:

On Editor code:

Button::makeIfCan('manage-users', 'create')->editor('editor'),

On routes:

Route::post('users', [UsersController::class, 'store'])->middleware('can:manage-users');

Please do not hesitate to submit a proposed implementation.

OzanKurt commented 1 year ago

Is it possible for you to check the versioning of the package?

Since I am not familiar with tests I can't make the packages tests run.

I've installed the editor standalone and it choose to install Laravel components of version 6.*. Which caused A LOT of depreciation errors, maybe its time to add a minimum version requirement for the composer packages. I've also tried running the tests via PHPUnit after upgrading the Laravel components and orchestra/testbench, it didn't go that well... It cannot locate the classes.

yajra commented 1 year ago

I was not able to update the editor package version and it stays on 1.x series. Also planning to bump this to 10.x to match the framework version but don't have time to do it yet.

github-actions[bot] commented 6 months ago

This issue is stale because it has been open for 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

OzanKurt commented 5 months ago

We might focus on this in the future.