-
Notifications
You must be signed in to change notification settings - Fork 9
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
feat: disallow sign up with duplicate domain [ref: #517] #650
Conversation
inc/rest.php
Outdated
return new WP_REST_Response( | ||
[ | ||
'data' => null, | ||
'message' => sprintf( __( 'Error: This site has been previously registered. You can try again %1$shere%2$s ', 'optimole-wp' ), '<a href="https://dashboard.optimole.com/register" target="_blank"> ', '</a>' ), |
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.
@HardeepAsrani, what's the advantage of redirecting to the register page?
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.
Followed the same notice pattern for the notice if the user is already registered. In case you want to try to log in as there is no other way to log in using a username/password from the plugin.
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.
@HardeepAsrani my point is more if we don't allow people to register with the same domain, what's the point of redirecting them to the register page?
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.
@selul Got it. I'll replace it with the login link and modify the message accordingly.
@HardeepAsrani Tested and it works fine when trying to create a new account using the wp plugin. I still can use a different account by manually creating it on the dashboard and connecting the website with the API key, is this expected? |
@irinelenache Yes, that's expected. Should I move it to Ready to Merge? |
@HardeepAsrani could you please fix the conflicts on this PR? Thank you! |
@abaicus Should be fine to merge now. |
🎉 This PR is included in version 3.10.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
All Submissions:
Changes proposed in this Pull Request:
Needs & Details at: https://github.com/Codeinwp/optimole-service/pull/1050
Closes https://github.com/Codeinwp/optimole-service/issues/517.
How to test the changes in this Pull Request:
Other information: