[BugFix] avoid backpack:build to generate interfaces for abstract models and others

[promatik]

Fixes #120.

WHY

BEFORE - What was wrong? What was happening before this PR?

backpack:crud was generating CRUDs for abstract models, and it was also skipping valid models that simply don't extend directly Illuminate\Database\Eloquent\Model.

HOW

How did you achieve that, in technical terms?

• First it tries to guess the namespace by the file path
• Then it also tries to find the namespace by the file contents.

Is it a breaking change or non-breaking change?

No.

How can we test the before & after?

• Having an abstract Model on Models folder (should not generate CRUD)
• Having a model that extends another Model or an abstract Model (should generate CRUD)

@tabacitu I think I'll ask @pxpm to take a quick look at this one, he is aware of this kind of stuff, requiring files on runtime, guessing namespaces ... he may find something that I should not have done 😅

[tabacitu]

Yes please 🙏 Take it away @pxpm

[promatik]

By the way, @pxpm, note that;
#105 < this a different thing 😅
Later we may discuss that one.

[pxpm]

When we have the class, why we don't reflect on it ?
If the reflection fails we are sure it's not a model, if the reflection works we can check if it's a $reflection->isSubclassOf() (or something similar), and reflection also let us check if it's abstract with $reflection->isAbstract().

I think using reflection would also remove the need for: // Try to load it form file content scenario.

Let me know what you think.

[tabacitu]

[promatik]

Changed to Reflection 👌
Much better, it also prevents crashes due to errors on the files 👌

---

Regarding the need of // Try to load it form file content there may be situations where we can't infer class from file path.

1. Models misplaced (not sure if possible, but it might(?))
2. Models inside a package, in the packages folder.

In these cases (maybe others), we need to try to load them from parsing the file content right?

Let me know @pxpm 🙌

[pxpm]

Wouldn't this become a non-issue if we have the paths and namespaces configurable?

I think something like this:

'model_containers' => [
  [
    'namespace' => 'App'
    'path' =>app_path(),
    'recursive' => false,
  ],
  [
    'namespace' => 'App\Models'
    'path' =>app_path('Models'),
    'recursive' => true,
  ],
  [
    'namespace' => 'Vendor\Package',
    'path' =>  app_path('packages/mypackage/models'),
    'recursive' => true
  ]
];

This way the config would say:

• I want to get all models in the "root" app_path() only (not recursively)
• I want everything inside app_path('Models') and it should use the base namespace App\Models. We should recursively find models in that directiory, eg: App\Models\Auth\User, App\Models\Blog\Post.
• I have a local package in my app/packages that also have models, I want them, but the base namespace is Vendor/Package.

It would work for everyone and every project by changing the configs ... the defaults could be "permissive", but I could configure it other way if needed.

Let me know what you guys think @promatik @tabacitu

[tabacitu]

Configurable model paths is beyond the scope of this PR. And if you ask me - beyond the scope of Generators. DevTools? Sure, that's easier to use. But Generators?... nah.

Let's not expand the scope please.

[tabacitu]

Ping! What's the status of this @pxpm & @promatik - can I merge it, do I have the green light?

Cheers!

[promatik]

@pxpm I applied your previous suggestion, I'll assign it to @tabacitu 👌

[tabacitu]

Thanks @promatik - I just tested a use case that I thought would be covered by this PR, but looks like not? Here's what I did:

• created a new model in app\Models\Paper.php that extends the current Page:

<?php

namespace App\Models;

use Backpack\PageManager\app\Models\Page as OriginalPage;
use Illuminate\Database\Eloquent\Factories\HasFactory;

class Paper extends OriginalPage
{
    use HasFactory;
}

Now after I run php artisan backpack:build I expected to see a Paper CRUD... right? That's the point of this PR? Well... it's not there... 👀 Am I missing something?

[promatik]

@tabacitu I tried that, created that file, run the build, everything worked 👌

Can you try again?
Maybe your model was not loaded, or something in the use's was not correct, because the condition to validate is;

$reflection->isSubclassOf(Model::class)

[jcastroa87]

Hi there @promatik i create "Paper" model who extend "Page" model.

On main totally ignore this model, but it this branch "load-models-by-namespace", created the CRUD

And update the model

Look ready to merge for me.

Cheers.