[6.x] Use the router for absolute urls

[GrahamCampbell]

1. The first change removed is due to the fact that url already adds in the base path (and everywhere else in the codebase uses this feature already - just search for occurrences of url(route().
2. The second change is a proper fix for [7.x] Fix unit test routes when base url has a trailing slash #32344. Moreover, reading the app config file was a hack anyway. We should ask the router what it thinks the absolute url is.

[driesvints]

Maybe it's best that you explain a bit why this is needed?

[GrahamCampbell]

Updated OP to be clearer.

[taylorotwell]

Let's just do this on 7.x if we're not fixing any real bugs.

[GrahamCampbell]

It also fixes when someone has their app.url ending with a slash. It causes urls to be generated with 2 slashes instead of one.

[GrahamCampbell]

6.x is definitely the right place for this fix.

[driesvints]

@GrahamCampbell can you rebase? There are some conflicts.

[jasonvarga]

Is there a way to define the real URL used in tests?

I was using config(['app.url' => 'http://test.com']); and $this->get('/foo') and this change breaks it. Now it always thinks it's http://localhost

[jasonvarga]

Looks like I can do url()->forceRootUrl('http://test.com');

[mnabialek]

This is pretty breaking change. In case having tests for multi domains application it's quite possible that someone used config(['app.url' => 'http://test.com']); to set other domain. I believe it should not go to 6.x branch especially it's not obvious that why tests stopped working.

Thanks @jasonvarga - your solution seems to be working fine.

[GrahamCampbell]

We're currently treating this as a bug fix. If more people complain this has actually broken this tests, we will look at other options, including reverting. So far only one app has been affected.

[trevorgehman]

Ah, I was looking for the PR that caused my test suite to fail. Here it is! Just had to change:

config(['app.url' => 'http://api.app.test]);

to:

URL::forceRootUrl('http://api.app.test');

[halaei]

@GrahamCampbell
This caused my tests to fail too on Laravel 6. I am not sure it is a big deal but here is the test:

Let say I have https://main.domain.com as my config('app.url').
Also consider I have a routing that allows hitting https://main.domain.com/main-only but returns a 404 error if I hit https://secondary.domain.com/main-only

The following test used to pass, now it causes a 404:

$this->get('https://secondary.domain.com/something')->assertStatus(200);
$this->get('/main-only')->assertStatus(200); // Previously it was passed now I see 404 error here

It probably has no bad effect in production. But I need to change my test.

[alecpl]

It also made my tests failing. I switch the app.url for some tests to http://admin.<usual-app-host>, which is my admin API "entry-point". Fixed with @jasonvarga hack above.

[GrahamCampbell]

Thanks for the feedback everyone. I am very hesitant to revert this PR now, since most people to be affected by it have already fixed their code now. If we revert, it will only break everything again for everyone who changed their code!

[jordanade]

This broke the links in my password-reset emails, in my case because while new users were created in an admin subdomain, which triggered a password-reset with broker()->sendResetLink(), the APP_URL and password.reset route were on a different subdomain. Trying to figure out the best way to fix this.

[jordanade]

To answer my own question, this seemed easy enough to add to my User model:

    public function sendPasswordResetNotification($token)
    {
        \URL::forceRootUrl(config('app.url'));
        parent::sendPasswordResetNotification($token);
    }

I still don't really understand why this is happening, since if we're asking the router for the absolute URL, and my password.reset route is wrapped in a Route::domain that specifies the correct domain, why doesn't that route return the correct domain instead of the domain for the code that is calling it?

[furkanbasersk8]

any solve?

[furkanbasersk8]

protected function buildMailMessage($url)
{
return (new MailMessage)
->subject(Lang::get('Reset Password Notification'))
->line(Lang::get('You are receiving this email because we received a password reset request for your account.'))
->action(Lang::get('Reset Password'), $url)
->line(Lang::get('This password reset link will expire in :count minutes.', ['count' => config('auth.passwords.'.config('auth.defaults.passwords').'.expire')]))
->line(Lang::get('If you did not request a password reset, no further action is required.'));
}