Add similar users component to feed page (#1320)

* Create modal entry component

* Add similar users modal

* Complete similar users modal

* Create UserSOcialNetwork component

* Rwemove duplicate file

* Changes for preliminary comments

* Changes for next comments

* Fix test

* Add prop API base

* Fix existing test

* Add initial tests for UserSocialNetwork component

* Make API calls async

With at least writing errors to the console

* Fix UserSocialNetwork tests

Some basic tests

* Add some tests to UserSocialNetwork

Make some of the code a bit more robust. Yay TDD!

* Move to api namespace - user/<user_name>/followers and /following

* Tweak user similarity score component

Was showing long string of numbers instead of adjusted to a scale of 1-10 and rounded.

* Moving follow features endpoints to API in front-end service

* Move to api namespace - follow_user and unfollow_user

* Add auth to /follow and /unfollow endpoint in front-end

* Add margin between similar users and brainzplayer

* Add some tests

* Update snapshot

* Add docstrings

* Change expected structure of response from /followers and /following

* Fix changes from code review

* Do not raise APIBadRequest on unfollowing not following user

* Resolve merge conflicts

* Add missing jest snapshot

Co-authored-by: Monkey Do <contact@monkeydo.digital>
Co-authored-by: Kartik Ohri <kartikohri13@gmail.com>

docs/dev/api.rst

@@ -86,6 +86,15 @@ Status API Endpoints
    :include-empty-docstring:
    :undoc-static:
 
+Social API Endpoints
+^^^^^^^^^^^^^^^^^^^^
+These apis allow to interact with social features of ListenBrainz.
+
+.. autoflask:: listenbrainz.webserver:create_app_rtfd()
+   :blueprints: social_api_v1
+   :include-empty-docstring:
+   :undoc-static:
+
 Rate limiting
 ^^^^^^^^^^^^^
 

listenbrainz/tests/integration/__init__.py

@@ -52,6 +52,7 @@ def setUp(self):
         IntegrationTestCase.setUp(self)
         TimescaleTestCase.setUp(self)
         self.user = db_user.get_or_create(1, 'testuserpleaseignore')
+        db_user.agree_to_gdpr(self.user['musicbrainz_id'])
         self.user2 = db_user.get_or_create(2, 'all_muppets_all_of_them')
 
     def tearDown(self):

listenbrainz/tests/integration/test_api.py

@@ -1,18 +1,24 @@
 import json
 import time
-from unittest.mock import patch
 
 import pytest
 from flask import url_for
 
 import listenbrainz.db.user as db_user
+import listenbrainz.db.user_relationship as db_user_relationship
 from listenbrainz import db
 from listenbrainz.tests.integration import ListenAPIIntegrationTestCase
 from listenbrainz.webserver.views.api_tools import is_valid_uuid
 
 
 class APITestCase(ListenAPIIntegrationTestCase):
 
+    def setUp(self):
+        super(APITestCase, self).setUp()
+        self.followed_user = db_user.get_or_create(3, 'followed_user')
+        self.follow_user_url = url_for("social_api_v1.follow_user", user_name=self.followed_user["musicbrainz_id"])
+        self.follow_user_headers = {'Authorization': 'Token {}'.format(self.user['auth_token'])}
+
     def test_get_listens_invalid_count(self):
         """If the count argument is negative, the API should raise HTTP 400"""
         url = url_for('api_v1.get_listens',
@@ -765,3 +771,69 @@ def test_delete_listen_invalid_keys(self):
         )
         self.assertEqual(
             response.json["error"], "invalid recording_msid: Recording MSID format invalid.")
+
+    def test_followers_returns_the_followers_of_a_user(self):
+        r = self.client.post(self.follow_user_url, headers=self.follow_user_headers)
+        self.assert200(r)
+
+        r = self.client.get(url_for("social_api_v1.get_followers", user_name=self.followed_user["musicbrainz_id"]))
+        self.assert200(r)
+        self.assertListEqual([self.user.musicbrainz_id], r.json['followers'])
+
+    def test_following_returns_the_people_who_follow_the_user(self):
+        r = self.client.post(self.follow_user_url, headers=self.follow_user_headers)
+        self.assert200(r)
+
+        r = self.client.get(url_for("social_api_v1.get_following", user_name=self.user["musicbrainz_id"]))
+        self.assert200(r)
+        self.assertListEqual(['followed_user'], r.json['following'])
+
+    def test_follow_user(self):
+        r = self.client.post(self.follow_user_url, headers=self.follow_user_headers)
+        self.assert200(r)
+        self.assertTrue(db_user_relationship.is_following_user(self.user.id, self.followed_user['id']))
+
+    def test_follow_user_requires_login(self):
+        r = self.client.post(self.follow_user_url)
+        self.assert401(r)
+
+    def test_following_a_nonexistent_user_errors_out(self):
+        r = self.client.post(url_for("social_api_v1.follow_user", user_name="user_doesnt_exist_lol"),
+                             headers=self.follow_user_headers)
+        self.assert404(r)
+
+    def test_following_yourself_errors_out(self):
+        r = self.client.post(url_for("social_api_v1.follow_user", user_name=self.user.musicbrainz_id),
+                             headers=self.follow_user_headers)
+        self.assert400(r)
+
+    def test_follow_user_twice_leads_to_error(self):
+        r = self.client.post(self.follow_user_url, headers=self.follow_user_headers)
+        self.assert200(r)
+        self.assertTrue(db_user_relationship.is_following_user(self.user.id, self.followed_user['id']))
+
+        # now, try to follow again, this time expecting a 400
+        r = self.client.post(self.follow_user_url, headers=self.follow_user_headers)
+        self.assert400(r)
+
+    def test_unfollow_user(self):
+        # first, follow the user
+        r = self.client.post(self.follow_user_url, headers=self.follow_user_headers)
+        self.assert200(r)
+        self.assertTrue(db_user_relationship.is_following_user(self.user.id, self.followed_user['id']))
+
+        # now, unfollow and check the db
+        r = self.client.post(url_for("social_api_v1.unfollow_user", user_name=self.followed_user["musicbrainz_id"]),
+                             headers=self.follow_user_headers)
+        self.assert200(r)
+        self.assertFalse(db_user_relationship.is_following_user(self.user.id, self.followed_user['id']))
+
+    def test_unfollow_not_following_user(self):
+        r = self.client.post(url_for("social_api_v1.unfollow_user", user_name=self.followed_user["musicbrainz_id"]),
+                             headers=self.follow_user_headers)
+        self.assert200(r)
+        self.assertFalse(db_user_relationship.is_following_user(self.user.id, self.followed_user['id']))
+
+    def test_unfollow_user_requires_login(self):
+        r = self.client.post(url_for("social_api_v1.unfollow_user", user_name=self.followed_user["musicbrainz_id"]))
+        self.assert401(r)

listenbrainz/webserver/__init__.py

@@ -285,3 +285,6 @@ def _register_blueprints(app):
 
     from listenbrainz.webserver.views.user_timeline_event_api import user_timeline_event_api_bp
     app.register_blueprint(user_timeline_event_api_bp, url_prefix=API_PREFIX)
+
+    from listenbrainz.webserver.views.social_api import social_api_bp
+    app.register_blueprint(social_api_bp, url_prefix=API_PREFIX)

listenbrainz/webserver/static/css/follow.less

@@ -51,12 +51,13 @@
   padding-bottom: 10px;
 }
 
-.follower-following-list {
+.follower-following-list, .similar-users-list {
   max-height: 250px;
   padding: 10px;
   overflow-y: scroll;
   border-radius: 2px;
   box-shadow: inset 0px 11px 8px -10px #ccc;
+  margin-bottom: 20px;
 
   > * {
     display: flex;

listenbrainz/webserver/static/css/main.less

@@ -185,7 +185,7 @@ h2.page-title { @media (max-width: @grid-float-breakpoint) { margin-top: 0px; }
       background-color: @orange;
     }
     &.purple {
-      background-color: @purple;
+      background-color: @blue;
     }
   }
 }

listenbrainz/webserver/static/js/src/APIService.ts

@@ -148,38 +148,69 @@ export default class APIService {
     return result.user_token;
   };
 
-  followUser = async (username: string): Promise<{ status: number }> => {
-    const response = await fetch(`/user/${username}/follow`, {
+  followUser = async (
+    userName: string,
+    userToken: string
+  ): Promise<{ status: number }> => {
+    if (!userName) {
+      throw new SyntaxError("Username missing");
+    }
+    if (!userToken) {
+      throw new SyntaxError("User token missing");
+    }
+    const response = await fetch(`${this.APIBaseURI}/user/${userName}/follow`, {
       method: "POST",
+      headers: {
+        Authorization: `Token ${userToken}`,
+      },
     });
     return { status: response.status };
   };
 
-  unfollowUser = async (username: string): Promise<{ status: number }> => {
-    const response = await fetch(`/user/${username}/unfollow`, {
-      method: "POST",
-    });
+  unfollowUser = async (
+    userName: string,
+    userToken: string
+  ): Promise<{ status: number }> => {
+    if (!userName) {
+      throw new SyntaxError("Username missing");
+    }
+    if (!userToken) {
+      throw new SyntaxError("User token missing");
+    }
+    const response = await fetch(
+      `${this.APIBaseURI}/user/${userName}/unfollow`,
+      {
+        method: "POST",
+        headers: {
+          Authorization: `Token ${userToken}`,
+        },
+      }
+    );
     return { status: response.status };
   };
 
-  getFollowersOfUser = async (username: string) => {
+  getFollowersOfUser = async (
+    username: string
+  ): Promise<{ followers: Array<string> }> => {
     if (!username) {
       throw new SyntaxError("Username missing");
     }
 
-    const url = `/user/${username}/followers`;
+    const url = `${this.APIBaseURI}/user/${username}/followers`;
     const response = await fetch(url);
     await this.checkStatus(response);
     const data = response.json();
     return data;
   };
 
-  getFollowingForUser = async (username: string) => {
+  getFollowingForUser = async (
+    username: string
+  ): Promise<{ following: Array<string> }> => {
     if (!username) {
       throw new SyntaxError("Username missing");
     }
 
-    const url = `/user/${username}/following`;
+    const url = `${this.APIBaseURI}/user/${username}/following`;
     const response = await fetch(url);
     await this.checkStatus(response);
     const data = response.json();
@@ -729,4 +760,20 @@ export default class APIService {
     await this.checkStatus(response);
     return response.status;
   };
+
+  getSimilarUsersForUser = async (
+    username: string
+  ): Promise<{
+    payload: Array<{ user_name: string; similarity: number }>;
+  }> => {
+    if (!username) {
+      throw new SyntaxError("Username missing");
+    }
+
+    const url = `${this.APIBaseURI}/user/${username}/similar-users`;
+    const response = await fetch(url);
+    await this.checkStatus(response);
+    const data = response.json();
+    return data;
+  };
 }

listenbrainz/webserver/static/js/src/FollowButton.tsx

@@ -34,6 +34,10 @@ type FollowButtonProps = {
   user: ListenBrainzUser;
   loggedInUser?: ListenBrainzUser;
   loggedInUserFollowsUser: boolean;
+  updateFollowingList?: (
+    user: ListenBrainzUser,
+    action: "follow" | "unfollow"
+  ) => void;
 };
 
 type FollowButtonState = {
@@ -83,25 +87,38 @@ class FollowButton extends React.Component<
   };
 
   followUser = () => {
-    const { user } = this.props;
-    this.APIService.followUser(user.name).then(({ status }) => {
-      if (status === 200) {
-        this.setState({ loggedInUserFollowsUser: true, justFollowed: true });
-      } else {
-        this.setState({ error: true });
+    const { user, loggedInUser, updateFollowingList } = this.props;
+    this.APIService.followUser(user.name, loggedInUser?.auth_token!).then(
+      ({ status }) => {
+        if (status === 200) {
+          this.setState({ loggedInUserFollowsUser: true, justFollowed: true });
+          if (updateFollowingList) {
+            updateFollowingList(user, "follow");
+          }
+        } else {
+          this.setState({ error: true });
+        }
       }
-    });
+    );
   };
 
   unfollowUser = () => {
-    const { user } = this.props;
-    this.APIService.unfollowUser(user.name).then(({ status }) => {
-      if (status === 200) {
-        this.setState({ loggedInUserFollowsUser: false, justFollowed: false });
-      } else {
-        this.setState({ error: true });
+    const { user, loggedInUser, updateFollowingList } = this.props;
+    this.APIService.unfollowUser(user.name, loggedInUser?.auth_token!).then(
+      ({ status }) => {
+        if (status === 200) {
+          this.setState({
+            loggedInUserFollowsUser: false,
+            justFollowed: false,
+          });
+          if (updateFollowingList) {
+            updateFollowingList(user, "unfollow");
+          }
+        } else {
+          this.setState({ error: true });
+        }
       }
-    });
+    );
   };
 
   getButtonDetails = (): {

listenbrainz/webserver/static/js/src/SimilarityScore.test.tsx

@@ -4,7 +4,7 @@ import { mount } from "enzyme";
 import SimilarityScore, { SimilarityScoreProps } from "./SimilarityScore";
 
 const props: SimilarityScoreProps = {
-  similarityScore: 0.2,
+  similarityScore: 0.239745792,
   user: { auth_token: "baz", name: "test" },
   type: "regular",
 };
@@ -30,15 +30,15 @@ describe("SimilarityScore", () => {
 
     /* sets class orange for score 0.5 */
     wrapper = mount<SimilarityScoreProps>(
-      <SimilarityScore {...{ ...props, similarityScore: 0.5 }} />
+      <SimilarityScore {...{ ...props, similarityScore: 0.57457 }} />
     );
     expect(wrapper.find(".progress").childAt(0).hasClass("orange")).toEqual(
       true
     );
 
     /* sets class purple for score 0.9 */
     wrapper = mount<SimilarityScoreProps>(
-      <SimilarityScore {...{ ...props, similarityScore: 0.9 }} />
+      <SimilarityScore {...{ ...props, similarityScore: 0.945792 }} />
     );
     expect(wrapper.find(".progress").childAt(0).hasClass("purple")).toEqual(
       true

listenbrainz/webserver/static/js/src/SimilarityScore.tsx

@@ -19,33 +19,41 @@ const getclassName = (similarityScore: number): string => {
 };
 
 const SimilarityScore = (props: SimilarityScoreProps) => {
-  const { similarityScore, user, type } = props;
+  const { user, type, similarityScore } = props;
+
+  // We transform the similarity score from a scale 0-1 to 0-10
+  const adjustedSimilarityScore = Number((similarityScore * 10).toFixed(1));
   const className = getclassName(similarityScore);
+  const percentage = adjustedSimilarityScore * 10;
 
   return (
-    <div className={`similarity-score ${type}`}>
+    <div
+      className={`similarity-score ${type}`}
+      title="Your similarity score with that user"
+    >
       <div
         className="progress"
-        aria-label="Similarity Score"
+        aria-label="Similarity score"
         role="progressbar"
         aria-valuemin={0}
         aria-valuemax={100}
-        aria-valuenow={similarityScore * 100}
+        aria-valuenow={percentage}
         tabIndex={0}
       >
         <div
           className={`progress-bar ${className}`}
           style={{
-            width: `${similarityScore * 100}%`,
+            width: `${percentage}%`,
           }}
         />
       </div>
       {type === "regular" ? (
         <p className="text-muted">
-          Your compatibility with {user?.name} is {similarityScore * 10}/10
+          Your compatibility with {user?.name} is {adjustedSimilarityScore}
+          /10
         </p>
       ) : (
-        <p className="small text-muted">{similarityScore * 10}/10</p>
+        <p className="small text-muted">{adjustedSimilarityScore}/10</p>
       )}
     </div>
   );