Skip to content

When formatting provided site name, only remove .tld if it's at the end#1297

Merged
mattstauffer merged 3 commits intolaravel:masterfrom
ErikDohmen:bugfix/1295-fix-tld-removal-only-if-at-end
Dec 1, 2022
Merged

When formatting provided site name, only remove .tld if it's at the end#1297
mattstauffer merged 3 commits intolaravel:masterfrom
ErikDohmen:bugfix/1295-fix-tld-removal-only-if-at-end

Conversation

@ErikDohmen
Copy link
Copy Markdown
Contributor

@ErikDohmen ErikDohmen commented Nov 2, 2022

(copied from Issue #1298 for context):

Description:
TL;DR: If your site name contains your TLD (e.g. if your TLD is .test and your site name contains .test) it will be removed inappropriately.

In line with our regular site setup we have the same locally with valet:

  • portal.test-work.test
  • welcome.test-work.test

The moment it goes wrong is when I try to use the isolate option. Then I get an error: portal-work isn't found as site name. That is correct as it is indeed portal.test-work. The problem seems to be that .test is removed not just at the end, but just as it is found in the site name.

Searches on internet doesn't bring any solutions to the table.

Steps To Reproduce:

  • add a link to your project with a specific site name in line with ours e.g. portal.test-work
  • try to isolate the site with any php version
  • you should get the error that the site name isn't found in the list of sites

Solutions in this PR:

  • Check if tld is at the end of the site name and, if so, remove it
  • Add corresponding test

Fixes #1295, Fixes #1298.

@ErikDohmen ErikDohmen force-pushed the bugfix/1295-fix-tld-removal-only-if-at-end branch from 0b3cbbf to 9e3a262 Compare November 2, 2022 10:50
o check if tld is at the end of the site name and if so remove it
o added corresponding test
@ErikDohmen ErikDohmen force-pushed the bugfix/1295-fix-tld-removal-only-if-at-end branch from 9e3a262 to 2d429c3 Compare November 2, 2022 10:55
Copy link
Copy Markdown
Collaborator

@mattstauffer mattstauffer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one small refactor and then this is good to go!

Comment thread cli/Valet/Site.php Outdated
@mattstauffer mattstauffer changed the title fix for tld removal at site name only if at the end When formatting provided site name, only remove .tld if it's at the end Nov 10, 2022
ErikDohmen and others added 2 commits November 11, 2022 09:37
Co-authored-by: Matt Stauffer <mattstauffer@users.noreply.github.com>
@ErikDohmen
Copy link
Copy Markdown
Contributor Author

@mattstauffer, thanks for the suggestion. I added it and added one more missing ) in your code suggestion to make it work.

@mattstauffer mattstauffer merged commit 11cb9ce into laravel:master Dec 1, 2022
@mattstauffer
Copy link
Copy Markdown
Collaborator

Thanks @ErikDohmen!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test in site name fails with isolation test in site name fails

2 participants