-
Notifications
You must be signed in to change notification settings - Fork 17
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: Returns the user's email if the account is not found. #132
Conversation
fix: [BUG] Undefined property: stdClass::$family_name
WalkthroughThe recent changes enhance user feedback in the OAuth process by providing clearer and more specific error messages related to account issues linked to email addresses. Additionally, the code formatting has been standardized for improved readability. These updates aim to elevate the user experience during OAuth interactions, making it easier for users to understand and resolve any issues. Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
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.
Actionable comments posted: 2
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/Controllers/OAuthController.php (2 hunks)
- src/Language/en/ShieldOAuthLang.php (1 hunks)
- src/Language/fa/ShieldOAuthLang.php (1 hunks)
- src/Language/fr/ShieldOAuthLang.php (1 hunks)
- src/Language/id/ShieldOAuthLang.php (1 hunks)
Files skipped from review due to trivial changes (1)
- src/Language/en/ShieldOAuthLang.php
Additional comments not posted (6)
src/Language/id/ShieldOAuthLang.php (4)
17-17
: Approved: Improved clarity for unknown error message.The change simplifies the message while maintaining its meaning.
19-19
: Approved: Enhanced clarity for OAuth class error message.The change adds emphasis to the occurrence of an error and clarifies the class name.
20-20
: Approved: Improved directness for anti-forgery error message.The change removes the apologetic tone and makes the message more direct.
21-21
: Approved: Enhanced specificity for account not found error message.The change provides a more specific context by referencing the email directly.
src/Controllers/OAuthController.php (2)
53-53
: LGTM! Formatting change approved.The minor formatting adjustment standardizes the code and does not impact functionality.
88-88
: Verify the inclusion of the email in the error message.Including the user's email in the error message can improve user feedback but may also introduce security or privacy concerns. Ensure that this change complies with your application's privacy policy and does not expose sensitive information.
src/Language/fa/ShieldOAuthLang.php
Outdated
@@ -18,7 +18,7 @@ | |||
'Callback' => [ | |||
'oauth_class_not_set' => 'خطای رخ داد، به نظر میرسد کلاس OAuth مورد نظر به درستی تنظیم نشده است.', | |||
'anti_forgery' => 'متاسفانه، تلاش شما ، یک درخواست جعلی تشخیص داده شد.', | |||
'account_not_found' => 'اکانت پیدا نشد. لطفا ابتدا ثبت نام کنید!', | |||
'account_not_found' => '(To be translated) There is no account registered with the email "{0}".', |
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.
Translate the placeholder message.
The message for 'account_not_found' is currently in English and marked as "(To be translated)". It should be translated to Persian to maintain consistency with the rest of the file.
- 'account_not_found' => '(To be translated) There is no account registered with the email "{0}".',
+ 'account_not_found' => 'هیچ حسابی با ایمیل "{0}" ثبت نشده است.',
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
'account_not_found' => '(To be translated) There is no account registered with the email "{0}".', | |
'account_not_found' => 'هیچ حسابی با ایمیل "{0}" ثبت نشده است.', |
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 is a good suggestion, apply it.
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.
@datamweb, I'm glad you found the suggestion useful. Please apply the following change to ensure the translation is consistent:
'account_not_found' => 'هیچ حسابی با ایمیل "{0}" ثبت نشده است.',
If you need any further assistance, feel free to ask!
@@ -18,7 +18,7 @@ | |||
'Callback' => [ | |||
'oauth_class_not_set' => 'Une erreur s’est produite, il semble que la classe OAuth n’est pas définie.', | |||
'anti_forgery' => 'Votre demande a été détectée comme erronée. Nous sommes désolés!', | |||
'account_not_found' => '(To be translated) Your account not found. Please register first!', | |||
'account_not_found' => '(To be translated) There is no account registered with the email "{0}".', |
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.
Translate the placeholder message.
The message for 'account_not_found' is currently in English and marked as "(To be translated)". It should be translated to French to maintain consistency with the rest of the file.
- 'account_not_found' => '(To be translated) There is no account registered with the email "{0}".',
+ 'account_not_found' => 'Aucun compte n\'est enregistré avec l\'email "{0}".',
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
'account_not_found' => '(To be translated) There is no account registered with the email "{0}".', | |
'account_not_found' => 'Aucun compte n\'est enregistré avec l\'email "{0}".', |
6b4a6e2
to
ce15658
Compare
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/Controllers/OAuthController.php (1 hunks)
- src/Language/en/ShieldOAuthLang.php (1 hunks)
- src/Language/fa/ShieldOAuthLang.php (1 hunks)
- src/Language/fr/ShieldOAuthLang.php (1 hunks)
- src/Language/id/ShieldOAuthLang.php (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- src/Controllers/OAuthController.php
- src/Language/en/ShieldOAuthLang.php
- src/Language/fa/ShieldOAuthLang.php
- src/Language/fr/ShieldOAuthLang.php
- src/Language/id/ShieldOAuthLang.php
src/Language/id/ShieldOAuthLang.php
Outdated
'unknown' => 'Kesalahan tidak diketahui!', | ||
'Callback' => [ | ||
'oauth_class_not_set' => 'Terjadi kesalahan, sepertinya class OAuth tidak terpasang.', | ||
'anti_forgery' => 'Maaf, permintaan Anda terdeteksi tidak valid!', | ||
'account_not_found' => 'Akun tidak ditemukan, silakan mendaftar terlebih dahulu!', | ||
'oauth_class_not_set' => 'Terjadi Kesalahan: sepertinya class "OAuth" tidak terpasang.', | ||
'anti_forgery' => 'Permintaan ini terdeteksi tidak valid!', |
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.
One thing at a time: A pull request should only contain one change. That does not mean only one commit, but one change - however many commits it took. The reason for this is that if you change X and Y, but send a pull request for both at the same time, we might really want X but disagree with Y, meaning we cannot merge the request. Using the Git-Flow branching model you can create new branches for both of these features and send two requests.
So if you think translation id needs improvement, separate it in a PR.
src/Language/fa/ShieldOAuthLang.php
Outdated
@@ -18,7 +18,7 @@ | |||
'Callback' => [ | |||
'oauth_class_not_set' => 'خطای رخ داد، به نظر میرسد کلاس OAuth مورد نظر به درستی تنظیم نشده است.', | |||
'anti_forgery' => 'متاسفانه، تلاش شما ، یک درخواست جعلی تشخیص داده شد.', | |||
'account_not_found' => 'اکانت پیدا نشد. لطفا ابتدا ثبت نام کنید!', | |||
'account_not_found' => '(To be translated) There is no account registered with the email "{0}".', |
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 is a good suggestion, apply it.
ce15658
to
3ce0e44
Compare
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.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- src/Controllers/OAuthController.php (1 hunks)
- src/Language/en/ShieldOAuthLang.php (1 hunks)
- src/Language/fa/ShieldOAuthLang.php (1 hunks)
- src/Language/fr/ShieldOAuthLang.php (1 hunks)
- src/Language/id/ShieldOAuthLang.php (1 hunks)
Files skipped from review due to trivial changes (1)
- src/Language/id/ShieldOAuthLang.php
Files skipped from review as they are similar to previous changes (4)
- src/Controllers/OAuthController.php
- src/Language/en/ShieldOAuthLang.php
- src/Language/fa/ShieldOAuthLang.php
- src/Language/fr/ShieldOAuthLang.php
This helps the user confirm which email is not found or registered when trying to log in.
Summary by CodeRabbit