thedevdojo / voyager

Voyager - The Missing Laravel Admin
https://voyager.devdojo.com
MIT License
11.77k stars 2.67k forks source link

Laravel 10 support #5753

Open jf-m opened 1 year ago

jf-m commented 1 year ago

This PR includes the last bits to make Voyager Laravel 10 compatible (work started there #5732 ).

Here's a breakdown of what I did:

This PR passes all the phpunit tests up until PHP 8.2 on my fork (https://github.com/jf-m/voyager/actions/runs/4600528910).

Any comments welcome, hope this helps !

mdenitti commented 1 year ago

This looks dodgy...anyone tested successfuly?

jf-m commented 1 year ago

@mdenitti can you clarify what is that you find "dodgy" ? Like stated before, you can add comments if there's something wrong from your point of view.

I must say that my auto-indent might have removed some spaces in the composer.json and phpunit.xml. Maybe this is what confuses you ?

I can add some more explanations about how I solved the issue at the first place.

The initial problem from the current state of the branch 1.6-l10 can be seen here: https://github.com/the-control-group/voyager/actions/runs/4080000210/jobs/7498342456?pr=5732.

ErrorException: require_once(/home/runner/work/voyager/voyager/vendor/orchestra/testbench-core/laravel/vendor/autoload.php): Failed to open stream: No such file or directory

It came from the following line of code:

https://github.com/the-control-group/voyager/blob/a74198baf70f26ca6aa02723aea39a03cc45b043/src/Commands/InstallCommand.php#L146

This require_once, to my knowledge, is here for compatibility reasons, so Voyager can be compatible for both Laravel < 7 and Laravel 8+. Indeed, since Laravel 8 and above, the seeds folder is called seeders and the namespace Database\Seeders; is mandatory on the seeders.

Right now, Voyager Seeder system is based on Laravel <7 with seeds and no namespace. Voyager in the InstallCommand check the Laravel version, if 8+ then it will rename the seeds to seeders and it will manually add the namespace to all the seeders files here : https://github.com/the-control-group/voyager/blob/a74198baf70f26ca6aa02723aea39a03cc45b043/src/Commands/InstallCommand.php#L165-L181

In my PR, I removed all this, because Voyager is no more compatible for Laravel <7. I renamed seeds to seeders and I added all the needed namespaces. By doing so, the require_once base_path('vendor/autoload.php'); is no more necessary because the seeders file have not changed (the Namespace is already there). This solved the main issue.

jf-m commented 1 year ago

@mdenitti I added a comment on each major changes for more clarity, to help the reviewer. Hope this helps. 👍

mdenitti commented 1 year ago

@mdenitti I added a comment on each major changes for more clarity, to help the reviewer. Hope this helps. 👍

Jean! Wonderfull this looks much better; Genius... Merging still blocked???? We need L10 support now :)))

dmatis2 commented 1 year ago

Is it worth downgrading somehow to L9 from L10? Or there will be support soon? 😊

hazbu commented 1 year ago

try this, worked for me on laravel 10 composer require tcg/voyager dev-1.6-l10

thank later

mdenitti commented 1 year ago

composer require tcg/voyager dev-1.6-l10

Found 1 security vulnerability advisory affecting 1 package. Run composer audit for a full list of advisories.

Installation failed, reverting ./composer.json and ./composer.lock to their original content.

octoxan commented 1 year ago

How is this taking so long to get merged? It works!

JonathanVorich commented 1 year ago

It looks like the-control-group do not intend to maintain the project any further. They have had no reaction since February, when I first mentioned the issue with Laravel 10. And also some links in the documentation no longer work. I think this repository will no longer be supported. Sad.

haydar commented 1 year ago

@marktopper, @marktopper Hello, I hope both of you are alive. Could you please pay some attention here?

sajaddp commented 1 year ago

Is this package no longer updated to support new versions of Laravel?