Add Unix username field#15
Merged
Merged
Conversation
# Conflicts: # userapp/core/schemas/general.py # userapp/core/schemas/users.py
Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new username field intended to represent a Unix username alongside netid, and updates API schemas/views/tests to expose it while preserving legacy “auth_*” compatibility fields.
Changes:
- Adds
usernameto theuserstable and to thejoined_projectsDB view (with an Alembic migration to backfill existing rows). - Updates Pydantic schemas (
UserGet,UserPost,UserPatch,JoinedProjectView) to includeusernameand recomputeauth_netid/auth_usernamebased onusernamevsnetid. - Updates/reshapes user API tests and test payload generation to include the new field and validate the new compatibility behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
userapp/core/schemas/users.py |
Adds username to user schemas and updates computed auth compatibility fields / POST validation. |
userapp/core/schemas/general.py |
Adds username to JoinedProjectView schema and updates computed auth compatibility fields. |
userapp/core/models/views.py |
Extends the SQLAlchemy view model to include username. |
userapp/core/models/tables.py |
Adds username column (unique) to the User table model. |
userapp/api/tests/test_users.py |
Updates user endpoint tests and adds schema/auth behavior tests for username vs netid. |
userapp/api/tests/test_users_active_field.py |
Removes redundant/older active/auth compatibility tests. |
userapp/api/tests/test_user_schemas_active.py |
Removes redundant/older schema active/auth computed field tests. |
userapp/api/tests/fake_data.py |
Extends test user payload factory to support username. |
userapp/api/routes/users.py |
Minor formatting-only change in the update route. |
userapp/api/routes/security.py |
Sets username when auto-creating users via OIDC. |
alembic/versions/47f8adc9859b_add_unix_username_field.py |
Migration to add/backfill username, add unique constraint, and recreate joined_projects view with username. |
Comments suppressed due to low confidence (1)
userapp/core/schemas/users.py:105
- PR description/tests expect that when creating an active user with
usernameomitted,usernamedefaults tonetid. Currently theUserPostvalidator only enforcesnetidpresence and never sets a default forusername, so/usersPOST will persistusername=NULLwhen omitted. Add validation/defaulting (e.g., setusername = netidwhenactiveis True andusernameis not provided) to match the intended behavior.
class UserPost(BaseModel):
model_config = ConfigDict(extra='ignore')
name: str
username: Optional[str] = Field(default=None)
email1: Optional[EmailStr] = Field(default=None)
email2: Optional[EmailStr] = Field(default=None)
netid: Optional[str] = Field(default=None)
netid_exp_datetime: Optional[datetime] = Field(default=None)
phone1: Optional[str] = Field(default=None)
phone2: Optional[str] = Field(default=None)
is_admin: Optional[bool] = Field(default=None)
active: Optional[bool] = Field(default=None)
unix_uid: Optional[int] = Field(default=None)
position: Optional[PositionEnum] = Field(default=None)
@field_serializer('position')
def serialize_position(self, position: PositionEnum) -> str:
return position.name if position is not None else None
@model_validator(mode="after")
def check_active_requires_netid(self):
if self.active and not self.netid:
raise ValueError("If active is True, netid must be provided.")
return self
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Adds
usernameto the user table and related user/project views and schemas.For active users,
netidis required, andusernamedefaults tonetidif omitted. Existing active users are backfilled the same way, with the exceptions found in the migration script.Resolves CHTC/chtc-user-ui#57