✨ Create person entity #10
Conversation
dankolbman
force-pushed
the
person-entity
branch
from
f017306 to
064284a
Compare
dankolbman
force-pushed
the
person-entity
branch
from
064284a to
1c64382
Compare
Implement post to create one, put to update one, get to read one get to read all and delete to delete one
Renamed existing tests to be person specific Update model tests to include create one, update one, find one, find all and delete one Update api tests to include post, put, get, and delete
tests/test_person_api.py
Outdated
| headers=self._api_headers()) | ||
|
|
||
| resp = json.loads(response.data.decode("utf-8")) | ||
| self.assertEqual(response.status_code, 404) |
There was a problem hiding this comment.
Also worth checking that the actual row was dropped in the database. It's possible that the api could hold the transaction open making object deleted only in the api's session.
dataservice/api/person/person.py
Outdated
|
|
||
|
|
||
| @person_api.route('/') | ||
| class NewPerson(Resource): |
There was a problem hiding this comment.
Any reason for the addition of this class rather than placing the 'post' on the Persons resource?
There was a problem hiding this comment.
This resource class pattern didn't make sense to me before but I think now I understand it a little bit better. I'm gonna go with Person and PersonList classes
dataservice/api/person/person.py
Outdated
| Update an existing person | ||
| """ | ||
| body = request.json | ||
| person = model.Person.query.filter_by(kf_id=person_id).first_or_404() |
There was a problem hiding this comment.
Person.query.get_or_404(id) is equivalent.
Could also use query.get() and catch on the orm.exc.NoResultFound to abort with a relevant error message.
There was a problem hiding this comment.
Looks like get and get_or_404 can only be used with primary keys
🗃 Add base mixin for the data model
Change NewPerson to PersonList resource class - PersonList has get and post Change ExistingPerson to Person resource class - Person has get, put, and delete
tests/test_person_api.py
Outdated
|
|
||
| return response | ||
|
|
||
| def _test_response_content(self, resp, status_code): |
There was a problem hiding this comment.
Good use case for paramatrized tests.
| def init_app(cls, app): | ||
| Config.init_app(app) | ||
|
|
||
| # email errors to the administrators |
There was a problem hiding this comment.
Not sure how emailing errors works, but is this necessary right now?
dankolbman
added
feature
Create common module for common code among resources Create new folder for person tests, move all person tests into it Separate out model code, resource code, serialization code for Person resources Refactor and replace Flask-Script with built in CLI Add support for api versioning Clean up Flask CLI commands Change api prefix from /api/v1 to /v1 Remove Flask-Script dependenct Replace relative imports with absolute imports
znatty22
force-pushed
the
person-entity
branch
from
a16f4d1 to
7af27b0
Compare
Create person entity for #2
PUTGETPATCHDELETE