Add a review-flag state for questions, independent of publish status

Flagged questions disappear from every student-facing surface (search,
exam simulation sampling, the missed-question review queue, bookmarks,
direct fetch) while staying published - flagging and unpublishing are
now separate, reversible actions with their own audit trail (who
flagged it, when, why). Backend-only for now: SUPER-only flag/unflag/
list endpoints, no client UI. Includes a one-time CLI sweep
(flag-media-questions) to flag every existing question with an image
so they can be reviewed.
This commit is contained in:
hz
2026-07-22 22:42:19 -05:00
parent e8d615985b
commit 9f62444a63
14 changed files with 187 additions and 9 deletions
@@ -0,0 +1,17 @@
using QuestionsApi.Services;
using Xunit;
namespace QuestionsApi.Tests;
public sealed class QuestionVisibilityTests
{
[Theory]
[InlineData(true, false, true)]
[InlineData(true, true, false)]
[InlineData(false, false, false)]
[InlineData(false, true, false)]
public void IsVisibleToStudent_MatchesTruthTable(bool isPublished, bool isFlagged, bool expected)
{
Assert.Equal(expected, QuestionVisibility.IsVisibleToStudent(isPublished, isFlagged));
}
}
@@ -39,7 +39,7 @@ public class QuestionReportsController : ControllerBase
return BadRequest($"details must be between {MinDetailsLength} and {MaxDetailsLength} characters"); return BadRequest($"details must be between {MinDetailsLength} and {MaxDetailsLength} characters");
var question = await _questions.GetByExternalIdAsync(questionExternalId); var question = await _questions.GetByExternalIdAsync(questionExternalId);
if (question is null || (!question.IsPublished && !User.IsInRole("SUPER"))) if (question is null || (!QuestionVisibility.IsVisibleToStudent(question.IsPublished, question.FlaggedForReviewUtc != null) && !User.IsInRole("SUPER")))
return NotFound(); return NotFound();
var userId = CurrentUserId(); var userId = CurrentUserId();
@@ -94,7 +94,7 @@ public class QuestionsController : ControllerBase
public async Task<IActionResult> Get(string id) public async Task<IActionResult> Get(string id)
{ {
var question = await _questions.GetByIdAsync(id); var question = await _questions.GetByIdAsync(id);
if (question is null || (!question.IsPublished && !User.IsInRole("SUPER"))) if (question is null || (!QuestionVisibility.IsVisibleToStudent(question.IsPublished, question.FlaggedForReviewUtc != null) && !User.IsInRole("SUPER")))
return NotFound(); return NotFound();
var percentages = await _statistics.GetPercentagesForQuestionAsync(question.ExternalId); var percentages = await _statistics.GetPercentagesForQuestionAsync(question.ExternalId);
@@ -119,7 +119,7 @@ public class QuestionsController : ControllerBase
return BadRequest("choiceId required"); return BadRequest("choiceId required");
var question = await _questions.GetByExternalIdAsync(req.QuestionExternalId.Trim()); var question = await _questions.GetByExternalIdAsync(req.QuestionExternalId.Trim());
if (question is null || (!question.IsPublished && !User.IsInRole("SUPER"))) if (question is null || (!QuestionVisibility.IsVisibleToStudent(question.IsPublished, question.FlaggedForReviewUtc != null) && !User.IsInRole("SUPER")))
return NotFound(); return NotFound();
if (!question.Choices.Any(c => c.Id == req.ChoiceId.Trim())) if (!question.Choices.Any(c => c.Id == req.ChoiceId.Trim()))
return BadRequest("choiceId must match a choice id"); return BadRequest("choiceId must match a choice id");
@@ -258,7 +258,7 @@ public class QuestionsController : ControllerBase
return BadRequest("externalId required"); return BadRequest("externalId required");
var question = await _questions.GetByExternalIdAsync(externalId.Trim()); var question = await _questions.GetByExternalIdAsync(externalId.Trim());
if (question is null || (!question.IsPublished && !User.IsInRole("SUPER"))) if (question is null || (!QuestionVisibility.IsVisibleToStudent(question.IsPublished, question.FlaggedForReviewUtc != null) && !User.IsInRole("SUPER")))
return NotFound(); return NotFound();
var userId = CurrentUserId(); var userId = CurrentUserId();
@@ -404,6 +404,37 @@ public class QuestionsController : ControllerBase
return Ok("Deleted"); return Ok("Deleted");
} }
[HttpPost("{id}/flag")]
[Authorize(Roles = "SUPER")]
public async Task<IActionResult> Flag(string id, [FromBody] FlagQuestionRequest req)
{
if (string.IsNullOrWhiteSpace(req.Reason))
return BadRequest("reason required");
var flagged = await _questions.FlagAsync(id, req.Reason.Trim(), CurrentUserId());
if (flagged is null)
return NotFound();
return Ok(QuestionFlagResponse.FromModel(flagged));
}
[HttpPost("{id}/unflag")]
[Authorize(Roles = "SUPER")]
public async Task<IActionResult> Unflag(string id)
{
var unflagged = await _questions.UnflagAsync(id);
if (unflagged is null)
return NotFound();
return Ok(QuestionFlagResponse.FromModel(unflagged));
}
[HttpGet("admin/flagged")]
[Authorize(Roles = "SUPER")]
public async Task<IActionResult> Flagged()
{
var flagged = await _questions.GetFlaggedAsync();
return Ok(flagged.Select(QuestionFlagResponse.FromModel));
}
private string CurrentUserId() => private string CurrentUserId() =>
User.FindFirstValue(ClaimTypes.NameIdentifier) User.FindFirstValue(ClaimTypes.NameIdentifier)
?? User.FindFirstValue(ClaimTypes.Name) ?? User.FindFirstValue(ClaimTypes.Name)
+3
View File
@@ -30,6 +30,9 @@ public class Question
public List<string> Tags { get; set; } = []; public List<string> Tags { get; set; } = [];
public List<QuestionReference> References { get; set; } = []; public List<QuestionReference> References { get; set; } = [];
public bool IsPublished { get; set; } = true; public bool IsPublished { get; set; } = true;
public DateTime? FlaggedForReviewUtc { get; set; }
public string FlagReason { get; set; } = "";
public string FlaggedByUserId { get; set; } = "";
public DateTime CreatedUtc { get; set; } = DateTime.UtcNow; public DateTime CreatedUtc { get; set; } = DateTime.UtcNow;
public DateTime UpdatedUtc { get; set; } = DateTime.UtcNow; public DateTime UpdatedUtc { get; set; } = DateTime.UtcNow;
} }
@@ -38,6 +38,11 @@ public class UpsertQuestionRequest
public bool IsPublished { get; set; } = true; public bool IsPublished { get; set; } = true;
} }
public class FlagQuestionRequest
{
public string Reason { get; set; } = "";
}
public class FirstQuestionAttemptRequest public class FirstQuestionAttemptRequest
{ {
public string QuestionExternalId { get; set; } = ""; public string QuestionExternalId { get; set; } = "";
@@ -111,6 +111,30 @@ public class QuestionResponse
} }
} }
// Admin-only shape for the flag/unflag/list-flagged endpoints - deliberately separate from
// QuestionResponse (which is student-facing and never exposes IsPublished or the flag fields).
public class QuestionFlagResponse
{
public string Id { get; set; } = "";
public string ExternalId { get; set; } = "";
public string Stem { get; set; } = "";
public bool IsPublished { get; set; }
public DateTime? FlaggedForReviewUtc { get; set; }
public string FlagReason { get; set; } = "";
public string FlaggedByUserId { get; set; } = "";
public static QuestionFlagResponse FromModel(Question question) => new()
{
Id = question.Id ?? "",
ExternalId = question.ExternalId,
Stem = question.Stem,
IsPublished = question.IsPublished,
FlaggedForReviewUtc = question.FlaggedForReviewUtc,
FlagReason = question.FlagReason,
FlaggedByUserId = question.FlaggedByUserId
};
}
public class FirstQuestionAttemptResponse public class FirstQuestionAttemptResponse
{ {
public string QuestionExternalId { get; set; } = ""; public string QuestionExternalId { get; set; } = "";
+10
View File
@@ -27,6 +27,16 @@ if (args.FirstOrDefault() == "seed-flashcards")
return; return;
} }
if (args.FirstOrDefault() == "flag-media-questions")
{
var flagConfiguration = new ConfigurationBuilder()
.AddJsonFile("appsettings.json", optional: true)
.AddEnvironmentVariables()
.Build();
Environment.ExitCode = await FlagMediaQuestionsCommand.RunAsync(flagConfiguration, Console.Out);
return;
}
var builder = WebApplication.CreateBuilder(args); var builder = WebApplication.CreateBuilder(args);
builder.Services.AddControllers(); builder.Services.AddControllers();
builder.Services.Configure<ForwardedHeadersOptions>(options => builder.Services.Configure<ForwardedHeadersOptions>(options =>
@@ -0,0 +1,21 @@
namespace QuestionsApi.Services;
// One-time sweep: flags every question that currently has at least one image so it can be
// pulled from student rotation for review, without touching IsPublished. Idempotent (only
// touches questions not already flagged, via QuestionStore.FlagAllWithMediaAsync's filter) -
// safe to re-run.
public static class FlagMediaQuestionsCommand
{
private const string Reason = "Bulk-flagged for image review";
private const string FlaggedBy = "system";
public static async Task<int> RunAsync(IConfiguration configuration, TextWriter output)
{
var mongo = new MongoDatabaseProvider(configuration);
var questions = new QuestionStore(mongo);
var flagged = await questions.FlagAllWithMediaAsync(FlaggedBy, Reason);
await output.WriteLineAsync($"Flagged {flagged} question(s) with images for review.");
return 0;
}
}
@@ -105,7 +105,11 @@ public sealed class QuestionBookmarkStore
} }
var questionMatch = new BsonDocument(); var questionMatch = new BsonDocument();
if (!includeUnpublished) questionMatch["Question.IsPublished"] = true; if (!includeUnpublished)
{
questionMatch["Question.IsPublished"] = true;
questionMatch["Question.FlaggedForReviewUtc"] = BsonNull.Value;
}
if (!string.IsNullOrWhiteSpace(system) && system != "All Systems") questionMatch["Question.System"] = system.Trim(); if (!string.IsNullOrWhiteSpace(system) && system != "All Systems") questionMatch["Question.System"] = system.Trim();
if (!string.IsNullOrWhiteSpace(discipline)) questionMatch["Question.Discipline"] = discipline.Trim(); if (!string.IsNullOrWhiteSpace(discipline)) questionMatch["Question.Discipline"] = discipline.Trim();
if (!string.IsNullOrWhiteSpace(query)) if (!string.IsNullOrWhiteSpace(query))
@@ -172,7 +176,11 @@ public sealed class QuestionBookmarkStore
new("$unwind", "$Question") new("$unwind", "$Question")
}; };
if (!includeUnpublished) if (!includeUnpublished)
pipeline.Add(new BsonDocument("$match", new BsonDocument("Question.IsPublished", true))); pipeline.Add(new BsonDocument("$match", new BsonDocument
{
{ "Question.IsPublished", true },
{ "Question.FlaggedForReviewUtc", BsonNull.Value }
}));
pipeline.Add(new BsonDocument("$group", new BsonDocument pipeline.Add(new BsonDocument("$group", new BsonDocument
{ {
{ "_id", BsonNull.Value }, { "_id", BsonNull.Value },
+50 -2
View File
@@ -26,7 +26,10 @@ public class QuestionStore
var filters = new List<FilterDefinition<Question>>(); var filters = new List<FilterDefinition<Question>>();
if (!includeUnpublished) if (!includeUnpublished)
{
filters.Add(builder.Eq(q => q.IsPublished, true)); filters.Add(builder.Eq(q => q.IsPublished, true));
filters.Add(builder.Eq(q => q.FlaggedForReviewUtc, null));
}
if (!string.IsNullOrWhiteSpace(req.System) && req.System != "All Systems") if (!string.IsNullOrWhiteSpace(req.System) && req.System != "All Systems")
filters.Add(builder.Eq(q => q.System, req.System.Trim())); filters.Add(builder.Eq(q => q.System, req.System.Trim()));
if (!string.IsNullOrWhiteSpace(req.Discipline)) if (!string.IsNullOrWhiteSpace(req.Discipline))
@@ -65,7 +68,7 @@ public class QuestionStore
await _col.Find(Builders<Question>.Filter.Empty).ToListAsync(); await _col.Find(Builders<Question>.Filter.Empty).ToListAsync();
public async Task<List<Question>> GetPublishedAsync() => public async Task<List<Question>> GetPublishedAsync() =>
await _col.Find(question => question.IsPublished).ToListAsync(); await _col.Find(question => question.IsPublished && question.FlaggedForReviewUtc == null).ToListAsync();
public async Task<Dictionary<string, Question>> GetByExternalIdsAsync(IEnumerable<string> externalIds) public async Task<Dictionary<string, Question>> GetByExternalIdsAsync(IEnumerable<string> externalIds)
{ {
@@ -125,6 +128,9 @@ public class QuestionStore
return null; return null;
question.CreatedUtc = existing.CreatedUtc; question.CreatedUtc = existing.CreatedUtc;
question.FlaggedForReviewUtc = existing.FlaggedForReviewUtc;
question.FlagReason = existing.FlagReason;
question.FlaggedByUserId = existing.FlaggedByUserId;
var result = await _col.ReplaceOneAsync(q => q.Id == id, question); var result = await _col.ReplaceOneAsync(q => q.Id == id, question);
return result.MatchedCount == 0 ? null : question; return result.MatchedCount == 0 ? null : question;
} }
@@ -143,6 +149,9 @@ public class QuestionStore
{ {
question.Id = existing.Id; question.Id = existing.Id;
question.CreatedUtc = existing.CreatedUtc; question.CreatedUtc = existing.CreatedUtc;
question.FlaggedForReviewUtc = existing.FlaggedForReviewUtc;
question.FlagReason = existing.FlagReason;
question.FlaggedByUserId = existing.FlaggedByUserId;
} }
else else
{ {
@@ -167,7 +176,7 @@ public class QuestionStore
{ {
var filter = includeUnpublished var filter = includeUnpublished
? Builders<Question>.Filter.Empty ? Builders<Question>.Filter.Empty
: Builders<Question>.Filter.Eq(q => q.IsPublished, true); : Builders<Question>.Filter.Eq(q => q.IsPublished, true) & Builders<Question>.Filter.Eq(q => q.FlaggedForReviewUtc, null);
var systems = await _col.Distinct(q => q.System, filter).ToListAsync(); var systems = await _col.Distinct(q => q.System, filter).ToListAsync();
return systems.Where(s => !string.IsNullOrWhiteSpace(s)).OrderBy(s => s).ToList(); return systems.Where(s => !string.IsNullOrWhiteSpace(s)).OrderBy(s => s).ToList();
} }
@@ -181,6 +190,45 @@ public class QuestionStore
return requested.Except(existing, StringComparer.Ordinal).OrderBy(id => id).ToList(); return requested.Except(existing, StringComparer.Ordinal).OrderBy(id => id).ToList();
} }
// One-time bulk sweep: flags every question that has at least one image and isn't already
// flagged, in a single update rather than fetching everything into memory.
public async Task<long> FlagAllWithMediaAsync(string flaggedByUserId, string reason)
{
var filter = Builders<Question>.Filter.Exists("Media.0", true)
& Builders<Question>.Filter.Eq(q => q.FlaggedForReviewUtc, null);
var update = Builders<Question>.Update
.Set(q => q.FlaggedForReviewUtc, DateTime.UtcNow)
.Set(q => q.FlagReason, reason)
.Set(q => q.FlaggedByUserId, flaggedByUserId);
var result = await _col.UpdateManyAsync(filter, update);
return result.ModifiedCount;
}
public async Task<Question?> FlagAsync(string id, string reason, string flaggedByUserId)
{
var update = Builders<Question>.Update
.Set(q => q.FlaggedForReviewUtc, DateTime.UtcNow)
.Set(q => q.FlagReason, reason)
.Set(q => q.FlaggedByUserId, flaggedByUserId);
return await _col.FindOneAndUpdateAsync(q => q.Id == id, update,
new FindOneAndUpdateOptions<Question> { ReturnDocument = ReturnDocument.After });
}
public async Task<Question?> UnflagAsync(string id)
{
var update = Builders<Question>.Update
.Set(q => q.FlaggedForReviewUtc, null)
.Set(q => q.FlagReason, "")
.Set(q => q.FlaggedByUserId, "");
return await _col.FindOneAndUpdateAsync(q => q.Id == id, update,
new FindOneAndUpdateOptions<Question> { ReturnDocument = ReturnDocument.After });
}
public async Task<List<Question>> GetFlaggedAsync() =>
await _col.Find(q => q.FlaggedForReviewUtc != null)
.SortByDescending(q => q.FlaggedForReviewUtc)
.ToListAsync();
public async Task<int> UpdateReferencesByExternalIdAsync(IEnumerable<QuestionReferencesUpdate> updates) public async Task<int> UpdateReferencesByExternalIdAsync(IEnumerable<QuestionReferencesUpdate> updates)
{ {
var now = DateTime.UtcNow; var now = DateTime.UtcNow;
@@ -0,0 +1,9 @@
namespace QuestionsApi.Services;
// Pure student-visibility decision for a question, kept dependency-free (like FlashcardScheduling)
// so it's unit testable without a controller or Mongo. Consolidates what used to be duplicated
// inline across QuestionsController/QuestionReportsController.
public static class QuestionVisibility
{
public static bool IsVisibleToStudent(bool isPublished, bool isFlagged) => isPublished && !isFlagged;
}
@@ -129,7 +129,7 @@ public sealed class ReviewQueueStore
foreach (var candidate in candidates) foreach (var candidate in candidates)
{ {
var question = await _questions.GetByExternalIdAsync(candidate.QuestionExternalId); var question = await _questions.GetByExternalIdAsync(candidate.QuestionExternalId);
if (question is { IsPublished: true }) return candidate; if (question is { IsPublished: true, FlaggedForReviewUtc: null }) return candidate;
await _items.UpdateOneAsync( await _items.UpdateOneAsync(
value => value.Id == candidate.Id && value.State == ReviewQueueStates.Pending, value => value.Id == candidate.Id && value.State == ReviewQueueStates.Pending,
Builders<ReviewQueueItem>.Update Builders<ReviewQueueItem>.Update
+1
View File
@@ -0,0 +1 @@
uid://b1nkwx4qlt27l
@@ -0,0 +1 @@
uid://cmhk4acaxbsjs