fix: ensure ban check for existing users #169
Conversation
WalkthroughThe pull request modifies the conditional logic in the Changes
Possibly related PRs
Suggested labels
Poem
Tip New featuresWalkthrough comment now includes:
Notes:
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 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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/Controllers/OAuthController.php (1 hunks)
Additional comments not posted (1)
src/Controllers/OAuthController.php (1)
114-114: Good safeguard added to prevent null reference errors.The addition of the check for
$this->userExistbefore invokingisBanned()is a prudent change that prevents potential null reference errors and ensures that the ban check is only applied to existing users, aligning with the PR's objectives.Please verify that
$this->userExistis correctly assigned in all relevant parts of the code to ensure that this change functions as intended.Verification successful
Verification successful:
$this->userExistis correctly assigned.The assignment of
$this->userExistis confirmed in theOAuthController.phpfile, ensuring that the null check and subsequentisBanned()call are valid and will function as intended. The change is correctly implemented.
$this->userExistis assigned with$findUser, ensuring it is set before use.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the assignment of `$this->userExist` in the codebase. # Test: Search for the assignment of `$this->userExist`. Expect: Assignments are correct and consistent. rg --type php '$this->userExist ='Length of output: 36
Script:
#!/bin/bash # Description: Search for any references to `$this->userExist` to understand its usage and assignment. # Test: Search for all occurrences of `$this->userExist` in the codebase. rg --type php '\$this->userExist'Length of output: 391
There was a problem hiding this comment.
Thank you for fixing this issue. I’ve been occupied with another project recently, so I haven’t had a chance to review the previous PR myself. I appreciate the fix.
If there’s anything else that needs attention or if further review is required, please let me know.
follow up: #167
Since we don't use
allow_register, I haven't had a chance to test its functionality.As a result, the
isBanned()check fails ifallow_registerconfig is set totrue. This PR ensures that the check applies only to existing users.Summary by CodeRabbit