Implement event check-in handler #93
Conversation
Merge from upstream
| orm_session = req.orm_session | ||
| reg_id = int(id) | ||
| reg_data = api.get_event_registration_by_id(orm_session, reg_id) | ||
| visit = EventParticipant( |
| orm_session = req.orm_session | ||
| reg_id = int(id) | ||
| reg_data = api.get_event_registration_by_id(orm_session, reg_id) | ||
| reg_data.visited = True |
There was a problem hiding this comment.
This will end in exception if reg_data is None. Please move the check above this line.
| reg_id = int(id) | ||
| reg_data = api.get_event_registration_by_id(orm_session, reg_id) | ||
| if not reg_data: | ||
| raise HTTPError(400, "Something wrong with registration") |
There was a problem hiding this comment.
I'd be more clear in error message:
'There is no registration record for id={id}'.format(id=reg_id)
|
This PR looks working, but now we need to prove this with tests. Add new data into DB inside of Please check asserts available at http://docs.cherrypy.org/en/latest/pkg/cherrypy.test.html#cherrypy.test.webtest.WebCase P.S. You'll need to enable |
| reg_id = int(id) | ||
| reg_data = api.get_event_registration_by_id(orm_session, reg_id) | ||
| if not reg_data: | ||
| raise HTTPError(400, |
There was a problem hiding this comment.
Why 400?
As I got the code right, reg_data will be None only in case we have no requested registration in database.
It will be a little bit more comprehensive if we return 404 Not Found error code in this case, won't it?
P.S. I do not insist on changing error code. I just want to understand why 400 Bad request?
P.P.S. @phiNumber I'm glad you joined this project ;)
There was a problem hiding this comment.
Actually, I don't really know if situation with 'Not Found' registration could possibly exist. If I am not mistaken, reg_id should be valid and inherited from layer above. Thus the ways reg_data is not valid are:
- someone changed/deleted DB record for this participant between time when 'register' button appeared and pressed (which, as I understand, will be the source of id in request)
- code exception
- [non-realistic] someone put his own self-created id into request
Please, correct me if I'm wrong.
P.S. @anxolerd me too;)
There was a problem hiding this comment.
Actually (3) is realistic is case of hacking attempts.
Still, I agree w/ @anxolerd, 404 is more verbose. On the other hand, I wouldn't expose the reason of bad request. Let's consider changing this in other dev iteration.
There was a problem hiding this comment.
We receive reg_id from url. Thus someone (with sufficient privileges) can easily misspell id in the URL (like 1111 instead of 111) and get a Bad request error. However the actual reason is Not found.
Implement event check-in handler. Close #72
|
I'm merging this. Tests to be written later as a part of #94 |
Implements #72