Skip to content

Conversation

Justin-f-Reeves
Copy link
Contributor

fix erroneous code in create tag route handler

fix erroneous code in create tag route handler
@vercel
Copy link

vercel bot commented Jan 4, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
rest-apis-flask-python ✅ Ready (Inspect) Visit Preview Jan 4, 2023 at 0:57AM (UTC)

@jslvtr
Copy link
Contributor

jslvtr commented Jan 11, 2023

Thank you for your contribution! I noticed that the deleted code are these three lines:

if TagModel.query.filter(TagModel.store_id == store_id).first():
            abort(400, message="A tag with that name already exists in that store.")

These three lines are present in about 24 files in this repo, so I reckon if we're to make this change, we should make it in all files where the code is shown.

I'm also thinking about whether it would be better to not delete this if statement, and instead complete it so that it checks for the tag name also. That would mean changing the if statement to this:

if TagModel.query.filter(
    TagModel.store_id == store_id, TagModel.name == tag_data["name"]
).first():
    abort(400, message="A tag with that name already exists in that store.")

If we were to do this, we should also change the TagModel.name column so it's no longer unique:

-    name = db.Column(db.String(80), unique=True, nullable=False)
+    name = db.Column(db.String(80), unique=False, nullable=False)

I know that the videos show this code and then deletes it, but now I'm increasingly convinced this is a better final application.

@Justin-f-Reeves
Copy link
Contributor Author

Thanks Jose! - I'm closing this PR since I've submitted a new one from a different branch that addresses your suggestions above - See #109

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.

2 participants