Laravel-Backpack / revise-operation

An admin interface for venturecraft/revisionable - audit log for your Eloquent entries.
Other
42 stars 10 forks source link

[Feature Request] Allow certain users to Restore revision depending on permission or something else #9

Closed jrbecart closed 4 years ago

jrbecart commented 4 years ago

Feature Request

What's the feature you think this package should have?

Possibility to remove the /restore route or add another operation name to manage permission differently from the list route. Be allowed to use permission.

protected function setupReviseRoutes($segment, $routeName, $controller)
    {
        Route::get($segment.'/{id}/revise', [
            'as' => $routeName.'.listRevisions',
            'uses' => $controller.'@listRevisions',
            'operation' => 'reviseList', // new operation name...
        ]);

        Route::post($segment.'/{id}/revise/{revisionId}/restore', [
            'as' => $routeName.'.restoreRevision',
            'uses' => $controller.'@restoreRevision',
            'operation' => 'reviseRestore', // new operation name
        ]);
    }

Have you already implemented a prototype solution, for your own project?

No

Do you see this as a core feature or an add-on?

Yes

welcome[bot] commented 4 years ago

Hello there! Thanks for opening your first issue on this repo!

Just a heads-up: Here at Backpack we use Github Issues only for tracking bugs. Talk about new features is also acceptable. This helps a lot in keeping our focus on improving Backpack. If you issue is not a bug/feature, please help us out by closing the issue yourself and posting in the appropriate medium (see below). If you're not sure where it fits, it's ok, a community member will probably reply to help you with that.

Backpack communication channels:

Please keep in mind Backpack offers no official / paid support. Whatever help you receive here, on Gitter, Slack or Stackoverflow is thanks to our awesome awesome community members, who give up some of their time to help their peers. If you want to join our community, just start pitching in. We take pride in being a welcoming bunch.

Thank you!

-- Justin Case The Backpack Robot

tabacitu commented 4 years ago

@jrbecart at first I thought this would be a good idea. And in theory it shouldn't be difficult at all to achieve:

But then I though - you should be able to do the exact same inside a setupReviseOperation() method. Backpack will automatically call it if it's there. And now that I think of it, it should be pretty easy and customizable over there to check the combination of user/route/permission and abort if something's not ok. MUCH easier than bloating the package with this additional logic. So something like this should do it:

public function setupReviseOperation() {
    // if route operation is `revise` AND
    // the called route name ends with `restoreRevision` AND
    // the user doesn't have the permission to do this
    // TODO: abort
}

Let me know what you think. Cheers!

jrbecart commented 4 years ago

@tabacitu Thanks! I was thinking at the same solution and I will probably doing it that way. It's just that I have many controllers... :(

My other solution was to do it in custom.php but I was unable to use the wildcard in the route so instead of overriding all routes I will do it in all my controllers.

// I attempted to replace myroute with wildcard such as {any}, {any?}, {any:.*} 
// to override all routes at once but with no luck
Route::post('myroute/{id}/revise/{revisionId}/restore', function () {
     abort(404);
});

Anyway you are right that's probably the easiest way and this will avoid to bloat the package.

tabacitu commented 4 years ago

👍 if you need it across many CrudControllers, you can:

Personally I'd put that inside app/Http/Controllers/Admin/Operations but to each his own. Hope it helps!