fix(api): include profile_id in profile creation response#144
Open
sehr-abrar wants to merge 9 commits into
Open
fix(api): include profile_id in profile creation response#144sehr-abrar wants to merge 9 commits into
sehr-abrar wants to merge 9 commits into
Conversation
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.
Summary
Profile creation returned the identifier only as
id, but clients expect aprofile_idfield to use in follow-up requests likeGET /profiles/{profile_id}.This PR adds
profile_idto theProfileResponseschema, mapped from the existingid. Theidfield is unchanged, so the change is additive and backward-compatible.Issue
Closes #78
Changes
api/schemas/profile.py: added aprofile_idcomputed field toProfileResponsethat returns the model's
id. A plainprofile_id: UUIDfield would not workbecause the schema uses
from_attributesand the ORM object has.id, not.profile_id, so the value is derived fromidinstead.tests/unit/test_profile_schema.py: new test module (5 tests) for the new fieldand backward compatibility.
Testing
make test-unit) (see pre-existing failures note)make test-integration) (N/A; the repo has no integration tests yet, so this collects 0 tests)make lint) (see pre-existing failures note)make typecheck)New tests in
tests/unit/test_profile_schema.pycover:profile_idis present inthe response, it equals both the source
idand the responseid,idis stillpresent for backward compatibility,
profile_idserializes to the same UUID stringas
idin JSON, andprofile_idis present when all optional fields areNone.Manual verification:
git checkout fix/78-profile-response-missing-profile-idpython -m pytest tests/unit/test_profile_schema.py -v(expect 5 passed)POST /profilesand confirm the JSON body now includesprofile_idalongsideid.Pre-existing failures (unrelated to this PR):
mainalready fails checks in modulesthis PR does not touch. On
main,make test-unitreports 55 failing tests (e.g.test_tech_detector.py,test_skill_extractor.py) andmake lintreports about 161ruff errors in those same modules. After this PR the counts do not increase (unit
failures 55 to 53, with the 5 new tests passing), and the two files I changed pass
ruff, black, and mypy individually. This PR does not try to fix the pre-existing failures.
Notes for Reviewers
idon purpose:frontend/src/components/ProfileForm.tsxreadsprofile.idfrom the create response, so removing or renaming it would break the frontend.
The change only adds
profile_id.ProfileResponseis shared by the create, get, and update endpoints inapi/routes/profiles.py, soprofile_idnow appears in all three responses.This is intended and safe since it only adds a field.