Skip to content

Fix help thread refresh bug #1345

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.

Sign up for GitHub

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

Open
digit01Wave wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix help thread refresh bug #1345

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
32 changes: 23 additions & 9 deletions ServerCore/Pages/Threads/PuzzleThreadComponent.razor
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@
{
<p>The more details you add here, the more helpful the responses you'll get will be!</p>
}
<textarea @bind="NewMessageText" class="form-control message-subtitle" rows="4" style="width:75%" placeholder="Type message here" autocomplete="off"></textarea>
<textarea @bind="NewMessageText" class="form-control message-subtitle" rows="4" style="width:75%" placeholder="Type message here" autocomplete="off" disabled="@isMutationRunning"></textarea>
Copy link
Copy Markdown
Contributor

@tabascq tabascq Jun 3, 2026

Choose a reason for hiding this comment

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

My understanding is that the C# code is running on the server since it's part of ServerCore, so updating based on these bools would require a message back to the client. Is a mutation such a long-running operation that it's worth sending a message back to the client saying to wait?

</div>
<div class="form-group message-subtitle">
<input type="button" value="Send" class="btn btn-default" @onclick="SendMessageAsync" />
<input type="button" value="Send" class="btn btn-default" @onclick="SendMessageAsync" disabled="@isMutationRunning" />
</div>
</div>
</div>
Expand All @@ -31,13 +31,13 @@
@if (ShouldShowClaimAction())
{
<br />
<button type="button" @onclick="ClaimLatestMessageAsync">Mark as read</button>
<button type="button" @onclick="ClaimLatestMessageAsync" disabled="@isMutationRunning">Mark as read</button>
}
else if (ShouldShowUnclaimAction())
{
<br />
<p>Question has already been marked as read by @LatestMessage.ClaimerName</p>
<button type="button" @onclick="UnclaimLatestMessageAsync">Undo mark as read</button>
<button type="button" @onclick="UnclaimLatestMessageAsync" disabled="@isMutationRunning">Undo mark as read</button>
}

@if (!string.IsNullOrEmpty(errorMessage))
Expand All @@ -62,10 +62,10 @@
{
<div id="TextAreaEdit_@message.ID">
<div class="form-group message-main-content">
<textarea class="form-control message-subtitle" style="width:100%" @bind="editTexts[message.ID]" rows="4" autocomplete="off"></textarea>
<textarea class="form-control message-subtitle" style="width:100%" @bind="editTexts[message.ID]" rows="4" autocomplete="off" disabled="@isMutationRunning"></textarea>
</div>
<div class="form-group">
<input type="button" value="Update" class="btn btn-default" @onclick="() => EditMessageAsync(message)" /> | <a role="button" class="message-subtitle" style="text-decoration:underline;color:@GetActionColor(message)" @onclick="() => CancelEdit(message)">Cancel</a>
<input type="button" value="Update" class="btn btn-default" @onclick="() => EditMessageAsync(message)" disabled="@isMutationRunning" /> | <a role="button" class="message-subtitle" style="text-decoration:underline;color:@GetActionColor(message)" @onclick="() => CancelEdit(message)">Cancel</a>
</div>
</div>
}
Expand All @@ -81,20 +81,34 @@
<span>
@if (IsAllowedToEditMessage(message))
{
<a role="button" class="message-subtitle" style="text-decoration:underline;color:@GetActionColor(message)" @onclick="() => StartEdit(message)">Edit</a>
@if (isMutationRunning)
{
<span class="message-subtitle" style="color:gray">Edit</span>
}
else
{
<a role="button" class="message-subtitle" style="text-decoration:underline;color:@GetActionColor(message)" @onclick="() => StartEdit(message)">Edit</a>
}
}
@if (IsAllowedToEditMessage(message) && IsAllowedToDeleteMessage(message))
{
<span>&nbsp;|&nbsp;</span>
}
@if (IsAllowedToDeleteMessage(message))
{
<a role="button" class="message-subtitle" style="color:@GetActionColor(message)" @onclick="() => DeleteMessageAsync(message)">Delete</a>
@if (isMutationRunning)
{
<span class="message-subtitle" style="color:gray">Delete</span>
}
else
{
<a role="button" class="message-subtitle" style="color:@GetActionColor(message)" @onclick="() => DeleteMessageAsync(message)">Delete</a>
}
}
</span>
</div>
</div>
}
</div>
}
</div>
</div>
16 changes: 15 additions & 1 deletion ServerCore/Pages/Threads/PuzzleThreadComponent.razor.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ public partial class PuzzleThreadComponent : ComponentBase, IDisposable

bool shouldRenderLocalTimes;

bool isMutationRunning;

ThreadMessageDTO LatestMessage => messages.FirstOrDefault();

protected override async Task OnInitializedAsync()
Expand Down Expand Up @@ -250,15 +252,26 @@ await RunMutationAsync(async () =>

async Task RunMutationAsync(Func<Task> mutation)
{
if (isMutationRunning)
Copy link
Copy Markdown
Contributor

@tabascq tabascq Jun 3, 2026

Choose a reason for hiding this comment

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

Since this isMutationRunning code lacks a critical section or equivalent, it only works 100% of the time if it can be guaranteed to run in a single threaded environment. Do we have any assurance of that?

Copy link
Copy Markdown
Contributor

@tabascq tabascq Jun 3, 2026

Choose a reason for hiding this comment

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

Would it be better to just wait for the previous mutation to complete and then perform the action?

{
errorMessage = "Please wait for the current action to finish.";
return;
}

try
{
isMutationRunning = true;
errorMessage = null;
await mutation();
Comment on lines 261 to 265
}
catch (UserOperationException ex)
{
errorMessage = ex.Message;
}
finally
{
isMutationRunning = false;
}
}

void UpsertMessage(ThreadMessageDTO message)
Expand All @@ -285,4 +298,5 @@ public void Dispose()
MessageListener.OnThreadMessage -= OnThreadMessageReceived;
}
}
}
}

64 changes: 37 additions & 27 deletions ServerCore/Pages/Threads/PuzzleThreadService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,19 @@ namespace ServerCore.Pages.Threads
{
public class PuzzleThreadService
{
private readonly PuzzleServerContext context;
private readonly IDbContextFactory<PuzzleServerContext> contextFactory;
private readonly IHubContext<ServerMessageHub> messageHub;

public PuzzleThreadService(PuzzleServerContext context, IHubContext<ServerMessageHub> messageHub)
public PuzzleThreadService(IDbContextFactory<PuzzleServerContext> contextFactory, IHubContext<ServerMessageHub> messageHub)
{
this.context = context;
this.contextFactory = contextFactory;
this.messageHub = messageHub;
}

public async Task<ThreadMessageDTO> SendMessageAsync(string threadId, int eventId, string subject, int puzzleId, int? teamId, int? playerId, bool isFromGameControl, int senderId, string text)
{
await using PuzzleServerContext context = await contextFactory.CreateDbContextAsync();

Event eventObj = await context.Events.Where(e => e.ID == eventId).FirstOrDefaultAsync();
if (eventObj == null)
{
Expand All @@ -48,7 +50,7 @@ public async Task<ThreadMessageDTO> SendMessageAsync(string threadId, int eventI
throw new InvalidOperationException("Puzzle not found.");
}

await ValidatePostPermissionAsync(eventObj, puzzle, isFromGameControl, senderId, teamId, playerId);
await ValidatePostPermissionAsync(context, eventObj, puzzle, isFromGameControl, senderId, teamId, playerId);

PuzzleUser sender = await context.PuzzleUsers.Where(user => user.ID == senderId).FirstOrDefaultAsync();
if (sender == null)
Expand All @@ -74,7 +76,7 @@ public async Task<ThreadMessageDTO> SendMessageAsync(string threadId, int eventI
message.PlayerID = playerId;

bool isMessageAdded = false;
using (IDbContextTransaction transaction = context.Database.BeginTransaction(System.Data.IsolationLevel.Serializable))
await using (IDbContextTransaction transaction = await context.Database.BeginTransactionAsync(System.Data.IsolationLevel.Serializable))
{
Message newestMessage = await context.Messages
.Where(existingMessage => existingMessage.ThreadId == threadId)
Expand All @@ -94,22 +96,24 @@ public async Task<ThreadMessageDTO> SendMessageAsync(string threadId, int eventI
}
}

ThreadMessageDTO dto = await BroadcastMessageAsync(message.ID);
ThreadMessageDTO dto = await BroadcastMessageAsync(context, message.ID);

if (isMessageAdded)
{
Message savedMessage = await GetMessageAsync(message.ID);
Message savedMessage = await GetMessageAsync(context, message.ID);
savedMessage.Event = eventObj;
await SendEmailNotifications(savedMessage, puzzle, eventObj);
await SendEmailNotifications(context, savedMessage, puzzle, eventObj);
}

return dto;
}

public async Task<ThreadMessageDTO> EditMessageAsync(int messageId, int currentUserId, string text)
{
Message message = await GetMessageAsync(messageId);
if (message == null || !await IsAllowedToModifyMessageAsync(message, currentUserId))
await using PuzzleServerContext context = await contextFactory.CreateDbContextAsync();

Message message = await GetMessageAsync(context, messageId);
if (message == null || !await IsAllowedToModifyMessageAsync(context, message, currentUserId))
{
throw new UserOperationException("You are not allowed to edit this message.");
}
Expand All @@ -124,13 +128,15 @@ public async Task<ThreadMessageDTO> EditMessageAsync(int messageId, int currentU
message.ModifiedDateTimeInUtc = DateTime.UtcNow;
await context.SaveChangesAsync();

return await BroadcastMessageAsync(message.ID);
return await BroadcastMessageAsync(context, message.ID);
}

public async Task<ThreadMessageDTO> DeleteMessageAsync(int messageId, int currentUserId)
{
Message message = await GetMessageAsync(messageId);
if (message == null || !await IsAllowedToModifyMessageAsync(message, currentUserId))
await using PuzzleServerContext context = await contextFactory.CreateDbContextAsync();

Message message = await GetMessageAsync(context, messageId);
if (message == null || !await IsAllowedToModifyMessageAsync(context, message, currentUserId))
{
throw new UserOperationException("You are not allowed to delete this message.");
}
Expand All @@ -139,38 +145,42 @@ public async Task<ThreadMessageDTO> DeleteMessageAsync(int messageId, int curren
message.ModifiedDateTimeInUtc = DateTime.UtcNow;
await context.SaveChangesAsync();

return await BroadcastMessageAsync(message.ID);
return await BroadcastMessageAsync(context, message.ID);
}

public async Task<ThreadMessageDTO> ClaimThreadAsync(int messageId, int currentUserId)
{
Message message = await GetMessageAsync(messageId);
if (message == null || !await IsAllowedToClaimMessageAsync(message, currentUserId) || message.ClaimerID.HasValue)
await using PuzzleServerContext context = await contextFactory.CreateDbContextAsync();

Message message = await GetMessageAsync(context, messageId);
if (message == null || !await IsAllowedToClaimMessageAsync(context, message, currentUserId) || message.ClaimerID.HasValue)
{
throw new UserOperationException("You cannot claim this thread! It may have already been claimed.");
}

message.ClaimerID = currentUserId;
await context.SaveChangesAsync();

return await BroadcastMessageAsync(message.ID);
return await BroadcastMessageAsync(context, message.ID);
}

public async Task<ThreadMessageDTO> UnclaimThreadAsync(int messageId, int currentUserId)
{
Message message = await GetMessageAsync(messageId);
if (message == null || !await IsAllowedToClaimMessageAsync(message, currentUserId))
await using PuzzleServerContext context = await contextFactory.CreateDbContextAsync();

Message message = await GetMessageAsync(context, messageId);
if (message == null || !await IsAllowedToClaimMessageAsync(context, message, currentUserId))
{
throw new UserOperationException("You are not allowed to unclaim this thread.");
}

message.ClaimerID = null;
await context.SaveChangesAsync();

return await BroadcastMessageAsync(message.ID);
return await BroadcastMessageAsync(context, message.ID);
}

private async Task<bool> IsAllowedToModifyMessageAsync(Message message, int currentUserId)
private static async Task<bool> IsAllowedToModifyMessageAsync(PuzzleServerContext context, Message message, int currentUserId)
{
Event eventObj = await context.Events.Where(e => e.ID == message.EventID).FirstOrDefaultAsync();
return eventObj != null
Expand All @@ -179,7 +189,7 @@ private async Task<bool> IsAllowedToModifyMessageAsync(Message message, int curr
&& message.Text != PuzzleThreadModel.DeletedMessage;
}

private async Task<bool> IsAllowedToClaimMessageAsync(Message message, int currentUserId)
private static async Task<bool> IsAllowedToClaimMessageAsync(PuzzleServerContext context, Message message, int currentUserId)
{
Event eventObj = await context.Events.Where(e => e.ID == message.EventID).FirstOrDefaultAsync();
PuzzleUser user = await context.PuzzleUsers.Where(u => u.ID == currentUserId).FirstOrDefaultAsync();
Expand All @@ -188,7 +198,7 @@ private async Task<bool> IsAllowedToClaimMessageAsync(Message message, int curre
&& (await user.IsAdminForEvent(context, eventObj) || await user.IsAuthorForEvent(context, eventObj));
}

private async Task ValidatePostPermissionAsync(Event eventObj, Puzzle puzzle, bool isFromGameControl, int senderId, int? teamId, int? playerId)
private static async Task ValidatePostPermissionAsync(PuzzleServerContext context, Event eventObj, Puzzle puzzle, bool isFromGameControl, int senderId, int? teamId, int? playerId)
{
PuzzleUser sender = await context.PuzzleUsers.Where(user => user.ID == senderId).FirstOrDefaultAsync();
if (sender == null)
Expand Down Expand Up @@ -225,7 +235,7 @@ private async Task ValidatePostPermissionAsync(Event eventObj, Puzzle puzzle, bo
}
}

private async Task<Message> GetMessageAsync(int messageId)
private static async Task<Message> GetMessageAsync(PuzzleServerContext context, int messageId)
{
return await context.Messages
.Include(m => m.Sender)
Expand All @@ -234,9 +244,9 @@ private async Task<Message> GetMessageAsync(int messageId)
.FirstOrDefaultAsync();
}

private async Task<ThreadMessageDTO> BroadcastMessageAsync(int messageId)
private async Task<ThreadMessageDTO> BroadcastMessageAsync(PuzzleServerContext context, int messageId)
{
Message message = await GetMessageAsync(messageId);
Message message = await GetMessageAsync(context, messageId);
ThreadMessageDTO dto = ToDto(message);
await messageHub.SendThreadMessage(dto);
return dto;
Expand All @@ -261,7 +271,7 @@ private static ThreadMessageDTO ToDto(Message message)
};
}

private async Task SendEmailNotifications(Message newMessage, Puzzle puzzle, Event eventObj)
private async Task SendEmailNotifications(PuzzleServerContext context, Message newMessage, Puzzle puzzle, Event eventObj)
{
string emailTitle = $"{newMessage.Subject} thread update!";
string emailContent = newMessage.Text;
Expand Down