Fix issue #49 #50
Fix issue #49 #50
Conversation
tests/test_sqlalchemy_data_layer.py
Outdated
| response = client.get('/persons/' + str(person.person_id) + '/relationships/computers?include=computers', | ||
| content_type='application/vnd.api+json') | ||
| assert response.status_code == 200 | ||
| assert (json.loads(response.get_data()))['links']['related'] == '/persons/' + str(person.person_id) + '/computers' |
There was a problem hiding this comment.
This is not needed, and shall be removed.
| from sqlalchemy.orm import sessionmaker, relationship | ||
| from sqlalchemy.ext.declarative import declarative_base | ||
| from flask import Blueprint, make_response | ||
| from flask import Blueprint, make_response, json |
There was a problem hiding this comment.
Use flask.json since it handles bytes and unicode correctly.
| assert response.status_code == 200 | ||
|
|
||
|
|
||
| def test_issue_49(session, client, register_routes, person, person_2): |
There was a problem hiding this comment.
- Feel free to propose a different test function name.
sodre
force-pushed
the
i-49-related-link
branch
2 times, most recently
from
7b9c25d to
15b6e72
Compare
- Set person missing and default to None in Computer Schema - Fix typo, the SQLAlchemy model uses .person not .owner
- Assert data is empty The related view is for person_detail page which does not exist since the computer is not attached to a person yet during this test.
- The flask version handles unicode and bytestream by default
- Add Person 1 - Add Person 2 - Access person/1/relationships/computers - Verify links.related is valid. - Access person/2/relationships/computers - Verify links.related is valid. (Fails)
- Use marshmallow to render related_url - This depends on marshmallow-jsonapi being updated. Closes miLibris#49
|
This PR is ready for review. The coverage/coveralls fail should be ignored. It occurs because the proposed fix trades 10-covered-lines of resource.py in favor of one function call to marshmallow-jsonapi get_related_url. When the prosed fix is applied, two other unit tests begin to fail unless they are corrected before-hand. I split those fixes in PRs #51, and #52, feel free to merge them first or change to order of the commits. cc: @akira-dev, @jcampbell |
| tmp_obj = obj | ||
| for field in value[1:-1].split('.'): | ||
| tmp_obj = getattr(tmp_obj, field) | ||
| related_view_kwargs[key] = tmp_obj |
There was a problem hiding this comment.
This line is the original source of the bug.
It was rewriting the related_view_kwargs template with the value for the current page access. From this point on, related_view_kwargs will remain in a broken state until the application is restarted
For more details see - miLibris/flask-rest-jsonapi#49 - miLibris/flask-rest-jsonapi#50
|
Any chance of taking a look at this PR? The coveralls failure is because the proposed solution reduces the number of lines of code in favor of calling an existing function in marshmallow-jsonapi. |
|
pinging @akira-dev 🏓 |
|
Thx |
3 participants
Uh oh!
There was an error while loading. Please reload this page.