Fix for 177 #187
Fix for 177 #187
Conversation
WalkthroughThis pull request updates the Changes
Poem
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/Libraries/GoogleOAuth.php (2)
95-119: Consider handling missing properties in the user info response.While the property name correction is good, I notice the method doesn't validate if the expected properties exist in the
$userInfoobject before trying to access them. Onlyfamily_nameis handled with the null coalescing operator.protected function setColumnsName(string $nameOfProcess, $userInfo): array { if ($nameOfProcess === 'syncingUserInfo') { return [ - $this->config->usersColumnsName['first_name'] => $userInfo->given_name, - $this->config->usersColumnsName['last_name'] => $userInfo->family_name ?? null, - $this->config->usersColumnsName['avatar'] => $userInfo->picture, + $this->config->usersColumnsName['first_name'] => $userInfo->given_name ?? null, + $this->config->usersColumnsName['last_name'] => $userInfo->family_name ?? null, + $this->config->usersColumnsName['avatar'] => $userInfo->picture ?? null, ]; } if ($nameOfProcess === 'newUser') { return [ // users tbl // OAuth 'username' => $userInfo->email, 'email' => $userInfo->email, 'password' => random_string('crypto', 32), 'active' => $userInfo->email_verified, - $this->config->usersColumnsName['first_name'] => $userInfo->given_name, - $this->config->usersColumnsName['last_name'] => $userInfo->family_name ?? null, - $this->config->usersColumnsName['avatar'] => $userInfo->picture, + $this->config->usersColumnsName['first_name'] => $userInfo->given_name ?? null, + $this->config->usersColumnsName['last_name'] => $userInfo->family_name ?? null, + $this->config->usersColumnsName['avatar'] => $userInfo->picture ?? null, ]; } return []; }
68-70: Consider better error handling for exceptions.The current approach of using
exit($e->getMessage())immediately terminates the script and may expose sensitive error information to end users. Consider implementing more robust error handling, such as logging the error and providing a user-friendly message.} catch (Exception $e) { - exit($e->getMessage()); + log_message('error', 'Google OAuth error: ' . $e->getMessage()); + throw new Exception('Failed to connect to Google OAuth service. Please try again later.'); }Also applies to: 88-90
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/Libraries/GoogleOAuth.php(1 hunks)
🔇 Additional comments (1)
src/Libraries/GoogleOAuth.php (1)
99-99: Good fix: Using the correct property for first name data.The change from using
$userInfo->nameto$userInfo->given_namecorrectly aligns with Google's OAuth2 user info response format. This ensures that only the first name is stored in the corresponding database field, fixing issue #177. The approach is consistent with line 112 which was already usinggiven_namefor the 'newUser' process.
1 participant
A quick edit to the library class to use the correct google library property name so that only the first name is populated into the
userstable field for first name.Addresses #177.
Summary by CodeRabbit