Add the ability to add/remove friends in `UserProfileHeader` #30467

cdwcgt · 2024-11-01T06:46:38Z

close There's no way to add a friend in-game #30386

what this pr do:

add new model named APIRelation, it's also used in "blocking relationships"
modified some logic that is using the friend function due to model changes
add a new method named UpdateLocalFriends to IAPIProvider for update friends list manually, because it is currently impossible to get the friend list in real time through push method.
add loading layer to ProfileHeaderButton for display the online request status.

need to be mentioned:

follower count is store in FollowersButton when profile user loaded, and data changes are handled locally, because it is not possible to update user in profileOverlay after performing an operation
loading layer is bit different from web

bdach

superficial review, just the most obvious problems

bdach · 2024-11-01T10:39:34Z

osu.Game/Online/API/Requests/Responses/APIRelation.cs

+        [JsonProperty("target")]
+        public APIUser? TargetUser { get; set; }


Where is this field referenced from? It's not in https://github.com/nanaya/osu-web/blob/c029567dce89fd68446f56e40870c3cec01d7591/resources/js/interfaces/user-relation-json.ts#L4-L8

But he does exist, I think it's this is in
https://github.com/nanaya/osu-web/blob/fe21e207e8f8106ae024d8eab9108b8481ea1b5a/app/Http/Controllers/FriendsController.php#L57

api will return APIUser when request friends but not for block relation

osu.Game/Online/API/Requests/Responses/APIRelation.cs

osu.Game/Online/API/Requests/FriendDeleteRequest.cs

osu.Game/Online/API/Requests/FriendAddRequest.cs

bdach · 2024-11-01T10:48:55Z

osu.Game/Overlays/Profile/Header/Components/FollowersButton.cs

+                {
+                    followerCount += status.Value == FriendStatus.None ? 1 : -1;
+
+                    api.UpdateLocalFriends();


Does this really need to refetch friends every time? The request just succeeded, you can tell what happened, why do a refetch?

I guess it would make sense in the case of adding users, because you might not have the APIUser instance present or whatever, but isn't/can't that be in the add friend request or response? Or even couldn't the APIUser be taken from the ambient overlay?

I tried removing the refetch but things crash, something or other about groups not being populated in the response of this.

@ppy/team-client can I get a second pair of eyes on this to make sure you're ok with it?

osu.Game/Tests/Visual/OsuTestScene.cs

bdach · 2024-11-01T10:52:12Z

osu.Game/Online/API/IAPIProvider.cs

+        /// <summary>
+        /// Update the friends status of the current user.
+        /// </summary>
+        void UpdateLocalFriends();


Related to preceding comment - if the need to refetch users can be removed, this method probably doesn't need to exist. (I guess there is #13604, but it's (a) out of scope here, and (b) not even a given that a poll like the OP of that proposes is the right thing to do.)

Ok, I'll keep him for now until the results come in?

cdwcgt · 2024-11-01T11:30:08Z

I have fixed some of the issues that I think are now fixable, and now we cannot update friends list in real time
But to be honest, I don't have a good solution. Maybe it can be done like beatmap updates or achievement push?

bdach · 2024-11-01T11:38:31Z

now we cannot update friends list in real time

I'm not sure what you're trying to say? You haven't undone any of the UpdateLocalFriends() changes, so that should still work?

cdwcgt · 2024-11-01T12:17:13Z

UpdateLocalFriends() will only be called when adding or deleting friends or logging in.
and I haven't added any polling logic, so if you add friends on the web page, they won't be displayed in real time in the game.

bdach · 2024-11-01T12:32:06Z

As I said, that's out of scope of this PR. Let's focus on the adding/removing of friends here.

bdach

seems fine

bdach · 2024-11-04T13:46:01Z

osu.Game/Online/API/Requests/GetFriendsRequest.cs

@@ -6,7 +6,7 @@

 namespace osu.Game.Online.API.Requests
 {
-    public class GetFriendsRequest : APIRequest<List<APIUser>>
+    public class GetFriendsRequest : APIRequest<List<APIRelation>>


Note that this works because API-side the format of the response is changed and is dependent on x-api-version. This means that at least this single change to this single file must go out before the next release, or the friends displays will be broken. (Discovered in testing with #30499.)

peppy · 2024-11-05T05:55:05Z

I've adjusted the slightly because the transition of the button states was irking me.

Before:

After:

Now, rather than the full state change being delayed, only the mutual status is. I think this makes more sense.

@bdach please double check you're okay with this.

cdwcgt · 2024-11-05T06:12:25Z

for adding friend we can get a APIRelation response for mutual status but maybe need two different code path?

not sure whether to do this or not.

peppy · 2024-11-05T07:21:13Z

Hmm, I've tried that but Mutual doesn't seem to be set where I expect it to:

peppy@1fcdf67

cdwcgt · 2024-11-05T07:31:02Z

~~I tried capturing the network request and it looks like the api is returning the correct values, but addedRequest.Response looks like nothing?~~

ok I got it, response is

{
	"user_relation": {
		"target_id": 960,
		"relation_type": "friend",
		"mutual": true
	}
}

The actual data is in the inner layer

my bad

cdwcgt · 2024-11-05T07:42:33Z

add AddFriendResponse to handle it

bdach

seems fine. the issue with the wrong request shape is probably why i couldn't get this to work without refetching friends, but i don't have the mental energy to try it again

cdwcgt added 9 commits November 1, 2024 12:52

add API model and request

bf53833

Fix existing friend logic

69b5bd3

add ability to shou loading layer and set icon for followersButton

350e1d6

logic in FollowersButton

45cc830

add test

0b2f4fa

store follower count locally

29ba13f

simpily test

b682285

typo

3bd116c

add tooltips

9e4c382

pull-request-size bot added the size/L label Nov 1, 2024

cdwcgt changed the title ~~Add the ability to add friends to your profile~~ Add the ability to add friends in UserProfileHeader Nov 1, 2024

bdach added type:online area:overlay-user-profile labels Nov 1, 2024

bdach requested changes Nov 1, 2024

View reviewed changes

cdwcgt added 4 commits November 1, 2024 19:15

add StringEnumConverter for RelationType

729c7f1

rename FriendAddRequest to AddFriendRequest

21b1c79

rename FriendDeleteRequest to DeleteFriendRequest

fbe6077

remove CreateAPIRelationFromAPIUser

1a92e5a

pull-request-size bot added size/XL and removed size/L labels Nov 1, 2024

bdach changed the title ~~Add the ability to add friends in UserProfileHeader~~ Add the ability to add/remove friends in UserProfileHeader Nov 4, 2024

bdach added 2 commits November 4, 2024 12:02

Adjust colours to actually match web

cb83300

Remove weird method

e064965

bdach added the next release Pull requests which are almost there. We'll aim to get them in the next release, but no guarantees! label Nov 4, 2024

bdach previously approved these changes Nov 4, 2024

View reviewed changes

bdach reviewed Nov 4, 2024

View reviewed changes

peppy self-requested a review November 5, 2024 05:32

Update status and count immediately after friend request completes

0087270

peppy dismissed bdach’s stale review via 0087270 November 5, 2024 05:55

peppy previously approved these changes Nov 5, 2024

View reviewed changes

peppy self-requested a review November 5, 2024 06:52

Handle response to get accurate mutual state immediately

1fcdf67

peppy dismissed their stale review via 1fcdf67 November 5, 2024 07:21

add AddFriendResponse

c576fd8

cdwcgt force-pushed the friend-add branch from ccde490 to c576fd8 Compare November 5, 2024 07:56

peppy approved these changes Nov 5, 2024

View reviewed changes

bdach approved these changes Nov 5, 2024

View reviewed changes

bdach merged commit 2bd12e1 into ppy:master Nov 5, 2024
11 of 13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add the ability to add/remove friends in `UserProfileHeader` #30467

Add the ability to add/remove friends in `UserProfileHeader` #30467

cdwcgt commented Nov 1, 2024 •

edited

Loading

bdach left a comment

bdach Nov 1, 2024

cdwcgt Nov 1, 2024

bdach Nov 1, 2024 •

edited

Loading

bdach Nov 4, 2024

bdach Nov 1, 2024

cdwcgt Nov 1, 2024

cdwcgt commented Nov 1, 2024

bdach commented Nov 1, 2024 •

edited

Loading

cdwcgt commented Nov 1, 2024

bdach commented Nov 1, 2024

bdach left a comment

bdach Nov 4, 2024

peppy commented Nov 5, 2024 •

edited

Loading

cdwcgt commented Nov 5, 2024

peppy commented Nov 5, 2024

cdwcgt commented Nov 5, 2024 •

edited

Loading

cdwcgt commented Nov 5, 2024 •

edited

Loading

bdach left a comment

		[JsonProperty("target")]
		public APIUser? TargetUser { get; set; }

Add the ability to add/remove friends in UserProfileHeader #30467

Add the ability to add/remove friends in UserProfileHeader #30467

Conversation

cdwcgt commented Nov 1, 2024 • edited Loading

what this pr do:

need to be mentioned:

bdach left a comment

Choose a reason for hiding this comment

bdach Nov 1, 2024

Choose a reason for hiding this comment

cdwcgt Nov 1, 2024

Choose a reason for hiding this comment

bdach Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

bdach Nov 4, 2024

Choose a reason for hiding this comment

bdach Nov 1, 2024

Choose a reason for hiding this comment

cdwcgt Nov 1, 2024

Choose a reason for hiding this comment

cdwcgt commented Nov 1, 2024

bdach commented Nov 1, 2024 • edited Loading

cdwcgt commented Nov 1, 2024

bdach commented Nov 1, 2024

bdach left a comment

Choose a reason for hiding this comment

bdach Nov 4, 2024

Choose a reason for hiding this comment

peppy commented Nov 5, 2024 • edited Loading

cdwcgt commented Nov 5, 2024

peppy commented Nov 5, 2024

cdwcgt commented Nov 5, 2024 • edited Loading

cdwcgt commented Nov 5, 2024 • edited Loading

bdach left a comment

Choose a reason for hiding this comment

Add the ability to add/remove friends in `UserProfileHeader` #30467

Add the ability to add/remove friends in `UserProfileHeader` #30467

cdwcgt commented Nov 1, 2024 •

edited

Loading

bdach Nov 1, 2024 •

edited

Loading

bdach commented Nov 1, 2024 •

edited

Loading

peppy commented Nov 5, 2024 •

edited

Loading

cdwcgt commented Nov 5, 2024 •

edited

Loading

cdwcgt commented Nov 5, 2024 •

edited

Loading