archtechx / tenancy

Automatic multi-tenancy for Laravel. No code changes needed.
https://tenancyforlaravel.com
MIT License
3.68k stars 434 forks source link

Fix tenant_route without absolute #1228

Closed hecktor-devsquad closed 6 months ago

hecktor-devsquad commented 6 months ago

This PR aims to resolve the issue when trying to pass Absolute: false to the tenant_route helper

When we try to generate a route with the tenant_route helper, passing absolute as false, we receive the following error. tenant_route(): argument #1 ($domain) must be of type string, null provided, called on line 1.

stancl commented 6 months ago

What use case do you have for calling the tenant_route() helper like this? It's a route() helper that generates the URL on a specific hostname, so how would relative URLs work?

hecktor-devsquad commented 6 months ago

I need to send a protocolless URL, and the easiest way to get it using this helper is to set absolute to false.

More like parse_url in these cases returns null and we treat strings later, it ends up giving the error.

willypriv commented 6 months ago

I'm having the same problem here, I need to get the url without the 'https', but if I pass the absolute as false I get the same error.

stancl commented 6 months ago

That to me sounds like abusing the implementation details of tenant_route() tbh. The helper is meant to generate absolute URLs with the specified hostname. Passing absolute: false would be a hack, since you still want an absolute URL, just without https://. Just do string manipulation on the result or write a custom helper.

hecktor-devsquad commented 6 months ago

@stancl Ok, so it doesn't make any sense for this helper to have the absolute parameter as it already is!

stancl commented 6 months ago

I think I went with that function signature to keep it identical to route(), but yeah, $absolute will be removed since it doesn't make sense to use here. Either way, it has nothing to do with whether the protocol is included, that's not what absolute/relative routes refers to.