-
Notifications
You must be signed in to change notification settings - Fork 2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
.org Plans: Fix site slug in plan cancellation survey #9207
Conversation
Code looks solid, and tests are passing. |
LGTM! |
@@ -130,7 +131,7 @@ const RemovePurchase = React.createClass( { | |||
notices.success( | |||
this.translate( '%(productName)s was removed from {{siteName/}}.', { | |||
args: { productName }, | |||
components: { siteName: <em>{ selectedSite.slug }</em> } | |||
components: { siteName: <em>{ slugToUrl( selectedSite.slug ) }</em> } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is doing exactly the same as the domain
attribute of the site entity, which is what should be used instead.
selectedSite.domain
instead of slugToUrl( selectedSite.slug )
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR fixes #9200, where the site slug is not properly formatted and displays
::
instead of/
when the Jetpack site is installed in a subdirectory.To test:
/
and not with::
in the "Are you sure you want to remove {PLAN} from {SLUG}" text in the removal step/
and not with::
in the "{PLAN} was removed from {SLUG}" text in the removal confirmation notice.slugToUrl
method pass:npm run test-client client/lib/url/test/index.js -- --grep "slugToUrl"
/cc @beaulebens @johnHackworth