Conversation
2f518bf to
fee68f9
Compare
| if not re.fullmatch(r'[^:,]*', username): | ||
| raise ValueError("Username cannot contain the characters ':' or ','.") | ||
| return username | ||
| class UserMin(BaseModel): |
There was a problem hiding this comment.
Any reason that you did not use UserGet here?
I would prefer if you used UserGet. I know it transfers more bytes then needed but I like that it keeps the complexity down.
|
|
||
| assert response.status_code == 400, f"Updating a group with a out of bounds unix_gid should return a 400 status code. Got {response.content} instead." | ||
|
|
||
| def test_add_group_with_point_of_contact(self, admin_client, user): |
There was a problem hiding this comment.
Can you add this same test for notes as well?
There was a problem hiding this comment.
I have one for notes at test_user_foreign_keys.py:76, but it's definitely in a weird place (refactor?)
There was a problem hiding this comment.
@fallow64 Oh great, yeah just copy paste to test_notes or if there is user specific stuff in it add a smaller test that is note specific.
| if not re.fullmatch(r'[^:,]*', username): | ||
| raise ValueError("Username cannot contain the characters ':' or ','.") | ||
| return username | ||
| class UserMin(BaseModel): |
There was a problem hiding this comment.
To reduce complexity can you use UserGet for this instead?
I am not worried about the increased transfer overhead.
This PR does the following:
activefield intoauth_netidandauth_usernameusernameandpasswordfieldsGroup.point_of_contact,Note.author,Project.staff1, andProject.staff2to user IDs in the databaseUserMinobject for those columns in the APIAlso:
with_db_error_handlingconsumes any HTTP response exceptionwith_db_error_handlingI'm slightly concerned about changing the behavior of
with_db_error_handlingincase anything wasn't meant to propagate to the user, but 🤷