refactor: change return error.respond() to raise #6780

Rits1272 · 2020-01-23T18:42:21Z

Checklist

I have read the Contribution & Best practices Guide and my PR follows them.
My branch is up-to-date with the Upstream development branch.
The unit tests pass locally with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

All the functions created/modified in this PR contain relevant docstrings.

codecov · 2020-01-23T18:48:27Z

Codecov Report

Merging #6780 into development will not change coverage.
The diff coverage is n/a.

@@             Coverage Diff              @@
##           development    #6780   +/-   ##
============================================
  Coverage        66.38%   66.38%           
============================================
  Files              313      313           
  Lines            15360    15360           
============================================
  Hits             10197    10197           
  Misses            5163     5163

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ab008c5...ab008c5. Read the comment docs.

iamareebjamal · 2020-01-23T18:51:41Z

app/instance.py

        }),
-        429,


Why are you changing this?

iamareebjamal · 2020-01-23T18:59:36Z

app/instance.py

@@ -269,5 +270,17 @@ def ratelimit_handler(error):
        })


+@app.errorhandler(HTTPException)


We don't want to handle any HTTPException, we want to handle our custom exceptions

houndci-bot · 2020-01-23T19:10:13Z

app/instance.py

@@ -6,6 +6,7 @@

 import sys
 from flask import Flask, json, make_response
+from werkzeug.exceptions import HTTPException


'werkzeug.exceptions.HTTPException' imported but unused

Rits1272 · 2020-01-23T19:12:24Z

Is that what we want @iamareebjamal?

iamareebjamal · 2020-01-23T19:12:36Z

app/instance.py

+        403,
+        {
+            'Content-Type': 'application/vnd.api+json'
+        })


Still wrong

Can you kindly please tell me @iamareebjamal what changes I need to do? I am registering error handler just as I did before for 429.

You can read the issue again

…server into development

app/api/schema/tracks.py

app/api/schema/sessions.py

app/api/orders.py

app/api/schema/sessions.py

Rits1272 · 2020-02-09T16:52:23Z

Please review my changes @iamareebjamal

iamareebjamal · 2020-02-09T17:00:27Z

It's still wrong. Read the issue again

Rits1272 · 2020-02-09T17:34:58Z

Please again check @iamareebjamal. Sorry for bothering you

iamareebjamal · 2020-02-09T17:35:58Z

Still wrong

iamareebjamal · 2020-02-18T09:42:57Z

Still wrong. Read my 2 comments carefully

Rits1272 · 2020-02-20T17:04:53Z

Please check this @iamareebjamal !

iamareebjamal · 2020-02-22T09:56:28Z

app/api/helpers/exceptions.py



-class UnprocessableEntity(JsonApiException):
+class UnprocessableEntity(JsonApiException, ErrorResponse):


Remove ErrorResponse

…server into development

Rits1272 · 2020-02-22T20:40:48Z

Please review again @iamareebjamal

Rits1272 · 2020-03-02T20:32:59Z

Kindly please check @iamareebjamal

iamareebjamal · 2020-02-23T03:59:46Z

app/api/helpers/exceptions.py

@@ -10,7 +11,7 @@ class UnprocessableEntity(JsonApiException):
    status = 422


-class ConflictException(JsonApiException):
+class ConflictException(JsonApiException, ErrorResponse):


Remove ErrorResponse

Rits1272 · 2020-03-06T17:39:22Z

I have made the required changes @iamareebjamal

houndci-bot · 2020-03-06T17:39:52Z

app/api/helpers/exceptions.py

@@ -1,4 +1,5 @@
 from flask_rest_jsonapi.exceptions import JsonApiException
+from app.api.helpers.errors import ErrorResponse


'app.api.helpers.errors.ErrorResponse' imported but unused

houndci-bot · 2020-05-06T18:18:33Z

app/api/helpers/permissions.py

-            return ForbiddenError(
-                {'source': ''}, 'Super admin access is required'
-            ).respond()
+            raise ForbiddenError({'source': ''}, 'Super admin access is required')


Black would make changes.

houndci-bot · 2020-05-06T18:18:34Z

app/api/helpers/permission_manager.py

@@ -27,15 +27,15 @@ def is_super_admin(view, view_args, view_kwargs, *args, **kwargs):
    """
    user = current_user
    if not user.is_super_admin:
-        return ForbiddenError({'source': ''}, 'Super admin access is required').respond()
+        raise ForbiddenError({'source': ''}, 'Super admin access is required')


Black would make changes.

niranjan94 · 2020-05-06T18:47:27Z

Here is an overview of what got changed by this pull request:

Issues
======
+ Solved 2
           

Clones removed
==============
+ app/api/event_invoices.py  -1
+ app/api/helpers/permission_manager.py  -2
+ app/api/orders.py  -1
+ app/api/role_invites.py  -2

See the complete overview on Codacy

iamareebjamal · 2020-05-06T18:54:00Z

Finally, after 48 commits for a small change

lgtm-com · 2020-05-06T19:17:55Z

This pull request fixes 6 alerts when merging ab008c5 into 40a1f28 - view on LGTM.com

fixed alerts:

6 for Illegal raise

Added error handler and changed return to raise statements

97eaf3d

auto-label bot added chore refactor labels Jan 23, 2020

iamareebjamal reviewed Jan 23, 2020

View reviewed changes

app/instance.py Outdated

}),

429,

Copy link

Member

iamareebjamal Jan 23, 2020

Choose a reason for hiding this comment

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

Why are you changing this?

Update

269ed50

iamareebjamal reviewed Jan 23, 2020

View reviewed changes

Update

f42938c

houndci-bot reviewed Jan 23, 2020

View reviewed changes

Registered custom errror handlers

446a75f

iamareebjamal reviewed Jan 23, 2020

View reviewed changes

Rits1272 added 3 commits February 2, 2020 23:12

Registered error handlers and changed return statements to raise

f20c638

Registered error handlers and changes return to raise statements

9c4e33e

Merge branch 'development' of https://github.com/fossasia/open-event-…

6909cac

…server into development

houndci-bot reviewed Feb 2, 2020

View reviewed changes

update

a15c45f

houndci-bot reviewed Feb 2, 2020

View reviewed changes

app/api/orders.py Outdated Show resolved Hide resolved

app/api/orders.py Outdated Show resolved Hide resolved

Update

fb97e74

houndci-bot reviewed Feb 2, 2020

View reviewed changes

app/api/schema/sessions.py Outdated Show resolved Hide resolved

Rits1272 added 4 commits February 2, 2020 23:30

Update

4a14f66

Merge branch 'development' into development

7df487e

Merge branch 'development' into development

5f41334

Merge branch 'development' into development

8c6a255

update

5a868ec

update

b7c39e6

Rits1272 added 2 commits February 20, 2020 22:31

Merge branch 'development' into development

ff9e91c

update

c7068ee

Merge branch 'development' into development

c71156f

iamareebjamal reviewed Feb 22, 2020

View reviewed changes

Rits1272 added 2 commits February 23, 2020 02:08

update

53638c1

Merge branch 'development' of https://github.com/Rits1272/open-event-…

50e43bf

…server into development

Merge branch 'development' into development

76c7260

iamareebjamal requested changes Mar 3, 2020

View reviewed changes

Rits1272 added 2 commits March 6, 2020 23:06

Merge branch 'development' into development

a8edad7

update

638c372

houndci-bot reviewed Mar 6, 2020

View reviewed changes

Rits1272 and others added 4 commits March 10, 2020 09:56

Merge branch 'development' into development

e76979d

update

23316c4

Merge branch 'development' into development

8f7fc28

finally fix the damn thing

66f1b85

houndci-bot reviewed May 6, 2020

View reviewed changes

iamareebjamal added 3 commits May 7, 2020 00:06

fix tests

8db9502

cleanup

e22f080

Merge branch 'development' into development

ab008c5

iamareebjamal changed the title ~~refactor: Registered error handlers and changed return statements to raise~~ refactor: change return error.respond() to raise May 6, 2020

iamareebjamal merged commit cea6494 into fossasia:development May 6, 2020

iamareebjamal mentioned this pull request Jul 6, 2020

Session POST request fails if track is submitted with it #7116

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: change return error.respond() to raise #6780

refactor: change return error.respond() to raise #6780

Rits1272 commented Jan 23, 2020

codecov bot commented Jan 23, 2020 •

edited

Loading

iamareebjamal Jan 23, 2020

iamareebjamal Jan 23, 2020

houndci-bot Jan 23, 2020

Rits1272 commented Jan 23, 2020

iamareebjamal Jan 23, 2020

Rits1272 Jan 23, 2020

iamareebjamal Jan 23, 2020

Rits1272 Jan 23, 2020

Rits1272 commented Feb 9, 2020

iamareebjamal commented Feb 9, 2020

Rits1272 commented Feb 9, 2020

iamareebjamal commented Feb 9, 2020

iamareebjamal commented Feb 18, 2020

Rits1272 commented Feb 20, 2020

iamareebjamal Feb 22, 2020

Rits1272 commented Feb 22, 2020

Rits1272 commented Mar 2, 2020

iamareebjamal Feb 23, 2020

Rits1272 commented Mar 6, 2020

houndci-bot Mar 6, 2020

houndci-bot May 6, 2020

houndci-bot May 6, 2020

niranjan94 commented May 6, 2020

iamareebjamal commented May 6, 2020

lgtm-com bot commented May 6, 2020

		@@ -269,5 +270,17 @@ def ratelimit_handler(error):
		})


		@app.errorhandler(HTTPException)



		class UnprocessableEntity(JsonApiException):
		class UnprocessableEntity(JsonApiException, ErrorResponse):

		@@ -1,4 +1,5 @@
		from flask_rest_jsonapi.exceptions import JsonApiException
		from app.api.helpers.errors import ErrorResponse

refactor: change return error.respond() to raise #6780

refactor: change return error.respond() to raise #6780

Conversation

Rits1272 commented Jan 23, 2020

Checklist

codecov bot commented Jan 23, 2020 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rits1272 commented Jan 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Rits1272 commented Feb 9, 2020

iamareebjamal commented Feb 9, 2020

Rits1272 commented Feb 9, 2020

iamareebjamal commented Feb 9, 2020

iamareebjamal commented Feb 18, 2020

Rits1272 commented Feb 20, 2020

Choose a reason for hiding this comment

Rits1272 commented Feb 22, 2020

Rits1272 commented Mar 2, 2020

Choose a reason for hiding this comment

Rits1272 commented Mar 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

niranjan94 commented May 6, 2020

iamareebjamal commented May 6, 2020

lgtm-com bot commented May 6, 2020

codecov bot commented Jan 23, 2020 •

edited

Loading