532d613393
Four things that share a spine, so they arrive together. **Folders.** A hand-picked set of questions, and the fourth thing a grant can name beside exam, discipline and category. Deliberately not `user_collections` with a sharing flag: a library is a consequence of access — you save what you can already see — while a folder is a source of it, and one table holding thousands of private lists beside a handful that confer permission is one mistake away from a leak. Built from the question manager, granted on /access. Membership stays with the owner and moderators so a grantee cannot widen their own reach, and deleting a folder takes its grants with it. Two live constraints had to be rewritten to accept it: `ck_grant_has_a_dimension` and `uq_grant_dimensions` both predate `folder_id`, so a folder-only grant failed the check and two folder grants collided on the unique index. **Per-question feedback.** The learner's half already existed. What was wrong was who could read it: any grant at all let an educator list and delete reports about the whole bank. Reports are now scoped by `question_scope_predicate`, the same predicate that decides which questions that educator can see, and a reply thread makes the report a conversation the learner can follow rather than a form that swallows what they said. **Per-section notes and article feedback.** Two tables on purpose: `article_section_notes` is private to whoever wrote it, `article_feedback` goes to whoever maintains the article. Both point at the section id inside `articles.sections` rather than at `article_section_index`, whose rows are dropped on unpublish — a cascade from there would delete a learner's writing because an educator took an article down for an afternoon. A rename keeps a note attached; a deleted section leaves it marked orphaned under the heading it was written on, for its writer alone to remove. The header's feedback badge covers both, because questions and reading are the same job to whoever is doing it. Migration i9f0a1b2c3d4. 556 backend and 572 frontend tests pass. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01TqXevQJhxFrM7jJg82cgZN
207 lines
10 KiB
Python
207 lines
10 KiB
Python
"""Per-section notes and feedback on an article — two things, kept apart.
|
|
|
|
Disposable SQLite. A note is the writer's and nobody else's; feedback goes to
|
|
whoever maintains the article. The case that matters most is what happens to
|
|
writing whose section has gone: it is kept, marked, and still says which
|
|
heading it was written under.
|
|
"""
|
|
import unittest
|
|
|
|
import test_quiz_builder as fixtures
|
|
from app.models.article import Article
|
|
from app.models.category_grant import CategoryGrant
|
|
from app.models.feedback import ArticleFeedback
|
|
from app.models.user_note import ArticleSectionNote
|
|
from app.routers import articles, feedback
|
|
|
|
SECTION_A = "a" * 32
|
|
SECTION_B = "b" * 32
|
|
|
|
|
|
class ArticleNoteTests(unittest.TestCase):
|
|
def setUp(self):
|
|
self.bank = fixtures.BuilderTests()
|
|
self.bank.setUp()
|
|
self.client = self.bank.client
|
|
self.db = self.bank.db
|
|
self.client.app.include_router(articles.router, prefix="/articles")
|
|
self.client.app.include_router(feedback.router, prefix="/feedback")
|
|
self.article = Article(
|
|
id=50, slug="kawasaki", title="Kawasaki disease", status="published",
|
|
category_id=2, user_id=self.bank.mod.id,
|
|
sections=[
|
|
{"id": SECTION_A, "slug": "diagnosis", "title": "Diagnosis",
|
|
"content": "Fever for five days.", "variant": "long"},
|
|
{"id": SECTION_B, "slug": "management", "title": "Management",
|
|
"content": "IVIG.", "variant": "long"},
|
|
])
|
|
self.db.add(self.article)
|
|
self.db.commit()
|
|
self.bank.user = self.bank.owner
|
|
|
|
def tearDown(self):
|
|
self.bank.tearDown()
|
|
|
|
def write(self, section_id=SECTION_A, content="IVIG within ten days."):
|
|
return self.client.put(f"/articles/50/notes/{section_id}", json={"content": content})
|
|
|
|
def drop_section(self, section_id=SECTION_A):
|
|
"""What saving an article without that section leaves behind."""
|
|
self.article.sections = [sec for sec in self.article.sections if sec["id"] != section_id]
|
|
self.db.add(self.article)
|
|
self.db.commit()
|
|
|
|
# ── the private note ──────────────────────────────────────────────────────
|
|
|
|
def test_a_note_is_written_read_back_and_emptied(self):
|
|
self.assertEqual(self.write().status_code, 200)
|
|
notes = self.client.get("/articles/50/notes").json()
|
|
self.assertEqual(len(notes), 1)
|
|
self.assertEqual(notes[0]["section_id"], SECTION_A)
|
|
self.assertEqual(notes[0]["section_title"], "Diagnosis")
|
|
self.assertFalse(notes[0]["orphaned"])
|
|
# Empty means gone, the same as a question note.
|
|
self.assertEqual(self.write(content=" ").status_code, 200)
|
|
self.assertEqual(self.client.get("/articles/50/notes").json(), [])
|
|
|
|
def test_a_note_is_nobody_elses_business(self):
|
|
self.write()
|
|
self.bank.user = self.bank.peer
|
|
self.assertEqual(self.client.get("/articles/50/notes").json(), [])
|
|
# Not even a moderator, who maintains the article, reads it.
|
|
self.bank.user = self.bank.mod
|
|
self.assertEqual(self.client.get("/articles/50/notes").json(), [])
|
|
|
|
def test_two_readers_keep_separate_notes_on_one_section(self):
|
|
self.write(content="Mine")
|
|
self.bank.user = self.bank.peer
|
|
self.write(content="Theirs")
|
|
self.assertEqual(self.client.get("/articles/50/notes").json()[0]["content"], "Theirs")
|
|
self.bank.user = self.bank.owner
|
|
self.assertEqual(self.client.get("/articles/50/notes").json()[0]["content"], "Mine")
|
|
|
|
def test_a_note_on_a_section_that_does_not_exist_is_refused(self):
|
|
self.assertEqual(self.write(section_id="c" * 32).status_code, 404)
|
|
|
|
# ── what happens to writing when a section changes ────────────────────────
|
|
|
|
def test_renaming_a_section_keeps_the_note_attached(self):
|
|
self.write()
|
|
# Reassigned rather than mutated in place: a JSON column does not
|
|
# notice an edit inside the list, which is also why the editor rewrites
|
|
# the whole array on save.
|
|
self.article.sections = [{**self.article.sections[0], "title": "Making the diagnosis"},
|
|
self.article.sections[1]]
|
|
self.db.add(self.article)
|
|
self.db.commit()
|
|
note = self.client.get("/articles/50/notes").json()[0]
|
|
# The id is what the note points at, so a retitled heading is still the
|
|
# same section — the note follows the rename rather than orphaning.
|
|
self.assertFalse(note["orphaned"])
|
|
self.assertEqual(note["section_title"], "Making the diagnosis")
|
|
|
|
def test_a_deleted_section_keeps_the_note_and_says_where_it_came_from(self):
|
|
self.write()
|
|
self.drop_section()
|
|
note = self.client.get("/articles/50/notes").json()[0]
|
|
self.assertTrue(note["orphaned"])
|
|
self.assertEqual(note["content"], "IVIG within ten days.")
|
|
# Losing somebody's writing silently is the outcome being engineered
|
|
# against; the heading it was written under is what makes it readable.
|
|
self.assertEqual(note["section_title"], "Diagnosis")
|
|
self.assertEqual(self.db.query(ArticleSectionNote).count(), 1)
|
|
|
|
def test_unpublishing_an_article_does_not_touch_its_notes(self):
|
|
"""The section index is dropped on unpublish; the notes are not keyed to it."""
|
|
self.write()
|
|
self.bank.user = self.bank.mod
|
|
self.article.status = "draft"
|
|
self.db.add(self.article)
|
|
self.db.commit()
|
|
self.bank.user = self.bank.owner
|
|
# A learner cannot read an unpublished article, so the note is out of
|
|
# reach — but it is still there, and comes back when the article does.
|
|
self.assertEqual(self.client.get("/articles/50/notes").status_code, 404)
|
|
self.assertEqual(self.db.query(ArticleSectionNote).count(), 1)
|
|
self.bank.user = self.bank.mod
|
|
self.article.status = "published"
|
|
self.db.add(self.article)
|
|
self.db.commit()
|
|
self.bank.user = self.bank.owner
|
|
self.assertEqual(len(self.client.get("/articles/50/notes").json()), 1)
|
|
|
|
def test_the_writer_can_delete_their_own_orphan(self):
|
|
self.write()
|
|
self.drop_section()
|
|
self.assertEqual(self.client.delete(f"/articles/50/notes/{SECTION_A}").status_code, 204)
|
|
self.assertEqual(self.db.query(ArticleSectionNote).count(), 0)
|
|
|
|
# ── feedback, which is the other thing entirely ───────────────────────────
|
|
|
|
def test_feedback_on_a_section_reaches_whoever_maintains_the_article(self):
|
|
sent = self.client.post("/feedback/articles/50",
|
|
json={"message": "The IVIG dose is wrong.", "section_id": SECTION_B})
|
|
self.assertEqual(sent.status_code, 201, sent.text)
|
|
self.bank.user = self.bank.mod
|
|
rows = self.client.get("/feedback/articles/50").json()
|
|
self.assertEqual(len(rows), 1)
|
|
self.assertEqual(rows[0]["section_title"], "Management")
|
|
self.assertEqual(rows[0]["href"], f"/articles/50?section={SECTION_B}")
|
|
self.assertFalse(rows[0]["section_gone"])
|
|
|
|
def test_feedback_and_a_note_are_not_the_same_row(self):
|
|
self.write(section_id=SECTION_B, content="Private thought")
|
|
self.client.post("/feedback/articles/50", json={"message": "Typo here.", "section_id": SECTION_B})
|
|
self.bank.user = self.bank.mod
|
|
reported = self.client.get("/feedback/articles/50").json()
|
|
self.assertEqual([row["message"] for row in reported], ["Typo here."])
|
|
# The private note is nowhere in what the maintainer sees.
|
|
self.assertNotIn("Private thought", str(reported))
|
|
|
|
def test_a_report_survives_the_section_it_was_about(self):
|
|
self.client.post("/feedback/articles/50", json={"message": "Wrong.", "section_id": SECTION_A})
|
|
self.drop_section()
|
|
self.bank.user = self.bank.mod
|
|
row = self.client.get("/feedback/articles/50").json()[0]
|
|
self.assertTrue(row["section_gone"])
|
|
self.assertEqual(row["section_title"], "Diagnosis")
|
|
|
|
def test_only_the_maintainer_reads_and_answers_article_feedback(self):
|
|
self.client.post("/feedback/articles/50", json={"message": "Wrong."})
|
|
row_id = self.db.query(ArticleFeedback).one().id
|
|
self.bank.user = self.bank.peer
|
|
self.assertEqual(self.client.get("/feedback/articles/50").status_code, 403)
|
|
self.assertEqual(self.client.patch(f"/feedback/articles/reports/{row_id}",
|
|
json={"status": "resolved"}).status_code, 403)
|
|
# A grant over the branch the article is filed in is enough.
|
|
self.db.add(CategoryGrant(category_id=1, user_id=self.bank.peer.id))
|
|
self.db.commit()
|
|
self.assertEqual(self.client.get("/feedback/articles/50").status_code, 200)
|
|
|
|
def test_a_reply_closes_the_thread_and_the_reader_can_see_it(self):
|
|
self.client.post("/feedback/articles/50", json={"message": "Wrong.", "section_id": SECTION_A})
|
|
row_id = self.db.query(ArticleFeedback).one().id
|
|
self.bank.user = self.bank.mod
|
|
answered = self.client.patch(f"/feedback/articles/reports/{row_id}",
|
|
json={"reply": "Rewritten.", "status": "resolved"})
|
|
self.assertEqual(answered.status_code, 200, answered.text)
|
|
self.bank.user = self.bank.owner
|
|
mine = self.client.get("/feedback/articles/50/mine").json()
|
|
self.assertEqual((mine[0]["reply"], mine[0]["status"]), ("Rewritten.", "resolved"))
|
|
|
|
def test_article_reports_share_the_educator_badge(self):
|
|
self.client.post("/feedback/articles/50", json={"message": "Wrong.", "section_id": SECTION_A})
|
|
self.bank.user = self.bank.mod
|
|
body = self.client.get("/feedback/open").json()
|
|
self.assertEqual(body["open"], 1)
|
|
self.assertEqual(body["items"][0]["kind"], "article")
|
|
|
|
def test_feedback_on_a_section_that_does_not_exist_is_refused(self):
|
|
self.assertEqual(self.client.post(
|
|
"/feedback/articles/50", json={"message": "Wrong.", "section_id": "z" * 32}).status_code, 404)
|
|
self.assertEqual(self.client.post(
|
|
"/feedback/articles/999", json={"message": "Wrong."}).status_code, 404)
|
|
|
|
|
|
if __name__ == "__main__":
|
|
unittest.main()
|