From 9f62444a63ed018dc42a824c47d99b7d1bd78a78 Mon Sep 17 00:00:00 2001 From: hz Date: Wed, 22 Jul 2026 22:42:19 -0500 Subject: [PATCH] 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. --- .../QuestionVisibilityTests.cs | 17 ++++++ .../Controllers/QuestionReportsController.cs | 2 +- .../Controllers/QuestionsController.cs | 37 +++++++++++-- api/QuestionsApi/Models/Question.cs | 3 ++ api/QuestionsApi/Models/QuestionRequests.cs | 5 ++ api/QuestionsApi/Models/QuestionResponse.cs | 24 +++++++++ api/QuestionsApi/Program.cs | 10 ++++ .../Services/FlagMediaQuestionsCommand.cs | 21 ++++++++ .../Services/QuestionBookmarkStore.cs | 12 ++++- api/QuestionsApi/Services/QuestionStore.cs | 52 ++++++++++++++++++- .../Services/QuestionVisibility.cs | 9 ++++ api/QuestionsApi/Services/ReviewQueueStore.cs | 2 +- tests/flashcard_ownership_ui_test.gd.uid | 1 + .../flashcard_session_navigation_test.gd.uid | 1 + 14 files changed, 187 insertions(+), 9 deletions(-) create mode 100644 api/QuestionsApi.Tests/QuestionVisibilityTests.cs create mode 100644 api/QuestionsApi/Services/FlagMediaQuestionsCommand.cs create mode 100644 api/QuestionsApi/Services/QuestionVisibility.cs create mode 100644 tests/flashcard_ownership_ui_test.gd.uid create mode 100644 tests/flashcard_session_navigation_test.gd.uid diff --git a/api/QuestionsApi.Tests/QuestionVisibilityTests.cs b/api/QuestionsApi.Tests/QuestionVisibilityTests.cs new file mode 100644 index 0000000..b39965f --- /dev/null +++ b/api/QuestionsApi.Tests/QuestionVisibilityTests.cs @@ -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)); + } +} diff --git a/api/QuestionsApi/Controllers/QuestionReportsController.cs b/api/QuestionsApi/Controllers/QuestionReportsController.cs index f202fef..09c2373 100644 --- a/api/QuestionsApi/Controllers/QuestionReportsController.cs +++ b/api/QuestionsApi/Controllers/QuestionReportsController.cs @@ -39,7 +39,7 @@ public class QuestionReportsController : ControllerBase return BadRequest($"details must be between {MinDetailsLength} and {MaxDetailsLength} characters"); 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(); var userId = CurrentUserId(); diff --git a/api/QuestionsApi/Controllers/QuestionsController.cs b/api/QuestionsApi/Controllers/QuestionsController.cs index 0116d2b..defbb92 100644 --- a/api/QuestionsApi/Controllers/QuestionsController.cs +++ b/api/QuestionsApi/Controllers/QuestionsController.cs @@ -94,7 +94,7 @@ public class QuestionsController : ControllerBase public async Task Get(string 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(); var percentages = await _statistics.GetPercentagesForQuestionAsync(question.ExternalId); @@ -119,7 +119,7 @@ public class QuestionsController : ControllerBase return BadRequest("choiceId required"); 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(); if (!question.Choices.Any(c => c.Id == req.ChoiceId.Trim())) return BadRequest("choiceId must match a choice id"); @@ -258,7 +258,7 @@ public class QuestionsController : ControllerBase return BadRequest("externalId required"); 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(); var userId = CurrentUserId(); @@ -404,6 +404,37 @@ public class QuestionsController : ControllerBase return Ok("Deleted"); } + [HttpPost("{id}/flag")] + [Authorize(Roles = "SUPER")] + public async Task 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 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 Flagged() + { + var flagged = await _questions.GetFlaggedAsync(); + return Ok(flagged.Select(QuestionFlagResponse.FromModel)); + } + private string CurrentUserId() => User.FindFirstValue(ClaimTypes.NameIdentifier) ?? User.FindFirstValue(ClaimTypes.Name) diff --git a/api/QuestionsApi/Models/Question.cs b/api/QuestionsApi/Models/Question.cs index f621fc1..0bf6a20 100644 --- a/api/QuestionsApi/Models/Question.cs +++ b/api/QuestionsApi/Models/Question.cs @@ -30,6 +30,9 @@ public class Question public List Tags { get; set; } = []; public List References { get; set; } = []; 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 UpdatedUtc { get; set; } = DateTime.UtcNow; } diff --git a/api/QuestionsApi/Models/QuestionRequests.cs b/api/QuestionsApi/Models/QuestionRequests.cs index 9ddbb26..7004cef 100644 --- a/api/QuestionsApi/Models/QuestionRequests.cs +++ b/api/QuestionsApi/Models/QuestionRequests.cs @@ -38,6 +38,11 @@ public class UpsertQuestionRequest public bool IsPublished { get; set; } = true; } +public class FlagQuestionRequest +{ + public string Reason { get; set; } = ""; +} + public class FirstQuestionAttemptRequest { public string QuestionExternalId { get; set; } = ""; diff --git a/api/QuestionsApi/Models/QuestionResponse.cs b/api/QuestionsApi/Models/QuestionResponse.cs index f9240ec..0c636ba 100644 --- a/api/QuestionsApi/Models/QuestionResponse.cs +++ b/api/QuestionsApi/Models/QuestionResponse.cs @@ -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 string QuestionExternalId { get; set; } = ""; diff --git a/api/QuestionsApi/Program.cs b/api/QuestionsApi/Program.cs index b62f74f..b1e094b 100644 --- a/api/QuestionsApi/Program.cs +++ b/api/QuestionsApi/Program.cs @@ -27,6 +27,16 @@ if (args.FirstOrDefault() == "seed-flashcards") 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); builder.Services.AddControllers(); builder.Services.Configure(options => diff --git a/api/QuestionsApi/Services/FlagMediaQuestionsCommand.cs b/api/QuestionsApi/Services/FlagMediaQuestionsCommand.cs new file mode 100644 index 0000000..ad5ee12 --- /dev/null +++ b/api/QuestionsApi/Services/FlagMediaQuestionsCommand.cs @@ -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 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; + } +} diff --git a/api/QuestionsApi/Services/QuestionBookmarkStore.cs b/api/QuestionsApi/Services/QuestionBookmarkStore.cs index bfdc6fd..33310de 100644 --- a/api/QuestionsApi/Services/QuestionBookmarkStore.cs +++ b/api/QuestionsApi/Services/QuestionBookmarkStore.cs @@ -105,7 +105,11 @@ public sealed class QuestionBookmarkStore } 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(discipline)) questionMatch["Question.Discipline"] = discipline.Trim(); if (!string.IsNullOrWhiteSpace(query)) @@ -172,7 +176,11 @@ public sealed class QuestionBookmarkStore new("$unwind", "$Question") }; 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 { { "_id", BsonNull.Value }, diff --git a/api/QuestionsApi/Services/QuestionStore.cs b/api/QuestionsApi/Services/QuestionStore.cs index 710e89a..3e6b74e 100644 --- a/api/QuestionsApi/Services/QuestionStore.cs +++ b/api/QuestionsApi/Services/QuestionStore.cs @@ -26,7 +26,10 @@ public class QuestionStore var filters = new List>(); if (!includeUnpublished) + { 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") filters.Add(builder.Eq(q => q.System, req.System.Trim())); if (!string.IsNullOrWhiteSpace(req.Discipline)) @@ -65,7 +68,7 @@ public class QuestionStore await _col.Find(Builders.Filter.Empty).ToListAsync(); public async Task> GetPublishedAsync() => - await _col.Find(question => question.IsPublished).ToListAsync(); + await _col.Find(question => question.IsPublished && question.FlaggedForReviewUtc == null).ToListAsync(); public async Task> GetByExternalIdsAsync(IEnumerable externalIds) { @@ -125,6 +128,9 @@ public class QuestionStore return null; 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); return result.MatchedCount == 0 ? null : question; } @@ -143,6 +149,9 @@ public class QuestionStore { question.Id = existing.Id; question.CreatedUtc = existing.CreatedUtc; + question.FlaggedForReviewUtc = existing.FlaggedForReviewUtc; + question.FlagReason = existing.FlagReason; + question.FlaggedByUserId = existing.FlaggedByUserId; } else { @@ -167,7 +176,7 @@ public class QuestionStore { var filter = includeUnpublished ? Builders.Filter.Empty - : Builders.Filter.Eq(q => q.IsPublished, true); + : Builders.Filter.Eq(q => q.IsPublished, true) & Builders.Filter.Eq(q => q.FlaggedForReviewUtc, null); var systems = await _col.Distinct(q => q.System, filter).ToListAsync(); 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(); } + // 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 FlagAllWithMediaAsync(string flaggedByUserId, string reason) + { + var filter = Builders.Filter.Exists("Media.0", true) + & Builders.Filter.Eq(q => q.FlaggedForReviewUtc, null); + var update = Builders.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 FlagAsync(string id, string reason, string flaggedByUserId) + { + var update = Builders.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 { ReturnDocument = ReturnDocument.After }); + } + + public async Task UnflagAsync(string id) + { + var update = Builders.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 { ReturnDocument = ReturnDocument.After }); + } + + public async Task> GetFlaggedAsync() => + await _col.Find(q => q.FlaggedForReviewUtc != null) + .SortByDescending(q => q.FlaggedForReviewUtc) + .ToListAsync(); + public async Task UpdateReferencesByExternalIdAsync(IEnumerable updates) { var now = DateTime.UtcNow; diff --git a/api/QuestionsApi/Services/QuestionVisibility.cs b/api/QuestionsApi/Services/QuestionVisibility.cs new file mode 100644 index 0000000..12687a3 --- /dev/null +++ b/api/QuestionsApi/Services/QuestionVisibility.cs @@ -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; +} diff --git a/api/QuestionsApi/Services/ReviewQueueStore.cs b/api/QuestionsApi/Services/ReviewQueueStore.cs index bf63817..54d0125 100644 --- a/api/QuestionsApi/Services/ReviewQueueStore.cs +++ b/api/QuestionsApi/Services/ReviewQueueStore.cs @@ -129,7 +129,7 @@ public sealed class ReviewQueueStore foreach (var candidate in candidates) { 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( value => value.Id == candidate.Id && value.State == ReviewQueueStates.Pending, Builders.Update diff --git a/tests/flashcard_ownership_ui_test.gd.uid b/tests/flashcard_ownership_ui_test.gd.uid new file mode 100644 index 0000000..65ff0be --- /dev/null +++ b/tests/flashcard_ownership_ui_test.gd.uid @@ -0,0 +1 @@ +uid://b1nkwx4qlt27l diff --git a/tests/flashcard_session_navigation_test.gd.uid b/tests/flashcard_session_navigation_test.gd.uid new file mode 100644 index 0000000..f4a39e1 --- /dev/null +++ b/tests/flashcard_session_navigation_test.gd.uid @@ -0,0 +1 @@ +uid://cmhk4acaxbsjs