Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Improvement] Change CRUD command & route from singular to plural #63

Open
3 tasks
tabacitu opened this issue Mar 2, 2020 · 12 comments
Open
3 tasks

[Improvement] Change CRUD command & route from singular to plural #63

tabacitu opened this issue Mar 2, 2020 · 12 comments

Comments

@tabacitu
Copy link
Member

tabacitu commented Mar 2, 2020

Right now we ask people to run

php artisan backpack:crud tag # use singular, not plural

Which is a little counter-intuitive. Since the db table is plural, I think it'll be easier for people to run the plural name instead. We chose to use the singular because, in the past, the Laravel best practice was to use singular for Resource Routes, and for Controller name. Now the best practice is to have plural for resource route (Route::resource('tags', 'TagController');) but still singular for Controller name. That's why, I think, in addition to changing the command from singular to plural, we should also change the route. So that it's:

-admin/tag
-admin/tag/create
-admin/tag/update
+admin/tags
+admin/tags/create
+admin/tags/update

But this means we need to be super-sure about both the single and plural name, basically asking both from the user. So maybe instead we should do something like this:

php artisan backpack:crud                           # no mandatory parameters

# What's the table in the database?
> tags
     # [ERROR] Did not find table "tags" in the database.
# Confirm or edit your entity name, in plural, in English? (ex: tags, products, users)
> tags
# Confirm or edit your entity name, in singular, in English? (ex: tag, product, user)
> tag
# Found Model "App\Tag" - do you want to use it? (y/n)
#    (N) > Where do you want to generate your new Model?
         > App\Models\Tag
#
# ---------------
# Generated app/Models/Tag.php
# Generated app/Http/Requests/Tag.php
# Generated app/Http/Controllers/Admin/TagCrudController.php
# Added Route::crud('tag', 'TagCrudController'); to routes/backpack/custom.php
# Added sidebar item to resources/views/vendor/backpack/base/inc/sidebar_contents.blade.php
# ---------------
#       Done 
# ---------------

Once you specify the table name (tags) we can offer sensible defaults for the rest of the prompts:

  • we already have the name of the entity in plural - tags;
  • we can get the singular by stripping the "s" - tag;
  • we can check the app folder for any class that extends Model that is either called Tag.php, or has $table = 'tags'; inside it; to see if the dev already has a Model they might want to use;
  • we know a good name place for the generated Model would be App\Models\Tag.php

This way, you don't have to remember if it's singular or plural, the command-line prompt tells you exactly what it needs. You just need to remember php artisan backpack:crud - which is super-easy to remember. Possible alias: php artisan make:crud. For backwards-compatibility, we should probably keep the "singular" as an OPTIONAL parameter. Or we could leave php artisan backpack:crud as-is, and create this new command php artisan make:crud which is more powerful. But I see no point in having both, tbh.

To sum up, what I propose we do here:

  • make parameter in php artisan backpack:crud optional;
  • prompts to ask for the table name; then ask to confirm both singular and plural form of the entity; and if there's already a Model, ask if they want to use the existing model;
  • change CRUD route to use plural form, just like table name: Route::crud('tags', 'TagCrudController');
@eduardoarandah
Copy link

Agree!
Spanish has some non-regular plural variations

I tend to use singular everywhere (why complicate life?)
except from things the user needs to see, like urls

@tabacitu
Copy link
Member Author

Update: I think we've got enough breaking changes as-is. And plenty of people have already installed 4.1 with the singular form. So let's postpone this for 6 months, until Backpack 4.2 - I hope that one will have less breaking changes.

@tabacitu tabacitu changed the title [4.1][Improvement] Change CRUD command & route from singular to plural [4.2][Improvement] Change CRUD command & route from singular to plural Apr 24, 2020
@stale
Copy link

stale bot commented Jun 23, 2020

Hi there!

Is this still an issue? No activity in 60 days. I'm going to mark it as stale for now, and close it in 14 days if no further activity occurs. I know you guys are all busy, but if this is important to you please reply or something, so I know not to close it.

Thank you!

--
Justin Case
The Backpack Robot

@joelmellon
Copy link

joelmellon commented Aug 17, 2020

@tabacitu "But this means we need to be super-sure about both the single and plural name, basically asking both from the user."

Laravel's Str::plural() works pretty well. Could you just use that?

"we can get the singular by stripping the 's' - tag;" won't always work, but again, Str::plural() should handle these cases just fine (starting with singular). I think this means that you wouldn't have to change the user interface either.

@tabacitu tabacitu changed the title [4.2][Improvement] Change CRUD command & route from singular to plural [Improvement] Change CRUD command & route from singular to plural Oct 15, 2020
@tabacitu
Copy link
Member Author

Yeah you're right @joelmellon , it works pretty well in English. And class names, models should be in English anyway. So that should work well!

Also note here (TLDR of my comment in Laravel-Backpack/CRUD#3121 (comment)) - we should also split the words by dashes! To end up with user-setting not usersetting 😌

@stale
Copy link

stale bot commented Dec 25, 2020

Hi there!

Is this still an issue? No activity in 60 days. I'm going to mark it as stale for now, and close it in 14 days if no further activity occurs. I know you guys are all busy, but if this is important to you please reply or something, so I know not to close it.

Thank you!

--
Justin Case
The Backpack Robot

@stale stale bot added the stale label Dec 25, 2020
@joelmellon
Copy link

Also note here (TLDR of my comment in Laravel-Backpack/CRUD#3121 (comment)) - we should also split the words by dashes! To end up with user-setting not usersetting 😌

That's a great idea. Solves the most if not all of the issues here I think.

@stale stale bot removed the stale label Dec 25, 2020
@stale
Copy link

stale bot commented Jun 26, 2021

Hi there!

Is this still an issue? No activity in 60 days. I'm going to mark it as stale for now, and close it in 14 days if no further activity occurs. I know you guys are all busy, but if this is important to you please reply or something, so I know not to close it.

Thank you!

--
Justin Case
The Backpack Robot

@pxpm
Copy link
Contributor

pxpm commented Jul 2, 2021

bump!

@stale
Copy link

stale bot commented Nov 28, 2021

Hi there!

Is this still an issue? No activity in 60 days. I'm going to mark it as stale for now, and close it in 14 days if no further activity occurs. I know you guys are all busy, but if this is important to you please reply or something, so I know not to close it.

Thank you!

--
Justin Case
The Backpack Robot

Copy link

stale bot commented Feb 11, 2024

Hi there!

Is this still an issue? No activity in 60 days. I'm going to mark it as stale for now, and close it in 14 days if no further activity occurs. I know you guys are all busy, but if this is important to you please reply or something, so I know not to close it.

Thank you!

--
Justin Case
The Backpack Robot

@stale stale bot added the stale label Feb 11, 2024
@pxpm
Copy link
Contributor

pxpm commented Feb 12, 2024

bump

@stale stale bot removed the stale label Feb 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

4 participants