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 the ability to add/remove friends in UserProfileHeader #30467

Merged
merged 18 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ public void TestFriendScore()
var api = (DummyAPIAccess)API;

api.Friends.Clear();
api.Friends.Add(friend);
api.Friends.Add(CreateAPIRelationFromAPIUser(friend));
});

int playerNumber = 1;
Expand Down
4 changes: 2 additions & 2 deletions osu.Game.Tests/Visual/Online/TestSceneDashboardOverlay.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ private void load()
if (supportLevel > 3)
supportLevel = 0;

((DummyAPIAccess)API).Friends.Add(new APIUser
((DummyAPIAccess)API).Friends.Add(CreateAPIRelationFromAPIUser(new APIUser
{
Username = @"peppy",
Id = 2,
Colour = "99EB47",
CoverUrl = @"https://osu.ppy.sh/images/headers/profile-covers/c3.jpg",
IsSupporter = supportLevel > 0,
SupportLevel = supportLevel
});
}));
}
}

Expand Down
84 changes: 84 additions & 0 deletions osu.Game.Tests/Visual/Online/TestSceneUserProfileHeader.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,20 @@

using System;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
using NUnit.Framework;
using osu.Framework.Allocation;
using osu.Framework.Graphics;
using osu.Framework.Testing;
using osu.Game.Configuration;
using osu.Game.Graphics.Containers;
using osu.Game.Online.API;
using osu.Game.Online.API.Requests;
using osu.Game.Online.API.Requests.Responses;
using osu.Game.Overlays;
using osu.Game.Overlays.Profile;
using osu.Game.Overlays.Profile.Header.Components;
using osu.Game.Rulesets.Osu;
using osu.Game.Users;

Expand All @@ -22,6 +27,10 @@ public partial class TestSceneUserProfileHeader : OsuTestScene
[Cached]
private readonly OverlayColourProvider colourProvider = new OverlayColourProvider(OverlayColourScheme.Green);

private DummyAPIAccess dummyAPI => (DummyAPIAccess)API;

private readonly ManualResetEventSlim requestLock = new ManualResetEventSlim();

[Resolved]
private OsuConfigManager configManager { get; set; } = null!;

Expand Down Expand Up @@ -400,5 +409,80 @@ public void TestManyTournamentBanners()
}
}, new OsuRuleset().RulesetInfo));
}

private APIUser nonFriend => new APIUser
{
Id = 727,
Username = "Whatever",
};

[Test]
public void TestAddFriend()
{
AddStep("Setup request", () =>
{
requestLock.Reset();

dummyAPI.HandleRequest = request =>
{
if (request is not FriendAddRequest req)
return false;

if (req.TargetId != nonFriend.OnlineID)
return false;

var apiRelation = CreateAPIRelationFromAPIUser(nonFriend);

Task.Run(() =>
{
requestLock.Wait(3000);
dummyAPI.Friends.Add(apiRelation);
req.TriggerSuccess(apiRelation);
});

return true;
};
});
AddStep("clear friend list", () => dummyAPI.Friends.Clear());
AddStep("Show non-friend user", () => header.User.Value = new UserProfileData(nonFriend, new OsuRuleset().RulesetInfo));
AddStep("Click followers button", () => this.ChildrenOfType<FollowersButton>().First().TriggerClick());
AddStep("Complete request", () => requestLock.Set());
AddUntilStep("Friend added", () => API.Friends.Any(f => f.TargetID == nonFriend.OnlineID));
}

[Test]
public void TestAddFriendNonMutual()
{
AddStep("Setup request", () =>
{
requestLock.Reset();

dummyAPI.HandleRequest = request =>
{
if (request is not FriendAddRequest req)
return false;

if (req.TargetId != nonFriend.OnlineID)
return false;

var apiRelation = CreateAPIRelationFromAPIUser(nonFriend);
apiRelation.Mutual = false;

Task.Run(() =>
{
requestLock.Wait(3000);
dummyAPI.Friends.Add(apiRelation);
req.TriggerSuccess(apiRelation);
});

return true;
};
});
AddStep("clear friend list", () => dummyAPI.Friends.Clear());
AddStep("Show non-friend user", () => header.User.Value = new UserProfileData(nonFriend, new OsuRuleset().RulesetInfo));
AddStep("Click followers button", () => this.ChildrenOfType<FollowersButton>().First().TriggerClick());
AddStep("Complete request", () => requestLock.Set());
AddUntilStep("Friend added", () => API.Friends.Any(f => f.TargetID == nonFriend.OnlineID));
}
}
}
34 changes: 19 additions & 15 deletions osu.Game/Online/API/APIAccess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ public partial class APIAccess : Component, IAPIProvider
private string password;

public IBindable<APIUser> LocalUser => localUser;
public IBindableList<APIUser> Friends => friends;
public IBindableList<APIRelation> Friends => friends;
public IBindable<UserActivity> Activity => activity;
public IBindable<UserStatistics> Statistics => statistics;

Expand All @@ -67,7 +67,7 @@ public partial class APIAccess : Component, IAPIProvider

private Bindable<APIUser> localUser { get; } = new Bindable<APIUser>(createGuestUser());

private BindableList<APIUser> friends { get; } = new BindableList<APIUser>();
private BindableList<APIRelation> friends { get; } = new BindableList<APIRelation>();

private Bindable<UserActivity> activity { get; } = new Bindable<UserActivity>();

Expand Down Expand Up @@ -360,19 +360,7 @@ private void attemptConnect()
}
}

var friendsReq = new GetFriendsRequest();
friendsReq.Failure += _ => state.Value = APIState.Failing;
friendsReq.Success += res =>
{
friends.Clear();
friends.AddRange(res);
};

if (!handleRequest(friendsReq))
{
state.Value = APIState.Failing;
return;
}
UpdateLocalFriends();

// The Success callback event is fired on the main thread, so we should wait for that to run before proceeding.
// Without this, we will end up circulating this Connecting loop multiple times and queueing up many web requests
Expand Down Expand Up @@ -624,6 +612,22 @@ public void UpdateStatistics(UserStatistics newStatistics)
localUser.Value.Statistics = newStatistics;
}

public void UpdateLocalFriends()
{
if (!IsLoggedIn)
return;

var friendsReq = new GetFriendsRequest();
friendsReq.Failure += _ => state.Value = APIState.Failing;
friendsReq.Success += res =>
{
friends.Clear();
friends.AddRange(res);
};

Queue(friendsReq);
}

private static APIUser createGuestUser() => new GuestUser();

private void setLocalUser(APIUser user) => Scheduler.Add(() =>
Expand Down
8 changes: 6 additions & 2 deletions osu.Game/Online/API/DummyAPIAccess.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public partial class DummyAPIAccess : Component, IAPIProvider
Id = DUMMY_USER_ID,
});

public BindableList<APIUser> Friends { get; } = new BindableList<APIUser>();
public BindableList<APIRelation> Friends { get; } = new BindableList<APIRelation>();

public Bindable<UserActivity> Activity { get; } = new Bindable<UserActivity>();

Expand Down Expand Up @@ -201,6 +201,10 @@ public void UpdateStatistics(UserStatistics newStatistics)
LocalUser.Value.Statistics = newStatistics;
}

public void UpdateLocalFriends()
{
}

public IHubClientConnector? GetHubConnector(string clientName, string endpoint, bool preferMessagePack) => null;

public IChatClient GetChatClient() => new TestChatClientConnector(this);
Expand All @@ -214,7 +218,7 @@ public void UpdateStatistics(UserStatistics newStatistics)
public void SetState(APIState newState) => state.Value = newState;

IBindable<APIUser> IAPIProvider.LocalUser => LocalUser;
IBindableList<APIUser> IAPIProvider.Friends => Friends;
IBindableList<APIRelation> IAPIProvider.Friends => Friends;
IBindable<UserActivity> IAPIProvider.Activity => Activity;
IBindable<UserStatistics?> IAPIProvider.Statistics => Statistics;

Expand Down
7 changes: 6 additions & 1 deletion osu.Game/Online/API/IAPIProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public interface IAPIProvider
/// <summary>
/// The user's friends.
/// </summary>
IBindableList<APIUser> Friends { get; }
IBindableList<APIRelation> Friends { get; }

/// <summary>
/// The current user's activity.
Expand Down Expand Up @@ -134,6 +134,11 @@ public interface IAPIProvider
/// </summary>
void UpdateStatistics(UserStatistics newStatistics);

/// <summary>
/// Update the friends status of the current user.
/// </summary>
void UpdateLocalFriends();
Comment on lines +137 to +140
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


/// <summary>
/// Schedule a callback to run on the update thread.
/// </summary>
Expand Down
31 changes: 31 additions & 0 deletions osu.Game/Online/API/Requests/FriendAddRequest.cs
bdach marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System.Net.Http;
using osu.Framework.IO.Network;
using osu.Game.Online.API.Requests.Responses;

namespace osu.Game.Online.API.Requests
{
public class FriendAddRequest : APIRequest<APIRelation>
{
public readonly int TargetId;

public FriendAddRequest(int targetId)
{
TargetId = targetId;
}

protected override WebRequest CreateWebRequest()
{
var req = base.CreateWebRequest();

req.Method = HttpMethod.Post;
req.AddParameter("target", TargetId.ToString(), RequestParameterType.Query);

return req;
}

protected override string Target => @"friends";
}
}
27 changes: 27 additions & 0 deletions osu.Game/Online/API/Requests/FriendDeleteRequest.cs
bdach marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using System.Net.Http;
using osu.Framework.IO.Network;

namespace osu.Game.Online.API.Requests
{
public class FriendDeleteRequest : APIRequest
{
public readonly int TargetId;

public FriendDeleteRequest(int targetId)
{
TargetId = targetId;
}

protected override WebRequest CreateWebRequest()
{
var req = base.CreateWebRequest();
req.Method = HttpMethod.Delete;
return req;
}

protected override string Target => $@"friends/{TargetId}";
}
}
2 changes: 1 addition & 1 deletion osu.Game/Online/API/Requests/GetFriendsRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@

namespace osu.Game.Online.API.Requests
{
public class GetFriendsRequest : APIRequest<List<APIUser>>
public class GetFriendsRequest : APIRequest<List<APIRelation>>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.)

{
protected override string Target => @"friends";
}
Expand Down
28 changes: 28 additions & 0 deletions osu.Game/Online/API/Requests/Responses/APIRelation.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
// See the LICENCE file in the repository root for full licence text.

using Newtonsoft.Json;

namespace osu.Game.Online.API.Requests.Responses
{
public class APIRelation
{
[JsonProperty("target_id")]
public int TargetID { get; set; }

[JsonProperty("relation_type")]
public RelationType RelationType { get; set; }
bdach marked this conversation as resolved.
Show resolved Hide resolved

[JsonProperty("mutual")]
public bool Mutual { get; set; }

[JsonProperty("target")]
public APIUser? TargetUser { get; set; }
Comment on lines +20 to +21
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

public enum RelationType
{
Friend,
Block,
}
}
4 changes: 2 additions & 2 deletions osu.Game/Overlays/Dashboard/Friends/FriendDisplay.cs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ public List<APIUser> Users
private Container itemsPlaceholder;
private LoadingLayer loading;

private readonly IBindableList<APIUser> apiFriends = new BindableList<APIUser>();
private readonly IBindableList<APIRelation> apiFriends = new BindableList<APIRelation>();

public FriendDisplay()
{
Expand Down Expand Up @@ -145,7 +145,7 @@ private void load(OverlayColourProvider colourProvider, IAPIProvider api)
controlBackground.Colour = colourProvider.Background5;

apiFriends.BindTo(api.Friends);
apiFriends.BindCollectionChanged((_, _) => Schedule(() => Users = apiFriends.ToList()), true);
apiFriends.BindCollectionChanged((_, _) => Schedule(() => Users = apiFriends.Select(f => f.TargetUser).ToList()), true);
}

protected override void LoadComplete()
Expand Down
Loading
Loading