chore(refactor): Move all custom API errors to errors.py #7019
Conversation
app/api/tax.py
Outdated
| ForbiddenException, | ||
| MethodNotAllowed, | ||
| ConflictException, | ||
| MethodNotAllowed, |
There was a problem hiding this comment.
continuation line unaligned for hanging indent
app/api/tax.py
Outdated
| ConflictException, | ||
| ForbiddenException, | ||
| MethodNotAllowed, | ||
| ConflictException, |
There was a problem hiding this comment.
Black would make changes.
trailing whitespace
| # "ticket sales-starts-at should be before event ends-at") | ||
|
|
||
| # if 'event_ends_at' in data and data['sales_ends_at'] > data['event_ends_at']: | ||
| # raise UnprocessableEntity({'pointer': '/data/attributes/sales-ends-at'}, | ||
| # raise UnprocessableEntityError({'pointer': '/data/attributes/sales-ends-at'}, |
There was a problem hiding this comment.
line too long (91 > 90 characters)
| {'pointer': '/data/attributes/sales-ends-at'}, | ||
| "sales-ends-at should be after sales-starts-at", | ||
| ) | ||
|
|
||
| # if 'event_ends_at' in data and data['sales_starts_at'] > data['event_ends_at']: | ||
| # raise UnprocessableEntity({'pointer': '/data/attributes/sales-starts-at'}, | ||
| # raise UnprocessableEntityError({'pointer': '/data/attributes/sales-starts-at'}, |
There was a problem hiding this comment.
line too long (93 > 90 characters)
app/api/schema/sessions.py
Outdated
| @@ -5,8 +5,7 @@ | |||
| from marshmallow_jsonapi import fields | |||
| from marshmallow_jsonapi.flask import Relationship | |||
| from sqlalchemy.orm.exc import NoResultFound | |||
|
|
|||
| from app.api.helpers.exceptions import ForbiddenException, UnprocessableEntity | |||
| from app.api.helpers.errors import ForbiddenError,UnprocessableEntityError | |||
There was a problem hiding this comment.
Black would make changes.
missing whitespace after ','
app/api/role_invites.py
Outdated
| @@ -5,7 +5,8 @@ | |||
| from app.api.bootstrap import api | |||
| from app.api.helpers.db import save_to_db | |||
| from app.api.helpers.errors import NotFoundError | |||
| from app.api.helpers.exceptions import ForbiddenException, UnprocessableEntity | |||
| from app.api.helpers.errors import ForbiddenError,UnprocessableEntityError | |||
There was a problem hiding this comment.
Black would make changes.
missing whitespace after ','
app/api/orders.py
Outdated
| @@ -636,7 +637,7 @@ def before_get(self, args, kwargs): | |||
| if not has_access( | |||
| 'is_coorganizer', event_id=order.event_id, user_id=order.user_id | |||
| ): | |||
| raise ForbiddenException( | |||
| raiseForbiddenError( | |||
There was a problem hiding this comment.
undefined name 'raiseForbiddenError'
app/api/orders.py
Outdated
| @@ -586,7 +587,7 @@ def before_delete_object(self, order, view_kwargs): | |||
| :return: | |||
| """ | |||
| if not has_access('is_coorganizer', event_id=order.event.id): | |||
| raise ForbiddenException({'source': ''}, 'Access Forbidden') | |||
| raiseForbiddenError({'source': ''}, 'Access Forbidden') | |||
There was a problem hiding this comment.
undefined name 'raiseForbiddenError'
app/api/orders.py
Outdated
| @@ -470,7 +471,7 @@ def before_update_object(self, order, data, view_kwargs): | |||
| and data[element] != getattr(order, element, None) | |||
| and element not in get_updatable_fields() | |||
| ): | |||
| raise ForbiddenException( | |||
| raiseForbiddenError( | |||
There was a problem hiding this comment.
undefined name 'raiseForbiddenError'
app/api/orders.py
Outdated
| @@ -459,7 +460,7 @@ def before_update_object(self, order, data, view_kwargs): | |||
| and order.status == 'completed' | |||
| and data[element] != getattr(order, element, None) | |||
| ): | |||
| raise ForbiddenException( | |||
| raiseForbiddenError( | |||
There was a problem hiding this comment.
undefined name 'raiseForbiddenError'
app/api/orders.py
Outdated
| @@ -447,7 +448,7 @@ def before_update_object(self, order, data, view_kwargs): | |||
|
|
|||
| elif current_user.id == order.user_id: | |||
| if order.status != 'initializing' and order.status != 'pending': | |||
| raise ForbiddenException( | |||
| raiseForbiddenError( | |||
There was a problem hiding this comment.
undefined name 'raiseForbiddenError'
app/api/orders.py
Outdated
| {'pointer': 'data/status'}, | ||
| "You cannot update the status of a completed paid order", | ||
| ) | ||
| elif element == 'status' and order.status == 'cancelled': | ||
| # Since the tickets have been unlocked and we can't revert it. | ||
| raise ForbiddenException( | ||
| raiseForbiddenError( |
There was a problem hiding this comment.
undefined name 'raiseForbiddenError'
app/api/orders.py
Outdated
| @@ -432,13 +433,13 @@ def before_update_object(self, order, data, view_kwargs): | |||
| and order.status == 'completed' | |||
| ): | |||
| # Since we don't have a refund system. | |||
| raise ForbiddenException( | |||
| raiseForbiddenError( | |||
There was a problem hiding this comment.
undefined name 'raiseForbiddenError'
app/api/orders.py
Outdated
| @@ -422,7 +423,7 @@ def before_update_object(self, order, data, view_kwargs): | |||
| element | |||
| ] != getattr(order, element, None): | |||
| if element != 'status' and element != 'deleted_at': | |||
| raise ForbiddenException( | |||
| raiseForbiddenError( | |||
There was a problem hiding this comment.
undefined name 'raiseForbiddenError'
app/api/orders.py
Outdated
| @@ -407,7 +408,7 @@ def before_update_object(self, order, data, view_kwargs): | |||
| and data[element] != getattr(order, element, None) | |||
| and element not in get_updatable_fields() | |||
| ): | |||
| raise ForbiddenException( | |||
| raiseForbiddenError( | |||
There was a problem hiding this comment.
undefined name 'raiseForbiddenError'
app/api/orders.py
Outdated
| @@ -63,7 +64,7 @@ def check_event_user_ticket_holders(order, data, element): | |||
| if element in ['event', 'user'] and data[element] != str( | |||
| getattr(order, element, None).id | |||
| ): | |||
| raise ForbiddenException( | |||
| raiseForbiddenError( | |||
There was a problem hiding this comment.
undefined name 'raiseForbiddenError'
app/api/orders.py
Outdated
| ConflictException | ||
| ) | ||
| from app.api.helpers.errors import ( | ||
| ForbiddenError,UnprocessableEntityError |
| UnprocessableEntity, | ||
| ConflictException | ||
| ) | ||
| from app.api.helpers.errors import ( |
There was a problem hiding this comment.
'app.api.helpers.errors.ForbiddenError' imported but unused
app/api/helpers/utilities.py
Outdated
| from app.api.helpers.exceptions import UnprocessableEntity | ||
| from app.api.helpers.errors import ( | ||
| UnprocessableEntityError | ||
|
|
app/api/helpers/utilities.py
Outdated
| @@ -9,7 +9,10 @@ | |||
| from flask import current_app | |||
| from itsdangerous import Serializer | |||
|
|
|||
| from app.api.helpers.exceptions import UnprocessableEntity | |||
| from app.api.helpers.errors import ( | |||
Satyam52
changed the title
|
This pull request introduces 2 alerts when merging eb1a214 into 069d8f8 - view on LGTM.com new alerts:
|
app/api/access_codes.py
Outdated
| @@ -6,9 +6,12 @@ | |||
| from app.api.helpers.db import safe_query | |||
| from app.api.helpers.exceptions import ( | |||
| ConflictException, | |||
There was a problem hiding this comment.
Why the love with ConflictException? Why leave it?
|
Syntactically wrong as well and the tests are also failing |
app/api/feedbacks.py
Outdated
| @@ -4,7 +4,7 @@ | |||
|
|
|||
| from app.api.bootstrap import api | |||
| from app.api.helpers.db import safe_query | |||
| from app.api.helpers.exceptions import ForbiddenException, UnprocessableEntity | |||
| from app.api.helpers.errors import ForbiddenError,UnprocessableEntityError | |||
There was a problem hiding this comment.
Black would make changes.
missing whitespace after ','
| @@ -4,7 +4,7 @@ | |||
|
|
|||
| from app.api.bootstrap import api | |||
| from app.api.helpers.db import safe_query | |||
| from app.api.helpers.exceptions import UnprocessableEntity | |||
| from app.api.helpers.errors import UnprocessableEntityError | |||
There was a problem hiding this comment.
'app.api.helpers.errors.UnprocessableEntityError' imported but unused
app/api/event_copyright.py
Outdated
| @@ -3,7 +3,7 @@ | |||
|
|
|||
| from app.api.bootstrap import api | |||
| from app.api.helpers.db import safe_query | |||
| from app.api.helpers.exceptions import ForbiddenException, UnprocessableEntity | |||
| from app.api.helpers.errors import ForbiddenError,UnprocessableEntityError | |||
There was a problem hiding this comment.
Black would make changes.
missing whitespace after ','
app/api/discount_codes.py
Outdated
| @@ -498,7 +500,7 @@ def before_get(self, args, kwargs): | |||
| elif discount.used_for == 'event' and has_access('is_admin'): | |||
| self.schema = DiscountCodeSchemaEvent | |||
| else: | |||
| raise UnprocessableEntity({'source': ''}, "Please verify your permission") | |||
| raise UnprocessableEntityError({'source': ''}, "Please verify your permission") | |||
There was a problem hiding this comment.
line too long (91 > 90 characters)
app/api/discount_codes.py
Outdated
| @@ -465,7 +467,7 @@ def before_get(self, args, kwargs): | |||
| elif discount.used_for == 'event' and has_access('is_admin'): | |||
| self.schema = DiscountCodeSchemaEvent | |||
| else: | |||
| raise UnprocessableEntity({'source': ''}, "Please verify your permission") | |||
| raise UnprocessableEntityError({'source': ''}, "Please verify your permission") | |||
There was a problem hiding this comment.
line too long (91 > 90 characters)
app/api/discount_codes.py
Outdated
| @@ -372,10 +374,10 @@ def before_get_object(self, view_kwargs): | |||
| elif discount.used_for == 'event': | |||
| self.schema = DiscountCodeSchemaEvent | |||
| else: | |||
| raise UnprocessableEntity({'source': ''}, "Please verify your permission") | |||
| raise UnprocessableEntityError({'source': ''}, "Please verify your permission") | |||
There was a problem hiding this comment.
line too long (95 > 90 characters)
app/api/discount_codes.py
Outdated
| @@ -308,7 +310,7 @@ def before_get(self, args, kwargs): | |||
| elif discount.used_for == 'event': | |||
| self.schema = DiscountCodeSchemaEvent | |||
| else: | |||
| raise UnprocessableEntity({'source': ''}, "Please verify your permission") | |||
| raise UnprocessableEntityError({'source': ''}, "Please verify your permission") | |||
There was a problem hiding this comment.
line too long (95 > 90 characters)
| MethodNotAllowed, | ||
| UnprocessableEntity, | ||
| ) | ||
| from app.api.helpers.errors import ( |
app/api/access_codes.py
Outdated
| @@ -160,7 +163,7 @@ def before_get(self, args, kwargs): | |||
| raise ObjectNotFound({'parameter': '{id}'}, "Access Code: not found") | |||
|
|
|||
| if not has_access('is_coorganizer', event_id=access.event_id): | |||
| raise UnprocessableEntity({'source': ''}, "Please verify your permission") | |||
| raise UnprocessableEntityError({'source': ''}, "Please verify your permission") | |||
There was a problem hiding this comment.
line too long (95 > 90 characters)
app/api/access_codes.py
Outdated
| ) | ||
| from app.api.helpers.errors import ( | ||
| UnprocessableEntityError , |
| @@ -25,7 +25,7 @@ def test_reject_tickets_of_different_events(db): | |||
| db.session.commit() | |||
|
|
|||
| with pytest.raises( | |||
| UnprocessableEntity, match=r'All tickets must belong to same event. Found: .*' | |||
| UnprocessableEntityError, match=r'All tickets must belong to same event. Found: .*' | |||
There was a problem hiding this comment.
Black would make changes.
line too long (91 > 90 characters)
Codecov Report
@@ Coverage Diff @@
## development #7019 +/- ##
===============================================
- Coverage 60.55% 60.53% -0.03%
===============================================
Files 260 259 -1
Lines 12877 12868 -9
===============================================
- Hits 7798 7789 -9
Misses 5079 5079
Continue to review full report at Codecov.
|
app/api/access_codes.py
Outdated
| ConflictException, | ||
| ForbiddenException, | ||
| UnprocessableEntity, | ||
| ConflictException |
There was a problem hiding this comment.
I didn't find conflictException in other file(error.py).
There was a problem hiding this comment.
so I should remove all exceptions from exception.py and move to error.py.there are only two left ConflictException and MethodNotAllowed?
There was a problem hiding this comment.
@iamareebjamal Shall I move all exception from exception.py to error.py ???
| ForbiddenException, | ||
| MethodNotAllowed, | ||
| UnprocessableEntity, | ||
| MethodNotAllowed |
app/api/orders.py
Outdated
| from app.api.helpers.errors import ( | ||
| ForbiddenError, | ||
| UnprocessableEntityError, | ||
| ConflictException |
app/api/users.py
Outdated
| from app.api.helpers.errors import ( | ||
| ForbiddenError, | ||
| UnprocessableEntityError, | ||
| ConflictException |
app/api/tax.py
Outdated
| from app.api.bootstrap import api | ||
| from app.api.helpers.db import get_count, safe_query | ||
| from app.api.helpers.exceptions import ( | ||
| from app.api.helpers.errors import ( |
app/api/stripe_authorization.py
Outdated
| from app.api.helpers.errors import ( | ||
| ForbiddenError, | ||
| UnprocessableEntityError, | ||
| ConflictException |
app/api/role_invites.py
Outdated
| @@ -4,8 +4,7 @@ | |||
|
|
|||
| from app.api.bootstrap import api | |||
| from app.api.helpers.db import save_to_db | |||
| from app.api.helpers.errors import NotFoundError | |||
| from app.api.helpers.exceptions import ForbiddenException, UnprocessableEntity | |||
| from app.api.helpers.errors import ForbiddenError, UnprocessableEntityError, NotFoundError | |||
app/api/events.py
Outdated
| from app.api.helpers.errors import ( | ||
| ForbiddenError, | ||
| UnprocessableEntityError, | ||
| ConflictException |
app/api/discount_codes.py
Outdated
| ForbiddenException, | ||
| MethodNotAllowed, | ||
| UnprocessableEntity, | ||
| MethodNotAllowed |
app/api/attendees.py
Outdated
| from app.api.helpers.errors import ( | ||
| ForbiddenError, | ||
| UnprocessableEntityError, | ||
| ConflictException |
app/api/access_codes.py
Outdated
| from app.api.helpers.errors import ( | ||
| UnprocessableEntityError, | ||
| ForbiddenError, | ||
| ConflictException |
|
Remove exceptions.py and resolve conflicts |
|
There are still merge conflicts |
|
@iamareebjamal resolved merge conflicts. |
|
Great work but I don't think you have Anyway I have created a PR to reformat files after your safe_query refactor #7020, so once it is merged, please rebase your branch and resolve conflicts and run |
|
So, I should install pre-commit and then rebase with development branch and run black . before commiting and pushing to my branch ? |
|
Yup |
| @@ -25,7 +25,8 @@ def test_reject_tickets_of_different_events(db): | |||
| db.session.commit() | |||
|
|
|||
| with pytest.raises( | |||
| UnprocessableEntity, match=r'All tickets must belong to same event. Found: .*' | |||
| UnprocessableEntityError, | |||
| match=r'All tickets must belong to same event. Found: .*', | |||
| from app.api.helpers.db import safe_query_kwargs | ||
| from app.api.helpers.exceptions import ForbiddenException | ||
| from app.api.helpers.errors import ForbiddenError |
There was a problem hiding this comment.
redefinition of unused 'ForbiddenError' from line 4
| from app.api.helpers.db import safe_query_kwargs | ||
| from app.api.helpers.exceptions import UnprocessableEntity | ||
| from app.api.helpers.errors import UnprocessableEntityError |
There was a problem hiding this comment.
redefinition of unused 'UnprocessableEntityError' from line 4
app/api/orders.py
Outdated
| ) | ||
| from app.api.helpers.errors import BadRequestError | ||
| from app.api.helpers.exceptions import ( | ||
| from app.api.helpers.db import safe_query, save_to_db, safe_query_kwargs, safe_query_by_id |
| from app.api.helpers.db import safe_query_kwargs | ||
| from app.api.helpers.exceptions import ForbiddenException | ||
| from app.api.helpers.errors import ForbiddenError |
There was a problem hiding this comment.
redefinition of unused 'ForbiddenError' from line 4
| from app.api.helpers.db import safe_query, safe_query_kwargs | ||
| from app.api.helpers.exceptions import ForbiddenException, UnprocessableEntity | ||
| from app.api.helpers.errors import ForbiddenError, UnprocessableEntityError |
There was a problem hiding this comment.
redefinition of unused 'ForbiddenError' from line 6
redefinition of unused 'UnprocessableEntityError' from line 6
| from app.api.helpers.db import safe_query_kwargs | ||
| from app.api.helpers.exceptions import ForbiddenException | ||
| from app.api.helpers.errors import ForbiddenError |
There was a problem hiding this comment.
redefinition of unused 'ForbiddenError' from line 4
| @@ -70,29 +70,25 @@ | |||
|
|
|||
| def validate_event(user, modules, data): | |||
| if not user.can_create_event(): | |||
| raise ForbiddenException({'source': ''}, "Please verify your Email") | |||
| raise ForbiddenError({'source': ''}, "Please verify your Email") | |||
|
|
app/api/attendees.py
Outdated
| UnprocessableEntityError, | ||
| ConflictException, | ||
| ) | ||
| from app.api.helpers.db import safe_query, safe_query_kwargs |
| @@ -1,13 +1,13 @@ | |||
| from flask_rest_jsonapi import ResourceDetail, ResourceList, ResourceRelationship | |||
| from flask_rest_jsonapi.exceptions import ObjectNotFound | |||
| from sqlalchemy.orm.exc import NoResultFound | |||
|
|
|||
| from app.api.bootstrap import api | ||
|
|
| ConflictException, | ||
| ForbiddenException, | ||
| UnprocessableEntity, | ||
| from app.api.helpers.db import safe_query, safe_query_by_id, safe_query_kwargs, save_to_db |
iamareebjamal
changed the title
Fixes #7008
Short description of what this resolves:
removed duplicates in exception.py and the corresponding files which were importing exception from app/api/helpers/exception.py.
Checklist
developmentbranch.