fix(auth): default public_profile_allow_chat_with_me to true for new users - #157
Conversation
…users Aligns the User entity constructor and Doctrine column mapping so both agree with the 2023 migration's DB-level default (1/true), closing the code/DB contradiction flagged in ClickUp 86bbdxzt6. Previously the constructor set false while the DB default was already 1, so new accounts silently diverged from the intended on-by-default chat setting once any code path relied on the DB default instead of the explicit ORM insert. Adds a unit test asserting a freshly constructed User defaults to chat-allowed true.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe user profile chat setting now defaults to enabled in both the ORM column mapping and new ChangesUser profile chat default
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~5 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The intended chat-default change is consistently implemented and has focused test coverage, with no concrete merge-blocking risk identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
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. Comment |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/openstackid/openapi/pr-157/ This page is automatically updated on each push to this PR. |
There was a problem hiding this comment.
🟢 Approval recommended
No unresolved review issues were identified, and the new default is covered by a unit test.
Pull request overview
Updates new-user chat permissions to default to enabled, aligning the entity and database defaults.
Changes:
- Sets
public_profile_allow_chat_with_metotrue. - Updates the Doctrine mapping default to
1. - Adds regression coverage for the constructor default.
File summaries
| File | Description |
|---|---|
tests/unit/UserMappingTest.php |
Verifies new users allow chat by default. |
app/libs/Auth/Models/User.php |
Aligns entity and database defaults. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ref: https://app.clickup.com/t/9014802374/86bbdxzt6
Summary
Resolves the openstackid-side default mismatch flagged in ClickUp 86bbdxzt6: the
Userentity constructor setpublic_profile_allow_chat_with_metofalse, while a 2023 migration (Version20230601162315) had already moved the DB column default to1. Per JP's recommendation on the ticket, the default should be on, so this aligns the constructor and the Doctrine column mapping with the existing DB default instead of the other way around.Changes
app/libs/Auth/Models/User.php: constructor now setspublic_profile_allow_chat_with_me = true; the#[ORM\Column(...)]mapping'soptions: ['default' => ...]updated from0to1to match.tests/unit/UserMappingTest.php: addedtestDefaultPublicProfileAllowChatWithMeIsTrue, asserting a freshly constructedUserdefaults to chat-allowedtrue.Impact
New accounts created via registration, social login, OTP, or
CreateSuperAdmin(none of which pass this flag explicitly) now default to chat-allowedtrue, matching the DB default and closing the code/DB disagreement called out on the ticket.Testing
vendor/bin/phpunit tests/unit/UserMappingTest.php— 2 passed, 8 assertions (run inside theidp-appcontainer).false, confirmed the new test fails (Failed asserting that false is true.), restored the fix, confirmed it passes again.Summary by CodeRabbit
New Features
Tests