Laravel-Backpack / community-forum

A workspace to discuss improvement and feature ideas, before they're actually implemented.
28 stars 0 forks source link

[Proposal] Prefix for route names #2

Open tabacitu opened 5 years ago

tabacitu commented 5 years ago

In 4.0 the "crud" prefix was removed from route names. But maybe that wasn't such a great idea. It helped differentiate between front-end routes and back-end routes (admin panel).

Maybe in 4.2 we add back the prefix to all CRUD route names?

Patabugen commented 5 years ago

I would opt for having it back, I think it's good to have that kind of namespacing from packages - in case I've got another controller for the same kind of model to handle non-crud stuff (I do!).

At the same time, if Backpack is most people's main package in an application then it makes sense for the route names not to be "second class routes" hiding behind a prefix.

tabacitu commented 5 years ago

I agree with both of your statements :-))

@Patabugen note that instead of changing your routes everywhere, you can add the "crud" prefix back to your custom CRUDs by adding this to your routes/backpack/custom.php:

Route::group([
    'prefix'     => config('backpack.base.route_prefix', 'admin'),
    'middleware' => ['web', config('backpack.base.middleware_key', 'admin')],
    'namespace'  => 'App\Http\Controllers\Admin',
+    'name' => 'crud.',
], function () { // custom admin routes
    // CRUD resources and other admin routes
    Route::crud('monster', 'MonsterCrudController');
    Route::crud('icon', 'IconCrudController');
    Route::crud('product', 'ProductCrudController');
}); // this should be the absolute last line of this file
tabacitu commented 5 years ago

Maybe this is a good opportunity to not only prefix the name of the CRUDs, but the name of anything that's inside the admin panel. Like dashboard, custom screens, auth, etc.

A great prefix for everything would be config('backpack.base.route_prefix', 'admin'). Which, for most people, is probably the default "admin". This would make ALL admin routes be behind an "admin" prefix. Nice and clean.

However. Because this "admin" prefix is configurable, if you ever decide to change the "admin" prefix to something else, you also have to change all route calls... Which kind of defeats the purpose of named routes... That's why they exist, so that you don't have to change them.

So maybe a better route name prefix would be backpack.?

PROs:

CONs:

All in all, I think the backpack prefix is a good solution. Thoughts?

Patabugen commented 5 years ago

haha it's ok, I only had one for this project!

I'd agree that a prefix is good, and that "backpack" is more specific than "crud" (someone... someone out there is running two crud-providing packages, and it's probably for a very good reason).

So yes - I'd agree with 'backpack' - but perhaps just by adding the 'route.prefix' example you shared above to the default configuration (i.e. don't set it in the Route::crud() method)

pxpm commented 5 years ago

Hey @tabacitu .

My main concern you already pointed (named routes are for easy identification and access).

That said i think backpack is a proper namespace to all backpack/admin routes. Or, dunno if it is possible, use the prefix in config, add an helper that mimics route() but automatically add that prefix to route name ? like: admin_route('products.index') would translate to 'admin.products.index'. (if admin is the prefix).

Best, Pedro

tabacitu commented 4 years ago

@pxpm hmm that's true. We already have backpack_url(). We could also add a backpack_route() helper. Then again, you'd still be typing "backpack" to all the admin routes, just that now you're using the helper instead of the static string 😄 So it's backpack_route('product.index') instead of route('backpack.product.index'). But it would have the benefit of making what you see inside php artisan route:list configurable. I think it's a great idea @pxpm - thanks!

@OwenMelbz what do you think about this? I know you had strong opinions about not having "Backpack" anywhere inside controller names, routes, etc, right?

Cheers!

tm-blg commented 3 years ago

terrible support, so sad

pxpm commented 3 years ago

Hello @tm-blg . Sorry to hear about your bad experience.

Is there something we can help with ? If it's regarding this issue with the route names, it's in the roadmap for backpack 4.2.

Best, Pedro

tm-blg commented 3 years ago

https://backpackforlaravel.com/docs/4.1/crud-fields#relationship

looks, docs haven't information about complete solution. How newbie can find out? I read more disscussions abou inline-create and too cant find out.

My problem: i dont sure where i'm need to do field with inline-create. i have errors with route after add inline-create, where i'm need to do route and how it influence on view from button 'add'.

my code:

PurchaseController extends CrudController use ListOperation, ShowOperation, CreateOperation, UpdateOperation, DeleteOperation, FetchOperation, InlineCreateOperation;

public function fetchItems() { return $this->fetch(\App\Item::class); }

public function setup()
{
            [
                'label' => 'purchase items', 'name' => 'items', 'type' => 'relationship',
                'entity' => 'items', 'attribute' => 'name',
                'ajax' => 'true',
                'wrapper' => ['class' => 'form-group col-md-12'],
            ]

}


Purchase extends Model

public function items()
{
    return $this->belongsToMany(
        Item::class,
        'purchase_has_item');
}

public function fetchItems()
{
    return $this->fetch(\App\Item::class);
}

custom.php

Route::crud('purchases', 'PurchaseController');
Route::crud('items', 'ItemController');

i spent 2 days, but cant find out. Please help me, rewrite my code, show me how to do.

pxpm commented 3 years ago

Hello @tm-blg I am sorry for the troubles you were in.

The docs for inline create are here: https://backpackforlaravel.com/docs/4.1/crud-operation-inline-create

You need to add the InlineCreateOperation inside your ItemCrudController.

In you field you need to setup inline_create => true, if your ItemCrudController url is /items you are good to go, if not please add in your field:

'inline_create' => [
    'entity' => 'your-items-crud-controller-segment-url' //probably is: item
]

By example:

If I have a relation like:

public function products()
    {
        return $this->belongsToMany(\App\Models\Product::class);
    }

I need to add the InlineCreateOperation to the ProductCrudController. There, my url is admin/product (notice that the relation is products) so I need to manually define the entity when initializing the field:

$this->crud->addField([
            'name' => 'products', //the belongstomany relation in the model
            'inline_create' => [
                'entity' => 'product' //the entity in the related controller
            ]
        ]);

I also need in this controller the fetch (that you already have):

public function fetchProducts()
    {
        return $this->fetch(\App\Models\Product::class);
    }

If you have other questions you can always post them in other places like:

https://gitter.im/BackpackForLaravel/Lobby or https://stackoverflow.com/search?q=backpack-for-laravel

There are more devs in those places that use Backpack and help each other. It's not a guaranteed answer, but it increases your chances of finding the solution faster.

Wish you the best, Pedro

tm-blg commented 3 years ago

water water, I did all this, but an error occurs with the route. can you change my code?

tm-blg commented 3 years ago

in docs also write this: 'Step 2. Use the relationship field inside the setupCreateOperation() or setupUpdateOperation() of the main entity (where you'd like to be able to click a button and a modal shows up, in our example ArticleCrudController), and define inline_create on it: // for 1-n relationships (ex: category) - inside ArticleCrudController [ 'type' => "relationship", 'name' => 'category', // the method on your model that defines the relationship 'ajax' => true, 'inline_create' => true, // assumes the URL will be "/admin/category/inline/create" ]

// for n-n relationships (ex: tags) - inside ArticleCrudController [ 'type' => "relationship", 'name' => 'tags', // the method on your model that defines the relationship 'ajax' => true, 'inline_create' => [ 'entity' => 'tag' ] // specify the entity in singular // that way the assumed URL will be "/admin/tag/inline/create" ]

// OPTIONALS - to customize behaviour [ 'type' => "relationship", 'name' => 'tags', // the method on your model that defines the relationship 'ajax' => true, 'inline_create' => [ // specify the entity in singular 'entity' => 'tag', // the entity in singular // OPTIONALS 'force_select' => true, // should the inline-created entry be immediately selected? 'modal_class' => 'modal-dialog modal-xl', // use modal-sm, modal-lg to change width 'modal_route' => route('tag-inline-create'), // InlineCreate::getInlineCreateModal() 'create_route' => route('tag-inline-create-save'), // InlineCreate::storeInlineCreate() 'include_main_form_fields' => ['field1', 'field2'], // pass certain fields from the main form to the modal ]'

tm-blg commented 3 years ago

which is misleading.

newbie gets questions.

create a method with this field? how to return a field? via crud or return?

tm-blg commented 3 years ago

i already check all twice, now i have error, i have it before when follow docs:

Symfony\Component\Routing\Exception\RouteNotFoundException Route [items-inline-create-save] not defined. (View: W:\domains\crud\resources\views\vendor\backpack\crud\fields\relationship\fetch_or_create.blade.php)

and now have reason new question, how it fix? i want to beat this problem and rewrite docs for help other.

pxpm commented 3 years ago

I sure not going to change your code.

The example I provided you works flawless, I'v just tested it, so you should be able to change the names and have it working for you.

From what seems your ItemCrudController uses item and not items so just change it to:

'inline_create' => [
'entity' => 'item'
]

And don't forget to add InlineCreateOperation inside ItemCrudController and AFTER the CreateOperation

The places I mentioned are the right ones to place this type of question, not here on github!

Wish you the best of lucks,

Pedro

tabacitu commented 3 years ago

Hey @tm-blg - I see you've had quite a back-and-forth 2 days ago and things got heated 👀😂 Hopefully now that a few days have passed... spirits have cooled down, we can all take a relaxed look at it and maybe even joke about it 😀 I've unlocked the conversation in the hopes I can nudge it to a close, aaaand... bring it back on topic - prefixes for route names. Remember that? 😂

That being said, I feel like there's some miscommunication or unmet expectations here, and I want to be serious for a bit, in order to clear that up.


@tm-blg I totally understand your frustration - bad docs or bugs can make us lose hours, days and our temper. I've been there myself! I really am sorry that our software/docs/team has caused these negative feelings for you, hopefully it's the last time 🤞We make efforts to avoid this exact kind of feeling for you - and you've mentioned you're willing to help improve our docs, so thanks a lot for wanting to help 🙏

(1) Please take into consideration that Backpack does NOT officially offer support - at all. Not because we're lazy, but because it's economically impossible. Backpack has had 940.000 downloads in the past 5 years. If each install asked for ~1h of coding help, that means we needed 1626 full-time employees, in 8-hours shifts 24/7 for 5 years - just to provide support. Needless to say - we do NOT have that kind of resources. We're a 3-developer company, not a 1626+ developer company.

(2) But yeah... we do provide some support, on StackOverflow and Github, even if we don't have to. But that's just because we love interacting with our customers and helping them out when they're in need. Moreover, a lot of our customers help other customers, because we're all kind people helping other kind people - it's a beautiful community that's been developing here, for years, based on people being kind to each other. Open-source (or public-source more accurately for Backpack) is a beautiful beautiful example of humans being "at their best". But in order to benefit from it, to help people be at their best and help you... you have to also be at your best and phrase problems in a way that does not offend people, and actually makes them want to help you out. I'm actually passionate about this subject (written communication for developers), enough so that I've written guidelines and a book about it.

(3) When an issue has a topic, please try to only talk about that particular issue there. If you want to talk about something else, open another issue. For example now we're talking about things that have nothing to do with prefixes for route names, and as a result we're spamming people, and it'll be more difficult to manage this task in the future.


Ok enough about that. Let's be productive here:

(4) Did Pedro's answers and example help you debug your problem? Is you problem now solved? If so, please let us know, so I can move your conversation above somewhere else - it sounds to me like it has nothing to do with the subject of this particular issue. If you were NOT able to fix your issue with the answers above, please open an issue on StackOverflow with the backpack-for-laravel tag. That's where Backpack devs ask for community support - we keep Github for bugs & features only. See our contact page for more details.

(5) If you think this whole situation was because of bad docs - please send a PR to our docs or let us know how we can improve them. It's notoriously difficult to write docs for everybody to understand, when you're the one who's written the code and understands how it works. It's a human perspective thing. So we rely on people like you to let us know when things aren't clear in the docs, because you come from a fresh perspective.

Let me know if you disagree with me... or if you are willing to make this collaboration work 💪 Cheers!