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

Add missing default response (HTTP/204) for DELETE handlers #28

Merged
merged 1 commit into from
Sep 24, 2016

Conversation

khorolets
Copy link
Collaborator

@khorolets khorolets commented Sep 23, 2016

Closes #26: Add missing default response (HTTP/204) for DELETE handlers
Refactor response decorator in namespace of flasл_resplus_patched
Add response(code=204) to delete in teams/resources.py
Updated team delete test to expect 204 status instead of 200

I will provide constant for 204-code in one of future commit if you don't mind. Any of packages haven't what you want. We have to create our own constants.

@coveralls
Copy link

coveralls commented Sep 23, 2016

Coverage Status

Coverage increased (+0.01%) to 91.131% when pulling d399b15 on khorolets:default_response_for_delete into 1235db7 on frol:master.

@coveralls
Copy link

coveralls commented Sep 23, 2016

Coverage Status

Coverage increased (+0.01%) to 91.131% when pulling b9ce629 on khorolets:default_response_for_delete into 1235db7 on frol:master.

@coveralls
Copy link

coveralls commented Sep 23, 2016

Coverage Status

Coverage increased (+0.01%) to 91.131% when pulling da51295 on khorolets:default_response_for_delete into 1235db7 on frol:master.

@frol frol changed the title #26 Add missing default response (HTTP/204) for DELETE handlers Add missing default response (HTTP/204) for DELETE handlers Sep 23, 2016
@@ -111,6 +111,7 @@ def patch(self, args, team):
)
@api.permission_required(permissions.WriteAccessPermission())
@api.response(code=http_exceptions.Conflict.code)
@api.response(code=204)
Copy link
Owner

Choose a reason for hiding this comment

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

204 is still used as a hard-coding here. Later, I would like to replace the http_exceptions with this http module.

@khorolets khorolets force-pushed the default_response_for_delete branch 2 times, most recently from 324035b to 3860d2f Compare September 24, 2016 06:21
@coveralls
Copy link

coveralls commented Sep 24, 2016

Coverage Status

Coverage increased (+0.4%) to 91.565% when pulling 3860d2f on khorolets:default_response_for_delete into 1235db7 on frol:master.

@coveralls
Copy link

coveralls commented Sep 24, 2016

Coverage Status

Coverage increased (+0.02%) to 91.142% when pulling 3860d2f on khorolets:default_response_for_delete into 1235db7 on frol:master.

@@ -14,13 +14,14 @@
from app.extensions.api.parameters import PaginationParameters
from app.modules.users import permissions
from app.modules.users.models import User
from flask.ext.restplus_patched._http import HTTPStatus
Copy link
Owner

@frol frol Sep 24, 2016

Choose a reason for hiding this comment

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

This is a deprecated way of importing flask extentions. Please, just use from flask_restplus_patched... and also put it in the previous group of imports.

Add response(code=204) to delete in teams/resources.py
Updated team delete test to expect 204 status instead of 200
Add constant for status code
@coveralls
Copy link

coveralls commented Sep 24, 2016

Coverage Status

Coverage increased (+0.02%) to 91.142% when pulling 90ac8ee on khorolets:default_response_for_delete into 1235db7 on frol:master.


from app.extensions import db
from app.extensions.api import Namespace, abort, http_exceptions
from app.extensions.api.parameters import PaginationParameters
from app.modules.users import permissions
from app.modules.users.models import User


Copy link
Owner

Choose a reason for hiding this comment

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

There is no need in this extra space line, but I will merge and we just fix this later.

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.

3 participants