responsiv / campaign-plugin

[PREMIUM] Send professional campaign messages to your subscribers.
http://octobercms.com/plugin/responsiv-campaign
5 stars 2 forks source link

Base app url not considered by `unsubscribe_url` and `browser_url` #29

Closed oliverpool closed 7 years ago

oliverpool commented 7 years ago

Hi, my october installation is in a subfolder http://domain.tdl/october/. When I send the test message, everything is fine and the {unsubscribe_url} and {browser_url} are working (relative to http://domain.tdl/october/: http://domain.tdl/october/campaign/...).

However when I launch the campaign, the {unsubscribe_url} and {browser_url} in the email are relative to the root of my domain http://domain.tdl/ (http://domain.tdl/campaign/...), which returns a 404!


Configuration in the .env:

QUEUE_DRIVER=database
APP_URL=http://domain.tdl/october/

Crontab is calling the /path/to/file/artisan schedule:run

oliverpool commented 7 years ago

However my own links (inserted via {{'impressum'|staticPage}} for instance) refer to the correct root in both cases!

oliverpool commented 7 years ago

@daftspunk : any idea on this bug?

daftspunk commented 7 years ago

My first guess would be the APP_URL is not being read correctly from your environment settings? Since the launched messages come from the crontable, this is the only source of truth when generating the URLs. There is no other way for the cron to know what the URL is, so it must be incorrect here.

To test this theory, run php artisan tinker and paste in this code

Cms::url('/');

You should see some output like this

>>> Cms::url('/hello');
=> "http://localhost/hello"

Where http://localhost is the APP_URL definition. This is where the campaign manager sources its base url from.

oliverpool commented 7 years ago

I have the following line in my .env file: APP_URL=http://localhost:8001/public/, but it outputs

>>> Cms::url('/');
=> "http://localhost:8001"

However Dotenv seems to work:

>>> Dotenv::findEnvironmentVariable('APP_URL')
=> "http://localhost:8001/public/"
>>> env('APP_URL')
=> "http://localhost:8001/public/"
oliverpool commented 7 years ago

It seems to be related to laravel: https://stackoverflow.com/a/29515452/3207406 (but it would be very nice if october could fix this!)

daftspunk commented 7 years ago

Have you tried the solution?

Url::forceRootUrl( Config::get('app.url') );
Cms::url('/');

If it works then maybe we can look at implementing it.

oliverpool commented 7 years ago

Yes, it seems to fix it!

>>> Cms::url('/');
=> "http://localhost:8001"

>>> Url::forceRootUrl(config('app.url'));
=> null
>>> Cms::url('/');
=> "http://localhost:8001/public"
daftspunk commented 7 years ago

The conversation in https://github.com/laravel/framework/pull/3918 is difficult to watch. This issue needs to be moved to the main October repo so it can be addressed properly.

Will leave this issue open as a point of reference.

daftspunk commented 7 years ago

Fixed in https://github.com/octobercms/library/commit/979f549952992c8da259810f51a7dc8a0f193b30. This will be available in Build 402 and onwards.

In the meantime, feel free to add these lines of code yourself to this file ~\vendor\october\rain\src\Foundation\Bootstrap\RegisterOctober.php inside the bootstrap() method. It will be deployed with the next stable rollout.

    /*
     * Workaround for CLI and URL based in subdirectory
     */
    if ($app->runningInConsole()) {
        $app['url']->forceRootUrl($app['config']->get('app.url'));
    }

Thanks for reporting this.