Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hearing post put #1

Open
wants to merge 17 commits into
base: hearing-post-put
Choose a base branch
from

Conversation

stephawe
Copy link

No description provided.

Copy link
Owner

@tuomas777 tuomas777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some minor comments, overall very nice work!!

@@ -80,6 +80,12 @@ def default_hearing(john_doe, contact_person):


@pytest.fixture()
def default_label():
label = Label.objects.create(id=1, label='The Label')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Setting an id explicitly here probably isn't a good idea as it might lead to collisions afterwards

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is indeed a terrible idea!

assert_hearing_equals(data, valid_hearing_json, john_smith_api_client.user)


# Test that a user can update a hearing
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't really seem to match the test? :)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops

assert data == {'status': 'User without organization cannot PUT hearings.'}


# Test that a user can update a hearing
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same as above

"servicemap_url": "url",
})
data['sections'][0]['title'] = 'First section'
data['sections'][1]['images'][0]['caption'] = 'New image caption'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this change actually verified somewhere?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is now :)

hearing = create_hearings(1)[0]
response = john_smith_api_client.post(endpoint, data=valid_hearing_json, format='json')
data = get_data_from_response(response, status_code=201)
data['sections'][0]['id'] = hearing.id
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be a bit better, if the id was actually from an existing section id from another hearing

return Response(serializer.data)
return response.Response(serializer.data)

def create(self, request, pk=None):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think create() will never get param pk

Stephane Weiss added 8 commits November 25, 2016 12:10
Add a function _create_or_update_sections
Use this function in both create and update
Only users from the same organization can update a hearing
Remove duplicated import Response
Expand Organization upon deserializing created/updated hearing
Return 'created_at' when deserializing created/updated hearing
Assign Organization while POSTing a hearing
Move the test_image into tests/utils.py
Add a function image_test_json which returns this image
Update the test_comment to use this function
@stephawe stephawe force-pushed the hearing-post-put branch 2 times, most recently from 2f74051 to b7bb684 Compare November 29, 2016 15:19
Stephane Weiss added 6 commits November 29, 2016 18:43
Allow to GET unpublished hearings if same organization
Add tests
Unpublished Hearing are now visible to users of the same organization
Function can now be used for filtering Hearings with hearing_lookup=''
Allow to update partially a Hearing using PATCH
Add test
tuomas777 pushed a commit that referenced this pull request Nov 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants