refactor: Deduplicate get_new_slug method #7015

sanjana-302 · 2020-05-15T22:46:00Z

Fixes #6984

Short description of what this resolves:

Adds a generic get_new_slug method, prevents repetition of code.

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.

app/models/event_type.py

houndci-bot · 2020-05-15T22:46:05Z

app/models/event_topic.py

@@ -34,7 +17,7 @@ class EventTopic(SoftDeletionModel):

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
-        self.slug = get_new_slug(name=self.name)
+        self.slug = get_new_slug(name=self.name,EventTopic)


SyntaxError: positional argument follows keyword argument
missing whitespace after ','

houndci-bot · 2020-05-15T22:46:05Z

app/models/event_sub_topic.py

@@ -37,7 +19,7 @@ class EventSubTopic(SoftDeletionModel):

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
-        self.slug = get_new_slug(name=self.name)
+        self.slug = get_new_slug(name=self.name,EventSubTopic)


SyntaxError: positional argument follows keyword argument
missing whitespace after ','

houndci-bot · 2020-05-15T22:46:05Z

app/models/event_location.py

@@ -30,7 +13,7 @@ class EventLocation(db.Model):

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
-        self.slug = get_new_slug(name=self.name)
+        self.slug = get_new_slug(name=self.name,EventLocation)


SyntaxError: positional argument follows keyword argument
missing whitespace after ','

houndci-bot · 2020-05-15T22:46:05Z

app/api/helpers/db.py

@@ -115,3 +116,23 @@ def get_count(query):
    count_q = query.statement.with_only_columns([func.count()]).order_by(None)
    count = query.session.execute(count_q).scalar()
    return count
+
+
+def get_new_slug(name,model):


missing whitespace after ','

iamareebjamal · 2020-05-15T23:15:58Z

app/api/helpers/db.py

@@ -115,3 +116,23 @@ def get_count(query):
    count_q = query.statement.with_only_columns([func.count()]).order_by(None)
    count = query.session.execute(count_q).scalar()
    return count
+
+
+def get_new_slug(name, model):


Great job, but this should take (model, name), so it can be called like get_new_slug(EventLocation, name)

Okay.I will do it.

lgtm-com · 2020-05-15T23:34:25Z

This pull request introduces 4 alerts when merging a9d9f52 into 1831bab - view on LGTM.com

new alerts:

4 for Unused import

houndci-bot · 2020-05-15T23:58:50Z

app/models/event_type.py

-import uuid
-
-from app.api.helpers.db import get_count
+from app.api.helpers.db import get_count, get_new_slug


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

houndci-bot · 2020-05-15T23:58:50Z

app/models/event_topic.py

-import uuid
-
-from app.api.helpers.db import get_count
+from app.api.helpers.db import get_count, get_new_slug


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

houndci-bot · 2020-05-15T23:58:51Z

app/models/event_sub_topic.py

-import uuid
-
-from app.api.helpers.db import get_count
+from app.api.helpers.db import get_count, get_new_slug


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

houndci-bot · 2020-05-15T23:58:51Z

app/models/event_location.py

-import uuid
-
-from app.api.helpers.db import get_count
+from app.api.helpers.db import get_count,  get_new_slug


'app.api.helpers.db.get_count' imported but unused
Black would make changes.

codecov · 2020-05-16T00:05:56Z

Codecov Report

Merging #7015 into development will increase coverage by 0.73%.
The diff coverage is 100.00%.

@@               Coverage Diff               @@
##           development    #7015      +/-   ##
===============================================
+ Coverage        59.82%   60.55%   +0.73%     
===============================================
  Files              259      260       +1     
  Lines            12875    12875              
===============================================
+ Hits              7702     7797      +95     
+ Misses            5173     5078      -95

Impacted Files	Coverage Δ
app/api/helpers/db.py	`100.00% <100.00%> (+14.28%)`	⬆️
app/models/event_location.py	`91.66% <100.00%> (+2.19%)`	⬆️
app/models/event_sub_topic.py	`93.75% <100.00%> (-1.91%)`	⬇️
app/models/event_topic.py	`93.75% <100.00%> (+2.44%)`	⬆️
app/models/event_type.py	`92.85% <100.00%> (+2.38%)`	⬆️
app/api/helpers/query.py	`43.47% <0.00%> (-0.53%)`	⬇️
app/api/helpers/payment.py	`23.87% <0.00%> (-0.49%)`	⬇️
app/api/tax.py	`52.45% <0.00%> (ø)`
app/api/roles.py	`57.14% <0.00%> (ø)`
app/api/users.py	`29.82% <0.00%> (ø)`
... and 31 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 05fc6d2...95d3742. Read the comment docs.

iamareebjamal · 2020-05-16T05:19:34Z

app/models/event_location.py

@@ -30,7 +13,7 @@ class EventLocation(db.Model):

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
-        self.slug = get_new_slug(name=self.name)
+        self.slug = get_new_slug(EventLocation, name=self.name)


Suggested change

self.slug = get_new_slug(EventLocation, name=self.name)

self.slug = get_new_slug(EventLocation, self.name)

iamareebjamal · 2020-05-16T05:19:53Z

app/models/event_sub_topic.py

@@ -37,7 +19,7 @@ class EventSubTopic(SoftDeletionModel):

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
-        self.slug = get_new_slug(name=self.name)
+        self.slug = get_new_slug(EventSubTopic, name=self.name)


Same. Remove keyword argument

iamareebjamal · 2020-05-16T05:20:02Z

app/models/event_topic.py

@@ -34,7 +17,7 @@ class EventTopic(SoftDeletionModel):

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
-        self.slug = get_new_slug(name=self.name)
+        self.slug = get_new_slug(EventTopic, name=self.name)


iamareebjamal · 2020-05-16T05:20:11Z

app/models/event_type.py

@@ -32,7 +15,7 @@ class EventType(SoftDeletionModel):

    def __init__(self, **kwargs):
        super().__init__(**kwargs)
-        self.slug = get_new_slug(name=self.name)
+        self.slug = get_new_slug(EventType, name=self.name)


niranjan94 · 2020-05-16T07:11:46Z

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

Complexity decreasing per file
==============================
+ app/models/event_location.py  -1
+ app/models/event_type.py  -1
+ app/models/event_sub_topic.py  -1
+ app/models/event_topic.py  -1
         

Clones removed
==============
+ app/models/event_type.py  -1
+ app/models/event_sub_topic.py  -1
+ app/models/event_topic.py  -1

See the complete overview on Codacy

sanjana-302 added 2 commits May 16, 2020 03:14

deduplicate get_new_slug

6c25730

Deduplicate get_new_slug method-add docstring

756105d

houndci-bot reviewed May 15, 2020

View reviewed changes

sanjana-302 closed this May 15, 2020

sanjana-302 reopened this May 15, 2020

Deduplicate get_new_slug-syntax correction

a9d9f52

iamareebjamal reviewed May 15, 2020

View reviewed changes

Deduplicate get_new_slug-correct param

5dcfe64

houndci-bot reviewed May 15, 2020

View reviewed changes

Deduplicate get_new_slug-remove unused import

42a4a81

iamareebjamal reviewed May 16, 2020

View reviewed changes

Deduplicate get_new_slug-remove keyword argument

95d3742

iamareebjamal approved these changes May 16, 2020

View reviewed changes

iamareebjamal changed the title ~~ref: Deduplicate get_new_slug method~~ refactor: Deduplicate get_new_slug method May 16, 2020

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

iamareebjamal merged commit 069d8f8 into fossasia:development May 16, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Deduplicate get_new_slug method #7015

refactor: Deduplicate get_new_slug method #7015

sanjana-302 commented May 15, 2020

houndci-bot May 15, 2020

houndci-bot May 15, 2020

houndci-bot May 15, 2020

houndci-bot May 15, 2020

iamareebjamal May 15, 2020

sanjana-302 May 15, 2020

lgtm-com bot commented May 15, 2020

houndci-bot May 15, 2020

houndci-bot May 15, 2020

houndci-bot May 15, 2020

houndci-bot May 15, 2020

codecov bot commented May 16, 2020 •

edited

iamareebjamal May 16, 2020

iamareebjamal May 16, 2020

iamareebjamal May 16, 2020

iamareebjamal May 16, 2020

niranjan94 commented May 16, 2020

	self.slug = get_new_slug(EventLocation, name=self.name)
	self.slug = get_new_slug(EventLocation, self.name)

refactor: Deduplicate get_new_slug method #7015

refactor: Deduplicate get_new_slug method #7015

Conversation

sanjana-302 commented May 15, 2020

Short description of what this resolves:

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

Choose a reason for hiding this comment

lgtm-com bot commented May 15, 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

codecov bot commented May 16, 2020 • edited

Codecov Report

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

niranjan94 commented May 16, 2020

codecov bot commented May 16, 2020 •

edited