From a326adb6804645f68f06f56d5f9b5324495fc30f Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Tue, 17 Oct 2023 17:01:24 +0500 Subject: [PATCH 1/2] Fix Reminder serialization/deserialization (#166) Closes #124 Before we dive into this PR, let's explain a few rules of serialization and deserialization in System.Text.Json (often referred as STJ). The important rule of serialization is that fields are ***not*** serialized unless the serializer gets passed an instance of `JsonSerializerOptions` that explicitly tells it to include fields. However, properties are serialized by default. The important rule of ***de***serialization is that class members are only deserialized if: 1) In case of properties, they must have a setter 2) If they do not have a setter, the constructor must have an argument, required or optional, that will act as the setter for that property. Unfortunately, both of these rules were ignored in some commit that refactored reminders. This PR is here to fix that issue by: 1) Converting fields in `Reminder.cs` to properties 2) Adding an optional argument to the `MemberData` constructor --- src/Data/MemberData.cs | 6 +++++- src/Data/Reminder.cs | 6 +++--- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Data/MemberData.cs b/src/Data/MemberData.cs index c7ddc27..b63f8ad 100644 --- a/src/Data/MemberData.cs +++ b/src/Data/MemberData.cs @@ -5,10 +5,14 @@ namespace Octobot.Data; /// public sealed class MemberData { - public MemberData(ulong id, DateTimeOffset? bannedUntil = null) + public MemberData(ulong id, DateTimeOffset? bannedUntil = null, List? reminders = null) { Id = id; BannedUntil = bannedUntil; + if (reminders is not null) + { + Reminders = reminders; + } } public ulong Id { get; } diff --git a/src/Data/Reminder.cs b/src/Data/Reminder.cs index 828fadb..42144f9 100644 --- a/src/Data/Reminder.cs +++ b/src/Data/Reminder.cs @@ -2,7 +2,7 @@ namespace Octobot.Data; public struct Reminder { - public DateTimeOffset At; - public string Text; - public ulong Channel; + public DateTimeOffset At { get; init; } + public string Text { get; init; } + public ulong Channel { get; init; } } From 687883bbf8294e595210f4e81003c14d23e713f8 Mon Sep 17 00:00:00 2001 From: Octol1ttle Date: Tue, 17 Oct 2023 17:07:01 +0500 Subject: [PATCH 2/2] Use MemberData to determine a subscriber's role list (#165) Closes #163 Discord's API sucks a lot. You ask it for a member, but it won't give you a member. This is why this PR updates the `GetEventNotificationMentions` method used to determine what roles and users should get pinged for a scheduled event. Previously, the bot asked Discord to provide the member for each subscriber to determine whether or not they have the event notification role (to avoid pinging people personally when the role would already ping them). With this pull request, the bot uses MemberData, it's own member storage, for that purpose (if you're wondering why, refer to the first two sentences) --- src/Services/Update/ScheduledEventUpdateService.cs | 4 ++-- src/Services/UtilityService.cs | 10 +++++----- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Services/Update/ScheduledEventUpdateService.cs b/src/Services/Update/ScheduledEventUpdateService.cs index 20d23fa..792eef9 100644 --- a/src/Services/Update/ScheduledEventUpdateService.cs +++ b/src/Services/Update/ScheduledEventUpdateService.cs @@ -286,7 +286,7 @@ public sealed class ScheduledEventUpdateService : BackgroundService }; var contentResult = await _utility.GetEventNotificationMentions( - scheduledEvent, data.Settings, ct); + scheduledEvent, data, ct); if (!contentResult.IsDefined(out var content)) { return Result.FromError(contentResult); @@ -412,7 +412,7 @@ public sealed class ScheduledEventUpdateService : BackgroundService IGuildScheduledEvent scheduledEvent, GuildData data, CancellationToken ct) { var contentResult = await _utility.GetEventNotificationMentions( - scheduledEvent, data.Settings, ct); + scheduledEvent, data, ct); if (!contentResult.IsDefined(out var content)) { return Result.FromError(contentResult); diff --git a/src/Services/UtilityService.cs b/src/Services/UtilityService.cs index b144ca7..5bc06ea 100644 --- a/src/Services/UtilityService.cs +++ b/src/Services/UtilityService.cs @@ -160,16 +160,16 @@ public sealed class UtilityService : IHostedService /// /// The scheduled event whose subscribers will be mentioned. /// - /// The settings of the guild containing the scheduled event + /// The data of the guild containing the scheduled event. /// The cancellation token for this operation. /// A result containing the string which may or may not have succeeded. public async Task> GetEventNotificationMentions( - IGuildScheduledEvent scheduledEvent, JsonNode settings, CancellationToken ct = default) + IGuildScheduledEvent scheduledEvent, GuildData data, CancellationToken ct = default) { var builder = new StringBuilder(); - var role = GuildSettings.EventNotificationRole.Get(settings); + var role = GuildSettings.EventNotificationRole.Get(data.Settings); var subscribersResult = await _eventApi.GetGuildScheduledEventUsersAsync( - scheduledEvent.GuildID, scheduledEvent.ID, withMember: true, ct: ct); + scheduledEvent.GuildID, scheduledEvent.ID, ct: ct); if (!subscribersResult.IsDefined(out var subscribers)) { return Result.FromError(subscribersResult); @@ -181,7 +181,7 @@ public sealed class UtilityService : IHostedService } builder = subscribers.Where( - subscriber => subscriber.GuildMember.IsDefined(out var member) && !member.Roles.Contains(role)) + subscriber => !data.GetOrCreateMemberData(subscriber.User.ID).Roles.Contains(role.Value)) .Aggregate(builder, (current, subscriber) => current.Append($"{Mention.User(subscriber.User)} ")); return builder.ToString(); }