Skip to content

Commit 0bf4b74

Browse files
HandyS11claude
andcommitted
Drop clan embed pinning and keep gated channels in spec order
The #claninfo anchor embeds are no longer pinned: the pin flag, the reconciler's pin step and the gateway pin operation are removed outright. The embeds still sit above the change feed because Discord orders messages by creation time and the reconciler already repairs declaration order. ChannelSpec.Order was declared but never applied, so a capability-gated channel created after the rest of its category (the clan pair appears only once a clan is detected) was appended at the bottom. The reconciler now ends every channel pass with EnsureChannelOrderAsync, which permutes the managed channels' existing position values into spec order — channels outside the list keep their place — and is a pure cache read when the order already matches, so the steady state issues no REST calls. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 0615394 commit 0bf4b74

9 files changed

Lines changed: 198 additions & 111 deletions

File tree

src/RustPlusBot.Features.Workspace/Gateway/DiscordWorkspaceGateway.cs

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -163,24 +163,43 @@ public async Task DeleteMessageAsync(ulong guildId,
163163
}
164164

165165
/// <inheritdoc />
166-
public async Task PinMessageAsync(ulong guildId,
167-
ulong channelId,
168-
ulong messageId,
166+
public async Task EnsureChannelOrderAsync(ulong guildId,
167+
ulong categoryId,
168+
IReadOnlyList<ulong> orderedChannelIds,
169169
CancellationToken cancellationToken)
170170
{
171-
var channel = client.GetGuild(guildId)?.GetTextChannel(channelId);
172-
if (channel is null)
171+
ArgumentNullException.ThrowIfNull(orderedChannelIds);
172+
var guild = client.GetGuild(guildId);
173+
if (guild is null)
173174
{
174175
return;
175176
}
176177

177-
if (await channel.GetMessageAsync(messageId).ConfigureAwait(false) is IUserMessage message)
178+
var live = orderedChannelIds
179+
.Select(id => guild.GetTextChannel(id))
180+
.Where(c => c is not null && c.CategoryId == categoryId)
181+
.ToList();
182+
if (live.Count < 2)
178183
{
179-
await message.PinAsync(new RequestOptions
184+
return;
185+
}
186+
187+
// Discord sorts a category's channels by position, ties by snowflake.
188+
var current = live.OrderBy(c => c.Position).ThenBy(c => c.Id).Select(c => c.Id);
189+
if (current.SequenceEqual(live.Select(c => c.Id)))
190+
{
191+
return;
192+
}
193+
194+
// Permute the channels' existing position values rather than assigning fresh ones, so every
195+
// channel outside the list keeps its place relative to the reordered block.
196+
var slots = live.Select(c => c.Position).Order().ToList();
197+
await guild.ReorderChannelsAsync(
198+
live.Select((c, i) => new ReorderChannelProperties(c.Id, slots[i])),
199+
new RequestOptions
180200
{
181201
CancelToken = cancellationToken
182202
}).ConfigureAwait(false);
183-
}
184203
}
185204

186205
/// <inheritdoc />

src/RustPlusBot.Features.Workspace/Gateway/IWorkspaceGateway.cs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -105,12 +105,17 @@ Task EditMessageAsync(ulong guildId,
105105
/// <param name="cancellationToken">Token to cancel the operation.</param>
106106
Task DeleteMessageAsync(ulong guildId, ulong channelId, ulong messageId, CancellationToken cancellationToken);
107107

108-
/// <summary>Pins a message. A no-op if it is already pinned or already gone.</summary>
108+
/// <summary>Puts the given channels of a category into the given on-screen order. A cache-read
109+
/// no-op when they already are; otherwise one bulk reorder call that permutes the channels'
110+
/// existing position values, so channels outside the list keep their place.</summary>
109111
/// <param name="guildId">The snowflake ID of the guild.</param>
110-
/// <param name="channelId">The snowflake ID of the channel containing the message.</param>
111-
/// <param name="messageId">The snowflake ID of the message to pin.</param>
112+
/// <param name="categoryId">The snowflake ID of the category the channels live under.</param>
113+
/// <param name="orderedChannelIds">The channel snowflakes in desired top-to-bottom order.</param>
112114
/// <param name="cancellationToken">Token to cancel the operation.</param>
113-
Task PinMessageAsync(ulong guildId, ulong channelId, ulong messageId, CancellationToken cancellationToken);
115+
Task EnsureChannelOrderAsync(ulong guildId,
116+
ulong categoryId,
117+
IReadOnlyList<ulong> orderedChannelIds,
118+
CancellationToken cancellationToken);
114119

115120
/// <summary>Deletes a channel by snowflake (no-op if already gone).</summary>
116121
/// <param name="guildId">The snowflake ID of the guild.</param>

src/RustPlusBot.Features.Workspace/Reconciler/WorkspaceReconciler.cs

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,19 @@ await backends.Gateway.DeleteChannelAsync(guildId, stale.DiscordChannelId, cance
257257
}
258258
}
259259

260+
// A capability-gated channel created after the rest of the category is appended at the
261+
// bottom by Discord; restore the declared order. The gateway only issues a reorder call
262+
// when the live order actually differs, so this is a cache read on the steady state.
263+
var ordered = specs.Where(s => result.ContainsKey(s.Key))
264+
.OrderBy(s => s.Order)
265+
.Select(s => result[s.Key])
266+
.ToList();
267+
if (ordered.Count > 1)
268+
{
269+
await backends.Gateway.EnsureChannelOrderAsync(guildId, categoryId, ordered, cancellationToken)
270+
.ConfigureAwait(false);
271+
}
272+
260273
return result;
261274
}
262275

@@ -334,7 +347,6 @@ await backends.Gateway.DeleteMessageAsync(guildId, channelId, staleId, cancellat
334347
}
335348

336349
ulong messageId;
337-
var newlyPosted = false;
338350
if (item.LiveId is { } liveId)
339351
{
340352
await backends.Gateway
@@ -347,25 +359,6 @@ await backends.Gateway
347359
messageId = await backends.Gateway
348360
.PostMessageAsync(guildId, channelId, item.Payload, cancellationToken)
349361
.ConfigureAwait(false);
350-
newlyPosted = true;
351-
}
352-
353-
// Pin only on first post: pinning is idempotent but costs an API call, and a
354-
// re-posted message (declaration-order repair) also lands here as newly posted.
355-
if (newlyPosted && item.Spec.Pinned)
356-
{
357-
try
358-
{
359-
await backends.Gateway.PinMessageAsync(guildId, channelId, messageId, cancellationToken)
360-
.ConfigureAwait(false);
361-
}
362-
#pragma warning disable CA1031 // Broad catch: an unpinned embed is still correct; never fail a reconcile over it.
363-
catch (Exception ex) when (!cancellationToken.IsCancellationRequested)
364-
#pragma warning restore CA1031
365-
{
366-
logger.LogWarning(ex, "Pinning message '{Key}' in guild {GuildId} failed.", item.Spec.Key,
367-
guildId);
368-
}
369362
}
370363

371364
await backends.Store.SaveMessageAsync(

src/RustPlusBot.Features.Workspace/Registry/MessageSpec.cs

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,4 @@ namespace RustPlusBot.Features.Workspace.Registry;
44
/// <param name="Scope">Global or per-server.</param>
55
/// <param name="Key">Stable key (persisted as MessageKey); also the renderer lookup key.</param>
66
/// <param name="ChannelKey">The <see cref="ChannelSpec.Key"/> of the channel it lives in.</param>
7-
/// <param name="Pinned">True to pin the message when it is first posted, so a channel that also
8-
/// carries transient messages keeps this one reachable from the pin bar.</param>
9-
internal sealed record MessageSpec(WorkspaceScope Scope, string Key, string ChannelKey, bool Pinned = false);
7+
internal sealed record MessageSpec(WorkspaceScope Scope, string Key, string ChannelKey);

src/RustPlusBot.Features.Workspace/Specs/ServerWorkspaceSpecProvider.cs

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,8 @@ public IEnumerable<MessageSpec> GetMessageSpecs() =>
3939
new(WorkspaceScope.PerServer, WorkspaceMessageKeys.ServerTeam, WorkspaceChannelKeys.ServerInfo),
4040
new(WorkspaceScope.PerServer, WorkspaceMessageKeys.ServerMap, WorkspaceChannelKeys.ServerMap),
4141

42-
// #claninfo also carries a transient change feed, so the anchored embeds are pinned to stay
43-
// reachable once the feed pushes them up.
44-
new(WorkspaceScope.PerServer, WorkspaceMessageKeys.ClanOverview, WorkspaceChannelKeys.ServerClanInfo, true),
45-
new(WorkspaceScope.PerServer, WorkspaceMessageKeys.ClanRoster, WorkspaceChannelKeys.ServerClanInfo, true),
46-
new(WorkspaceScope.PerServer, WorkspaceMessageKeys.ClanInvites, WorkspaceChannelKeys.ServerClanInfo, true),
42+
new(WorkspaceScope.PerServer, WorkspaceMessageKeys.ClanOverview, WorkspaceChannelKeys.ServerClanInfo),
43+
new(WorkspaceScope.PerServer, WorkspaceMessageKeys.ClanRoster, WorkspaceChannelKeys.ServerClanInfo),
44+
new(WorkspaceScope.PerServer, WorkspaceMessageKeys.ClanInvites, WorkspaceChannelKeys.ServerClanInfo),
4745
];
4846
}

tests/RustPlusBot.Features.Workspace.Tests/CapabilityGatedChannelTests.cs

Lines changed: 0 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -85,57 +85,6 @@ public async Task Leaves_ungated_channels_untouched()
8585
Assert.True(harness.Gateway.ChannelExists(1, info.DiscordChannelId));
8686
}
8787

88-
[Fact]
89-
public async Task Pins_a_pinned_message_only_when_it_is_newly_posted()
90-
{
91-
var harness = NewHarness()
92-
.WithChannel(WorkspaceScope.PerServer, "info", "channel.info.name")
93-
.WithMessage(WorkspaceScope.PerServer, "clan.overview", "info", "overview", pinned: true);
94-
var sut = harness.Build();
95-
96-
await sut.ReconcileServerAsync(1, ServerId);
97-
98-
var (pinnedChannelId, pinnedMessageId) = Assert.Single(harness.Gateway.PinnedMessages);
99-
var record = await harness.Store.GetMessageAsync(1, ServerId, "clan.overview");
100-
Assert.NotNull(record);
101-
Assert.Equal(record!.DiscordMessageId, pinnedMessageId);
102-
Assert.Equal(record.DiscordChannelId, pinnedChannelId);
103-
104-
// Second reconcile: the message is still live, so it is edited — and not pinned again.
105-
await sut.ReconcileServerAsync(1, ServerId);
106-
107-
Assert.Single(harness.Gateway.PinnedMessages);
108-
Assert.Equal(1, harness.Gateway.EditedMessages);
109-
}
110-
111-
[Fact]
112-
public async Task Does_not_pin_messages_that_are_not_marked_pinned()
113-
{
114-
var harness = NewHarness()
115-
.WithChannel(WorkspaceScope.PerServer, "info", "channel.info.name")
116-
.WithMessage(WorkspaceScope.PerServer, "server.info", "info", "info-text");
117-
var sut = harness.Build();
118-
119-
await sut.ReconcileServerAsync(1, ServerId);
120-
121-
Assert.Empty(harness.Gateway.PinnedMessages);
122-
}
123-
124-
[Fact]
125-
public async Task A_failed_pin_does_not_fail_the_reconcile()
126-
{
127-
var harness = NewHarness()
128-
.WithChannel(WorkspaceScope.PerServer, "info", "channel.info.name")
129-
.WithMessage(WorkspaceScope.PerServer, "clan.overview", "info", "overview", pinned: true);
130-
harness.Gateway.ThrowOnPin = true;
131-
var sut = harness.Build();
132-
133-
await sut.ReconcileServerAsync(1, ServerId);
134-
135-
Assert.Empty(harness.Gateway.PinnedMessages);
136-
Assert.NotNull(await harness.Store.GetMessageAsync(1, ServerId, "clan.overview"));
137-
}
138-
13988
private static ReconcilerHarness GatedHarness(bool available) => NewHarness()
14089
.WithChannel(WorkspaceScope.PerServer, "info", "channel.info.name")
14190
.WithChannel(WorkspaceScope.PerServer, "clanchat", "channel.teamchat.name", 1, "clan")

tests/RustPlusBot.Features.Workspace.Tests/Fakes/FakeWorkspaceGateway.cs

Lines changed: 53 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,20 @@ internal sealed class FakeWorkspaceGateway : IWorkspaceGateway
1111
private readonly ConcurrentDictionary<ulong, Channel> _channels = new();
1212
private readonly List<ulong> _deletedMessageIds = [];
1313
private readonly ConcurrentDictionary<ulong, Message> _messages = new();
14-
private readonly List<(ulong ChannelId, ulong MessageId)> _pinnedMessages = [];
1514
private readonly List<MessagePayload> _postedPayloads = [];
1615
private ulong _nextId = 1000;
16+
private int _nextPosition;
1717

1818
public IReadOnlyList<string> MissingPermissions { get; set; } = [];
1919

20-
/// <summary>When true, <see cref="PinMessageAsync"/> throws (pin-failure path).</summary>
21-
public bool ThrowOnPin { get; set; }
22-
2320
public int CreatedCategories { get; private set; }
2421
public int CreatedChannels { get; private set; }
2522
public int PostedMessages { get; private set; }
2623
public int EditedMessages { get; private set; }
24+
25+
/// <summary>How many <see cref="EnsureChannelOrderAsync"/> calls actually moved channels.</summary>
26+
public int ReorderCalls { get; private set; }
27+
2728
public IReadOnlyCollection<ulong> ChannelIds => [.. _channels.Keys];
2829
public IReadOnlyCollection<ulong> CategoryIds => [.. _categories.Keys];
2930

@@ -33,9 +34,6 @@ internal sealed class FakeWorkspaceGateway : IWorkspaceGateway
3334
/// <summary>Payloads posted via <see cref="PostMessageAsync"/>, in call order.</summary>
3435
public IReadOnlyList<MessagePayload> PostedPayloads => _postedPayloads;
3536

36-
/// <summary>Pairs pinned via <see cref="PinMessageAsync"/>, in call order.</summary>
37-
public IReadOnlyList<(ulong ChannelId, ulong MessageId)> PinnedMessages => _pinnedMessages;
38-
3937
public bool CategoryExists(ulong guildId, ulong categoryId) => _categories.ContainsKey(categoryId);
4038

4139
public Task<ulong?> FindCategoryAsync(ulong guildId, string name, CancellationToken cancellationToken)
@@ -72,7 +70,8 @@ public Task<ulong> CreateChannelAsync(ulong guildId,
7270
CancellationToken cancellationToken)
7371
{
7472
var id = NextId();
75-
_channels[id] = new Channel(id, categoryId, name, profile);
73+
// Mirrors Discord: a new channel is appended at the bottom of its category.
74+
_channels[id] = new Channel(id, categoryId, name, profile, _nextPosition++);
7675
CreatedChannels++;
7776
return Task.FromResult(id);
7877
}
@@ -84,7 +83,8 @@ public Task ApplyChannelSettingsAsync(ulong guildId,
8483
ChannelPermissionProfile profile,
8584
CancellationToken cancellationToken)
8685
{
87-
_channels[channelId] = new Channel(channelId, categoryId, name, profile);
86+
var position = _channels.TryGetValue(channelId, out var existing) ? existing.Position : _nextPosition++;
87+
_channels[channelId] = new Channel(channelId, categoryId, name, profile, position);
8888
return Task.CompletedTask;
8989
}
9090

@@ -129,14 +129,38 @@ public Task DeleteMessageAsync(ulong guildId, ulong channelId, ulong messageId,
129129
return Task.CompletedTask;
130130
}
131131

132-
public Task PinMessageAsync(ulong guildId, ulong channelId, ulong messageId, CancellationToken cancellationToken)
132+
public Task EnsureChannelOrderAsync(ulong guildId,
133+
ulong categoryId,
134+
IReadOnlyList<ulong> orderedChannelIds,
135+
CancellationToken cancellationToken)
133136
{
134-
if (ThrowOnPin)
137+
var live = orderedChannelIds
138+
.Select(id => _channels.TryGetValue(id, out var c) && c.CategoryId == categoryId ? c : null)
139+
.OfType<Channel>()
140+
.ToList();
141+
if (live.Count < 2)
135142
{
136-
throw new InvalidOperationException($"Pinning message {messageId} failed.");
143+
return Task.CompletedTask;
137144
}
138145

139-
_pinnedMessages.Add((channelId, messageId));
146+
var current = live.OrderBy(c => c.Position).ThenBy(c => c.Id).Select(c => c.Id);
147+
if (current.SequenceEqual(live.Select(c => c.Id)))
148+
{
149+
return Task.CompletedTask;
150+
}
151+
152+
// Same permutation semantics as the Discord gateway: reuse the channels' existing position
153+
// values so everything else keeps its place.
154+
var slots = live.Select(c => c.Position).Order().ToList();
155+
for (var i = 0; i < live.Count; i++)
156+
{
157+
_channels[live[i].Id] = live[i] with
158+
{
159+
Position = slots[i]
160+
};
161+
}
162+
163+
ReorderCalls++;
140164
return Task.CompletedTask;
141165
}
142166

@@ -154,6 +178,16 @@ public Task DeleteCategoryAsync(ulong guildId, ulong categoryId, CancellationTok
154178

155179
public IReadOnlyList<string> GetMissingBotPermissions(ulong guildId) => MissingPermissions;
156180

181+
/// <summary>The category's channel ids in on-screen order (position, then snowflake).</summary>
182+
/// <param name="categoryId">The category whose channels to list.</param>
183+
public IReadOnlyList<ulong> ChannelOrder(ulong categoryId) =>
184+
[
185+
.. _channels.Values.Where(c => c.CategoryId == categoryId)
186+
.OrderBy(c => c.Position)
187+
.ThenBy(c => c.Id)
188+
.Select(c => c.Id),
189+
];
190+
157191
private ulong NextId() => Interlocked.Increment(ref _nextId);
158192

159193
public void ExternallyDeleteChannel(ulong channelId) => _channels.TryRemove(channelId, out _);
@@ -167,7 +201,12 @@ public Task DeleteCategoryAsync(ulong guildId, ulong categoryId, CancellationTok
167201

168202
private sealed record Category(ulong Id, string Name);
169203

170-
private sealed record Channel(ulong Id, ulong CategoryId, string Name, ChannelPermissionProfile Profile);
204+
private sealed record Channel(
205+
ulong Id,
206+
ulong CategoryId,
207+
string Name,
208+
ChannelPermissionProfile Profile,
209+
int Position);
171210

172211
private sealed record Message(ulong Id, ulong ChannelId, MessagePayload Payload);
173212
}

tests/RustPlusBot.Features.Workspace.Tests/Reconciler/ReconcilerHarness.cs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,9 @@ public ReconcilerHarness WithChannel(WorkspaceScope scope,
3535
public ReconcilerHarness WithMessage(WorkspaceScope scope,
3636
string key,
3737
string channelKey,
38-
string text,
39-
bool pinned = false)
38+
string text)
4039
{
41-
_messageProviders.Add(new StubMessageProvider([new MessageSpec(scope, key, channelKey, pinned)]));
40+
_messageProviders.Add(new StubMessageProvider([new MessageSpec(scope, key, channelKey)]));
4241
_renderers.Add(new StubRenderer(key, text));
4342
return this;
4443
}
@@ -108,10 +107,9 @@ public ReconcilerBuilderReusing WithChannel(WorkspaceScope scope,
108107
public ReconcilerBuilderReusing WithMessage(WorkspaceScope scope,
109108
string key,
110109
string channelKey,
111-
string text,
112-
bool pinned = false)
110+
string text)
113111
{
114-
_messageProviders.Add(new ListMessageProvider([new MessageSpec(scope, key, channelKey, pinned)]));
112+
_messageProviders.Add(new ListMessageProvider([new MessageSpec(scope, key, channelKey)]));
115113
_renderers.Add(new ListMessageRenderer(key, text));
116114
return this;
117115
}

0 commit comments

Comments
 (0)