thedevdojo / voyager

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

Issue - Can't be used in sub-directory (routes & assets) #130

Closed Thiktak closed 7 years ago

Thiktak commented 7 years ago

Hello,

I tried to install this awersome tools in a subdirectory ... but I got some issues. Indeed, i found:

maiorano84 commented 7 years ago

I think part of this might be corrected in yesterday's PR #120

In this, I had removed all static references to the /admin route, and created a configuration option to prefix Voyager routes with whatever you like. Let me know if this helps.

Thiktak commented 7 years ago

I downloaded your fork :)

Still some issues:

But yeah ! Good job :p

maiorano84 commented 7 years ago

Funny you should mention, I actually just found those on my own myself. My search was even more generic (searched on "admin"), so some of the leftover urls got lost in the noise. You should be good now.

Note that searching for "/admin" will yield two results:

One is a default value in the media uploader Javascript that is to be overridden by an object passed into its constructor

The other is a part of the README

Thiktak commented 7 years ago

Ok, thanks :) I will test. Do you want to fix assets too ? I already did it. I can request a PR to this repo or yours, if you want to include all on one package.

Thiktak commented 7 years ago

Yeah ! Yes, I see the javascript's one.

I think there is an issue. If you clear your view's cache (php artisan view:clear), you will have a Route [users.index] not defined.. Same for posts & cie. By default, there is no route for them.

maiorano84 commented 7 years ago

Interesting. I'll look into that.

Though, I'm wondering how that can be triggered from a view:clear command. Those routes should all be declared as resources, so - provided the database entries under the data_types table are there - those routes should be valid.

maiorano84 commented 7 years ago

I've run php artisan view:clear under a fresh Homestead installation. Unfortunately, I'm not able to replicate your issue.

If you could, can you detail out how to replicate the problem either on PR #120 or in my fork? That would certainly help narrow things down.

Thiktak commented 7 years ago

Ok. I will restart an fresh installation. Maybe the instllation part shold make some magic :)

Issue - Route [voyager.database.store_bread] not defined.

maiorano84 commented 7 years ago

Ah, no, you're right about that one. I didn't alias the store_bread route. New commit is ready.

Thiktak commented 7 years ago

Yeah ! fixed ! I think some magic happened during installation :)

BUT, yes, there is a but ...

maiorano84 commented 7 years ago

Breadcrumbs appear to be pointing fine on my side. Do you mean the links that are generated in the menu itself?

Those come from the built-in seeder. The following commit corrects that issue, but there's a caveat:

https://github.com/the-control-group/voyager/pull/120/commits/ffa59e7c34deae5c40a391ee20adf25b2d157a2f

In order for those links to be generated with the correct prefix, the config value needs to be set before running the installer. You can test this yourself by creating a fresh installation, setting the voyager.routes.prefix config value, and then running the installation process. The links will be seeded properly then.

Otherwise, seeding the database with the wrong prefix will necessitate changing each route in each menu item.

Provided my PR is accepted, I can probably work on making this an interactive part of the installation process (ie: User is presented with a prompt to set the route prefix during the voyager:install process)

Thiktak commented 7 years ago

Hi,

Indeed. It's working :)

On my side, the breadcrumbs still not working. I edited the master.blade.php view add changing line 82 : <li class="active"><a href="{{ route('voyager.dashboard') . '/..' . $breadcrumb_url }}">

ChrisThompsonTLDR commented 7 years ago

I recommend moving to using a route namespace to silo all the routes related to voyager.

Also, a route prefix would be nice. I have routes from Voyager colliding with existing routes now. I see a prefix being applied in VoyagerServiceProvider.php. Is there a reason it's not in the routes.php file?

ChrisThompsonTLDR commented 7 years ago

src/views/tools/database/edit-add-bread.blade.php line 27 doesn't respect the route prefix. It's still hardcoded to /admin/.

ChrisThompsonTLDR commented 7 years ago

None of the menu_items appear to respect the voyager.routes.prefix config param.

maiorano84 commented 7 years ago

In my branch, it did:

https://github.com/maiorano84/voyager/blob/route-prefix/src/views/tools/database/edit-add-bread.blade.php

It appears the author incorrectly merged another piece of functionality that affected this:

https://github.com/the-control-group/voyager/blob/master/src/views/tools/database/edit-add-bread.blade.php

maiorano84 commented 7 years ago

@tnylea Are you able to remediate?

tnylea commented 7 years ago

Thanks @maiorano84 :) I've just merged your PR and will update the latest version within a few hours. I'll be writing more tests as well to prevent these from happening :)

Thanks.

maiorano84 commented 7 years ago

Anytime, brotato chip. Keep up the great work! It's still a young project, but it's showing a lot of promise!

github-actions[bot] commented 4 years ago

This issue has been automatically locked since there has not been any recent activity after it was closed. If you have further questions please ask in our Slack group.