refactor: remove repetitive safe_query #7017

satya7289 · 2020-05-16T16:00:50Z

Short description of what this resolves:

change the all safe_query(db, Session, 'id', view_kwargs['session_id'], 'session_id') into safe_query_kwargs(Session, view_kwargs, 'session_id')

Changes proposed in this pull request:

Remove repetitive safe_query for session

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.

houndci-bot · 2020-05-16T16:01:27Z

app/api/tracks.py

@@ -1,7 +1,7 @@
 from flask_rest_jsonapi import ResourceDetail, ResourceList, ResourceRelationship

 from app.api.bootstrap import api
-from app.api.helpers.db import safe_query
+from app.api.helpers.db import safe_query, safe_query_kwargs


'app.api.helpers.db.safe_query' imported but unused

houndci-bot · 2020-05-16T16:01:27Z

app/api/session_types.py

@@ -1,7 +1,7 @@
 from flask_rest_jsonapi import ResourceDetail, ResourceList, ResourceRelationship

 from app.api.bootstrap import api
-from app.api.helpers.db import safe_query
+from app.api.helpers.db import safe_query, safe_query_kwargs


'app.api.helpers.db.safe_query' imported but unused

houndci-bot · 2020-05-16T16:01:28Z

app/api/microlocations.py

@@ -1,7 +1,7 @@
 from flask_rest_jsonapi import ResourceDetail, ResourceList, ResourceRelationship

 from app.api.bootstrap import api
-from app.api.helpers.db import safe_query
+from app.api.helpers.db import safe_query, safe_query_kwargs


'app.api.helpers.db.safe_query' imported but unused

houndci-bot · 2020-05-16T16:01:28Z

app/api/helpers/db.py

+        parameter_name,
+        # TODO(Areeb): Check that only admin can pass this parameter
+        request.args.get('get_trashed') != 'true',
+    )


no newline at end of file

houndci-bot · 2020-05-16T16:01:28Z

app/api/helpers/db.py

@@ -129,3 +129,21 @@ def get_new_slug(model, name):
        return slug
    else:
        return '{}-{}'.format(slug, uuid.uuid4().hex)
+
+def safe_query_kwargs(model, kwargs, parameter_name):


expected 2 blank lines, found 1

codecov · 2020-05-16T16:07:41Z

Codecov Report

Merging #7017 into development will decrease coverage by 0.00%.
The diff coverage is 20.46%.

@@               Coverage Diff               @@
##           development    #7017      +/-   ##
===============================================
- Coverage        60.55%   60.55%   -0.01%     
===============================================
  Files              260      260              
  Lines            12875    12877       +2     
===============================================
+ Hits              7797     7798       +1     
- Misses            5078     5079       +1

Impacted Files	Coverage Δ
app/api/orders.py	`27.05% <0.00%> (ø)`
app/api/events.py	`21.70% <2.77%> (ø)`
app/api/users.py	`29.82% <7.69%> (ø)`
app/api/discount_codes.py	`22.27% <8.33%> (ø)`
app/api/event_invoices.py	`48.88% <16.66%> (ø)`
app/api/sessions.py	`41.46% <16.66%> (ø)`
app/api/tickets.py	`46.66% <16.66%> (ø)`
app/api/event_sub_topics.py	`64.40% <20.00%> (ø)`
app/api/feedbacks.py	`50.64% <20.00%> (ø)`
app/api/notifications.py	`76.27% <20.00%> (ø)`
... and 26 more

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 069d8f8...df41a00. Read the comment docs.

iamareebjamal · 2020-05-16T16:11:02Z

app/api/helpers/db.py

@@ -129,3 +129,21 @@ def get_new_slug(model, name):
        return slug
    else:
        return '{}-{}'.format(slug, uuid.uuid4().hex)
+
+def safe_query_kwargs(model, kwargs, parameter_name):


Wrong place to add the method

Where should I place this method?

Near others

iamareebjamal · 2020-05-16T16:13:26Z

Fix hound and codacy issues

iamareebjamal · 2020-05-17T06:03:04Z

app/api/helpers/db.py

+                message eg 'event_id'
+    :return:
+    """
+    return safe_query_without_soft_deleted_entries(


You are replacing safe_query method, so you should use it instead, not safe_query_without_soft_deleted_entries

And move it above safe_query_without_soft_deleted_entries

iamareebjamal · 2020-05-17T07:40:31Z

Great job, but a lot of safe_query calls are still remaining

satya7289 · 2020-05-17T07:42:00Z

Great job, but a lot of safe_query calls are still remaining

Yes, do I cover all safe_query calls in this PR?

iamareebjamal · 2020-05-17T07:42:44Z

Yes, obviously the issue will only be resolved once all such calls are handled

satya7289 · 2020-05-17T07:43:18Z

Yes, obviously the issue will only be resolved once all such calls are handled

Ok

iamareebjamal · 2020-05-17T07:56:07Z

Also, calls like this are very similar:

safe_query(EventInvoice, 'identifier', view_kwargs['event_invoice_identifier'], 'event_invoice_identifier')

The only difference is identifier instead of id, so safe_query_kwargs can take a keyword argument, column with default value id, and then the call will change to safe_query(EventInvoice, view_kwargs, 'event_invoice_identifier', column='identifier')

satya7289 · 2020-05-17T09:03:54Z

Also, calls like this are very similar:
safe_query(EventInvoice, 'identifier', view_kwargs['event_invoice_identifier'], 'event_invoice_identifier')
The only difference is identifier instead of id, so safe_query_kwargs can take a keyword argument, column with default value id, and then the call will change to safe_query(EventInvoice, view_kwargs, 'event_invoice_identifier', column='identifier')

ok

iamareebjamal · 2020-05-17T14:50:38Z

app/api/orders.py

@@ -631,7 +632,7 @@ def before_get(self, args, kwargs):
            )
            kwargs['id'] = order.id
        elif kwargs.get('id'):
-            order = safe_query(Order, 'id', kwargs['id'], 'id')
+            order = safe_query_kwargs(Order, kwargs, 'id')


Use safe_query_by_id here

iamareebjamal · 2020-05-17T14:57:00Z

Great job. Final thing, please change the PR title to match semantic guidelines, see other merged PRs for reference

houndci-bot · 2020-05-17T15:07:54Z

app/api/orders.py

@@ -16,6 +16,8 @@
 from app.api.helpers.db import (
    safe_query,
    save_to_db,
+    safe_query_kwargs,
+    safe_query_by_id


Black would make changes.

niranjan94 · 2020-05-17T15:10:45Z

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

Clones removed
==============
+ app/api/event_topics.py  -1
+ app/api/event_copyright.py  -1
+ app/api/feedbacks.py  -2
+ app/api/custom_forms.py  -2
+ app/api/event_types.py  -1
+ app/api/tax.py  -2
+ app/api/tickets.py  -1
+ app/api/event_sub_topics.py  -1

See the complete overview on Codacy

iamareebjamal

Great Job!

satya7289 · 2020-05-17T15:14:01Z

Great Job!

Thank you so much.

added safe_query_kwargs

ec05946

houndci-bot reviewed May 16, 2020

View reviewed changes

iamareebjamal reviewed May 16, 2020

View reviewed changes

satya7289 added 2 commits May 16, 2020 21:49

Fix hound and codacy issues

9610384

move funtion

cdb5810

iamareebjamal reviewed May 17, 2020

View reviewed changes

move the function

ec7f088

replace repetative safe_query with safe_query_kwargs

90c51d0

add column option in safe_query_kwargs

cbb352f

iamareebjamal reviewed May 17, 2020

View reviewed changes

add safe_query_by_id in two places

df41a00

houndci-bot reviewed May 17, 2020

View reviewed changes

satya7289 changed the title ~~added safe_query_kwargs~~ refactor: Remove repetitive safe_query May 17, 2020

auto-label bot added chore refactor labels May 17, 2020

satya7289 changed the title ~~refactor: Remove repetitive safe_query~~ refactor: remove repetitive safe_query May 17, 2020

iamareebjamal approved these changes May 17, 2020

View reviewed changes

iamareebjamal merged commit a914592 into fossasia:development May 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: remove repetitive safe_query #7017

refactor: remove repetitive safe_query #7017

satya7289 commented May 16, 2020

houndci-bot May 16, 2020

houndci-bot May 16, 2020

houndci-bot May 16, 2020

houndci-bot May 16, 2020

houndci-bot May 16, 2020

codecov bot commented May 16, 2020 •

edited

Loading

iamareebjamal May 16, 2020

satya7289 May 16, 2020

iamareebjamal May 16, 2020

iamareebjamal commented May 16, 2020

iamareebjamal May 17, 2020 •

edited

Loading

iamareebjamal commented May 17, 2020

satya7289 commented May 17, 2020

iamareebjamal commented May 17, 2020

satya7289 commented May 17, 2020

iamareebjamal commented May 17, 2020

satya7289 commented May 17, 2020

iamareebjamal May 17, 2020

iamareebjamal commented May 17, 2020

houndci-bot May 17, 2020

niranjan94 commented May 17, 2020

iamareebjamal left a comment

satya7289 commented May 17, 2020

refactor: remove repetitive safe_query #7017

refactor: remove repetitive safe_query #7017

Conversation

satya7289 commented May 16, 2020

Short description of what this resolves:

Changes proposed in this pull request:

Checklist

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

Choose a reason for hiding this comment

codecov bot commented May 16, 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

iamareebjamal commented May 16, 2020

iamareebjamal May 17, 2020 • edited Loading

Choose a reason for hiding this comment

iamareebjamal commented May 17, 2020

satya7289 commented May 17, 2020

iamareebjamal commented May 17, 2020

satya7289 commented May 17, 2020

iamareebjamal commented May 17, 2020

satya7289 commented May 17, 2020

Choose a reason for hiding this comment

iamareebjamal commented May 17, 2020

Choose a reason for hiding this comment

niranjan94 commented May 17, 2020

iamareebjamal left a comment

Choose a reason for hiding this comment

satya7289 commented May 17, 2020

codecov bot commented May 16, 2020 •

edited

Loading

iamareebjamal May 17, 2020 •

edited

Loading