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
Add callback URL domain to auth screen #45445
Conversation
Hi @jorgeatorres, Apart from reviewing the code changes, please make sure to review the testing instructions as well. You can follow this guide to find out what good testing instructions should look like: |
Test Results SummaryCommit SHA: d902b52
To view the full API test report, click here. To view the full E2E test report, click here. To view all test reports, visit the WooCommerce Test Reports Dashboard. |
<p> | ||
<?php | ||
/* Translators: %s Domain name from the callback URL. */ | ||
printf( esc_html__( 'Credentials will be shared with the website %s.', 'woocommerce' ), '<strong>' . esc_html( wp_parse_url( $callback_url, PHP_URL_HOST ) ) . '</strong>' ); |
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.
A few small/optional things, just wanted to briefly discuss them:
- Should we drop the use of the word 'website'? It implies that accessing the domain should lead to a website of some kind, which may not always be the case.
- I guess this is implicit, but I'm wondering about structuring the sentence something like: "If you approve this request, credentials will be shared with <DOMAIN>. You should not proceed if you have any concerns about the validity of that domain." Overkill?
If we prefer to go ahead without these changes, totally fine: it works well, is already a nice enhancement and I'm hitting approve.
... +I guess we need a template version tag bump. |
* add callback URL domain to auth screen * add changelog file * bump template version * tweak wording
Submission Review Guidelines:
Changes proposed in this Pull Request:
Add callback URL domain to auth screen:
How to test the changes in this Pull Request:
Using the WooCommerce Testing Instructions Guide, include your detailed testing instructions:
wp-env start
http://localhost:8888/wc-auth/v1/authorize?app_name=Example+App&scope=read_write&user_id=1&return_url=https%3A%2F%2Fwoo.com%2F&callback_url=https%3A%2F%2Fexample.com
and make sure it looks similar to the screenshot aboveChangelog entry
Significance
Type
Message
Comment