London | 26-SDC-March | Zobeir Rigi | Legacy-Code | Sprint 1 | Add ability to unfollow users - #253
Conversation
Zobeir-Rigi
added
📅 Sprint 1
SlideGauge
left a comment
SlideGauge
left a comment
There was a problem hiding this comment.
Overall good job, could you address a couple of notes from my side?
| def unfollow(follower: User, followee: User): | ||
| with db_cursor() as cur: | ||
| cur.execute( | ||
| "DELETE FROM follows WHERE follower = %(follower_id)s AND followee = %(followee_id)s", |
There was a problem hiding this comment.
Parameterized DELETE — correct and injection-safe, and consistent with follow(). Good instinct.
| app.add_url_rule("/bloom/<id_str>", methods=["GET"], view_func=get_bloom) | ||
| app.add_url_rule("/blooms/<profile_username>", view_func=user_blooms) | ||
| app.add_url_rule("/hashtag/<hashtag>", view_func=hashtag) | ||
| app.add_url_rule( |
There was a problem hiding this comment.
could you reformat it to match the surrounding coding style? (overall, even if you want to keep it multiline, the arguments should be indented additionally, like this:
app.add_url_rule(
"/unfollow/<profile_username>",
methods=["POST"],
view_func=do_unfollow)
| follower_id=follower.id, | ||
| followee_id=followee.id, | ||
| ), | ||
| ) No newline at end of file |
There was a problem hiding this comment.
could you add newline in the end of the file please?
| if (!username) return; | ||
|
|
||
| await apiService.followUser(username); | ||
| if (button.textContent === "Unfollow") { |
There was a problem hiding this comment.
Here you decide whether to follow or unfollow by checking button.textContent === "Unfollow". That works right now, but have a think: textContent is the text a human reads on the screen. What happens to this logic the day someone renames the button to "Stop following", or the app gets translated into another language?
You're already storing a piece of state on this button so the handler can read it later — see the data-username attribute a few lines up. Could the follow/unfollow state travel the same way, instead of being inferred from the label? Have a look at how data-username is set and then read back, and see if you can apply the same idea here.
SlideGauge
added
Reviewed
2 participants
Bug
Users could follow other users, but had no way to unfollow them.
Fix
Added unfollow functionality by displaying an Unfollow button for followed users and implementing the backend endpoint to remove follow relationships.