MVJ-112: area search attachments: hide user and attachment path from API response in public #819
Conversation
NC-jsAhonen
force-pushed
the
MVJ-112-api-does-not-give-file-url-in-response
branch
from
1a08571 to
7cde77f
Compare
NC-jsAhonen
force-pushed
the
MVJ-112-api-does-not-give-file-url-in-response
branch
from
7cde77f to
9863eb9
Compare
Now added. |
|
Looks good! It could be useful to add a comment, or add the list of fields in a named variable to indicate what is being done and why |
|
Created variables for the excluded fields and commented. They turned out to be useful in the unit tests as well. |
forms/tests/test_api.py
Outdated
|
|
||
|
|
||
| @pytest.mark.django_db | ||
| def test_area_search_attachment_post_public( |
There was a problem hiding this comment.
Should this test exist in the areasearch app?
forms/tests/test_api.py
Outdated
| ): | ||
| example_file = SimpleUploadedFile(name="example.txt", content=b"Lorem lipsum") | ||
| payload = { | ||
| "field": Field.objects.all().first().id, |
There was a problem hiding this comment.
I would recommend creating the field with a factory here, so that the test has control over what the field is.
There was a problem hiding this comment.
@henrinie-nc the fields are created by FieldFactory in the imported fixture basic_form. AFAIK there is no importance in this ID in this tests, the receiving model just requires it.
There was a problem hiding this comment.
I think a Field requires a Section, which in turn requires a Form, so might as well use a complete fixture to pull those.
There was a problem hiding this comment.
Missed that detail, thanks - then it really makes sense like it is
| response = admin_client.post(url, data=payload, content_type="application/json") | ||
|
|
||
| assert response.status_code == 201 | ||
| assert Attachment.objects.filter(answer=response.json()["id"]).exists() |
There was a problem hiding this comment.
Should this also test that it doesn't return any unwanted fields?
forms/tests/test_api.py
Outdated
| @@ -430,6 +432,97 @@ def test_attachment_delete( | |||
| assert os.path.isfile(file_path) is False | |||
|
|
|||
|
|
|||
| @pytest.mark.django_db | |||
| def test_attachment_post_public( | |||
There was a problem hiding this comment.
The test name could clarify what is it testing for, and enhanced with comments. It is not very clear what is being tested, the name indicates it is just to test that the endpoint works
forms/tests/test_api.py
Outdated
| ) | ||
| == 0 | ||
| ) | ||
|
|
There was a problem hiding this comment.
How about testing the areasearch endpoint, that it doesnt return the undesired fields?
…h field is being used
…add assert to test if attachment is linked to an area search
With these changes, the API response to the POST requests will hide sensitive information about the attachments.