Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add delete ability #8

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Jaskaran2
Copy link

Have added the deleting ability, and also a add-email option in nav tab for better navigation

Screen.Recording.2022-10-15.at.4.56.38.PM.mov

@saroshfarhan
Copy link

saroshfarhan commented Oct 15, 2022

@Jaskaran2 just saw your PR and checked your commits, I had a doubt, how are you deleting entries from link_hits table? I didn't see any delete action for this table just the track_data table's entries are deleted. Or if I made a mistake in reading your commits can you please give me some pointers on that? Thanks

Also, shouldn't it redirect to the index page if no records are found, then we may not need to always click on the Add E-mail link on the navbar. Just a thought, you have done a really good job of deleting the feature and adding the navbar component!!!

@Jaskaran2
Copy link
Author

@Jaskaran2 just saw your PR and checked your commits, I had a doubt, how are you deleting entries from link_hits table? I didn't see any delete action for this table just the track_data table's entries are deleted. Or if I made a mistake in reading your commits can you please give me some pointers on that? Thanks

Also, shouldn't it redirect to the index page if no records are found, then we may not need to always click on the Add E-mail link on the navbar. Just a thought, you have done a really good job of deleting the feature and adding the navbar component!!!

HI @saroshfarhan , sorry I missed deleting enteries form from link_hits? I will fix that. And yes its a good suggestion what when there are no more enteries to show we should be redirected to index page. I will work on that too thank you for your inputs.

@saroshfarhan
Copy link

@Jaskaran2 look super sleek and crisp, I was overthinking deleting link_hits by executing complex SQL using inner joins and whatnot. I didn't think of this simple delete method. Thanks a lot for this commit I learnt a lot from this. Maybe you are not really new to flask I guess 😆
Keep up the good work!!!🚀

@Dilshan-H
Copy link
Owner

Related to #4

@Jaskaran2
Copy link
Author

@Jaskaran2 look super sleek and crisp, I was overthinking deleting link_hits by executing complex SQL using inner joins and whatnot. I didn't think of this simple delete method. Thanks a lot for this commit I learnt a lot from this. Maybe you are not really new to flask I guess 😆 Keep up the good work!!!🚀

Actually it's really my first go at flask, thanks a lot though😁. I was just thinking if there is a parent child relationship between TrackData and LinkHits so will that not delete link hits if parent gets deleted? I don't understand the code completely but the model looks to have a parent child relation to me.

Related to #4

Yes @Dilshan-H

@Jaskaran2
Copy link
Author

@Jaskaran2 look super sleek and crisp, I was overthinking deleting link_hits by executing complex SQL using inner joins and whatnot. I didn't think of this simple delete method. Thanks a lot for this commit I learnt a lot from this. Maybe you are not really new to flask I guess 😆 Keep up the good work!!!🚀

Thanks a lot 😄.

@Dilshan-H Dilshan-H added the hacktoberfest-accepted Accept for Hacktoberfest-22 label Oct 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Accept for Hacktoberfest-22
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants