feat(auth): add timezone field to AbstractUser#439
Open
nossila wants to merge 6 commits into
Open
Conversation
Add a writable, free-form `timezone` field (IANA name, blank default) to AbstractUser alongside preferred_language, and expose it on UserBaseSerializer as an owner-private field (in fields + private_fields, not read_only). Includes the testproject migration and tests for updating it, owner-only visibility, and the owner field-set. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughAdds an optional Changestimezone field on AbstractUser
Dockerfile Git configuration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a user “timezone” preference to baseapp_auth (similar to preferred_language) and wires it through the REST API and test suite, with a supporting migration in the testproject concrete user model.
Changes:
- Add
AbstractUser.timezone(CharField(max_length=64, blank=True, default="")) tobaseapp_auth. - Expose
timezoneas an owner-private, writable field on the RESTUserBaseSerializer. - Add
testprojectmigration and integration tests covering owner visibility and PATCH updates.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
baseapp_auth/models.py |
Adds the timezone field to the abstract user model. |
baseapp_auth/rest_framework/users/serializers.py |
Exposes timezone in REST serialization as an owner-private writable field. |
baseapp_auth/tests/integration/test_users.py |
Extends integration tests to assert the field is present for self and writable via PATCH. |
testproject/users/migrations/0007_user_timezone.py |
Adds the DB column for the testproject concrete User model. |
Dockerfile |
Removes a git safe.directory step that could abort in submodule builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@baseapp_auth/models.py`:
- Around line 103-109: The timezone CharField in the model currently accepts any
arbitrary string value without validation against IANA timezone standards. Add a
field-level validator to the timezone field that validates the input against a
list of valid IANA timezone names and rejects invalid values like "foo/bar"
before they are persisted to the database. You can implement this by either
adding a validators parameter with a custom validator function that checks
against a known list of IANA timezones, or by using Django's choices parameter
to constrain the field to only valid timezone values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: dfd4462a-60d0-4048-8e8b-e7ad51c336d9
📒 Files selected for processing (5)
Dockerfilebaseapp_auth/models.pybaseapp_auth/rest_framework/users/serializers.pybaseapp_auth/tests/integration/test_users.pytestproject/users/migrations/0007_user_timezone.py
💤 Files with no reviewable changes (1)
- Dockerfile
Reject values that aren't valid IANA timezone names (e.g. "foo/bar") via a field-level validator backed by zoneinfo.available_timezones(); blank is still allowed. Includes a regression test for invalid input. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Add timezone to the user edit form beside preferred_language, surface both as changelist columns, and add a preferred_language list filter. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Run the git safe.directory step on a clean working dir (before COPY) and widen it to '*'. When built as a submodule, /usr/src/app/.git is a gitlink file pointing outside the build context, which made git abort repo discovery if the step ran from /usr/src/app. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Fold the validator AlterField (0008) back into the field's AddField (0007) so the testproject has a single timezone migration instead of two. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@baseapp_auth/tests/integration/test_users.py`:
- Around line 155-160: The test test_cant_update_timezone_with_invalid_value has
a weak assertion that only verifies the timezone is not set to the invalid value
"foo/bar", but does not ensure it remains unchanged. Store the original timezone
value from user_client.user.timezone before making the PATCH request, then after
refresh_from_db() and the 400 response assertion, replace the current inequality
check with an equality assertion that confirms the timezone matches the original
stored value, ensuring complete persistence validation.
In `@Dockerfile`:
- Around line 44-48: The RUN git config --global --add safe.directory '*'
command uses a global wildcard pattern that weakens Git's repository ownership
protection for the entire image. Replace the global wildcard setting with either
removing the command entirely (if no post-copy Git operations require it) or
scope the safe.directory trust to the specific repository path `/usr/src/app`
only, using git config --global --add safe.directory /usr/src/app instead. This
maintains the functionality for the current repo while avoiding unnecessary
global security exposure.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 35ff1f24-ac08-423c-a26f-7a7ef18d121d
📒 Files selected for processing (5)
Dockerfilebaseapp_auth/admin.pybaseapp_auth/models.pybaseapp_auth/tests/integration/test_users.pytestproject/users/migrations/0007_user_timezone.py
🚧 Files skipped from review as they are similar to previous changes (2)
- testproject/users/migrations/0007_user_timezone.py
- baseapp_auth/models.py
Comment on lines
+155
to
+160
| def test_cant_update_timezone_with_invalid_value(self, user_client): | ||
| data = {"timezone": "foo/bar"} | ||
| r = user_client.patch(self.reverse(kwargs={"pk": user_client.user.pk}), data) | ||
| h.responseBadRequest(r) | ||
| user_client.user.refresh_from_db() | ||
| assert user_client.user.timezone != "foo/bar" |
There was a problem hiding this comment.
Assert exact persistence on invalid timezone update.
Line 160 currently checks != "foo/bar", which can still pass if the value changes to something else. Capture the original value before PATCH and assert it is unchanged after the 400 response.
Suggested test hardening
def test_cant_update_timezone_with_invalid_value(self, user_client):
+ previous_timezone = user_client.user.timezone
data = {"timezone": "foo/bar"}
r = user_client.patch(self.reverse(kwargs={"pk": user_client.user.pk}), data)
h.responseBadRequest(r)
user_client.user.refresh_from_db()
- assert user_client.user.timezone != "foo/bar"
+ assert user_client.user.timezone == previous_timezone🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@baseapp_auth/tests/integration/test_users.py` around lines 155 - 160, The
test test_cant_update_timezone_with_invalid_value has a weak assertion that only
verifies the timezone is not set to the invalid value "foo/bar", but does not
ensure it remains unchanged. Store the original timezone value from
user_client.user.timezone before making the PATCH request, then after
refresh_from_db() and the 400 response assertion, replace the current inequality
check with an equality assertion that confirms the timezone matches the original
stored value, ensuring complete persistence validation.
Comment on lines
+44
to
+48
| # Trust all repo paths for git, using '*' so it covers the submodule case. Run it BEFORE | ||
| # the COPY so it executes on a clean working dir: when this repo is built as a submodule, | ||
| # the copied /usr/src/app/.git is a gitlink file pointing outside the build context, which | ||
| # makes git abort repo discovery (even for `config --global`) if run from /usr/src/app. | ||
| RUN git config --global --add safe.directory '*' |
There was a problem hiding this comment.
Avoid global safe.directory='*'; scope trust to this repo path only.
On Line 48, trusting * globally weakens Git’s repository ownership protection for the entire image. Prefer either removing this setting (if no post-copy Git ops need it) or scoping to /usr/src/app only.
Suggested hardening
-RUN git config --global --add safe.directory '*'
+RUN git config --global --add safe.directory /usr/src/app📝 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.
Suggested change
| # Trust all repo paths for git, using '*' so it covers the submodule case. Run it BEFORE | |
| # the COPY so it executes on a clean working dir: when this repo is built as a submodule, | |
| # the copied /usr/src/app/.git is a gitlink file pointing outside the build context, which | |
| # makes git abort repo discovery (even for `config --global`) if run from /usr/src/app. | |
| RUN git config --global --add safe.directory '*' | |
| # Trust all repo paths for git, using '*' so it covers the submodule case. Run it BEFORE | |
| # the COPY so it executes on a clean working dir: when this repo is built as a submodule, | |
| # the copied /usr/src/app/.git is a gitlink file pointing outside the build context, which | |
| # makes git abort repo discovery (even for `config --global`) if run from /usr/src/app. | |
| RUN git config --global --add safe.directory /usr/src/app |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Dockerfile` around lines 44 - 48, The RUN git config --global --add
safe.directory '*' command uses a global wildcard pattern that weakens Git's
repository ownership protection for the entire image. Replace the global
wildcard setting with either removing the command entirely (if no post-copy Git
operations require it) or scope the safe.directory trust to the specific
repository path `/usr/src/app` only, using git config --global --add
safe.directory /usr/src/app instead. This maintains the functionality for the
current repo while avoiding unnecessary global security exposure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Add a user timezone preference to
baseapp_auth, alongsidepreferred_language.AbstractUser.timezone— free-formCharField(max_length=64, blank=True, default="")(IANA name).UserBaseSerializeras an owner-private, writable field (added tofields+private_fields, kept out ofread_only_fields) — mirrorspreferred_language.testprojectmigration for the concrete test user.Also drops a
git config --global --add safe.directoryDocker step that aborted when the repo is built as a submodule.Test plan
docker compose exec web pytest baseapp_auth/tests/integration/test_users.py→ timezone update + field-set tests pass (the one unrelated failure is a Celery broker connection error — no broker in the isolated compose).black/isort/flake8clean on changed files.🤖 Generated with Claude Code
Summary by CodeRabbit
Release Notes