From abbe6e6b2eeb0a84b94b1d49d0036082c0081488 Mon Sep 17 00:00:00 2001 From: Tim Van Baak Date: Wed, 15 Sep 2021 00:17:14 -0700 Subject: [PATCH] Track Ersatz status with flag instead of null character Previously, articles written by Ersatz Scrivener were represented in the data model by having a NULL character id. This had several awkward consequences, such as therebeing no real way to distinguish Ersatz articles from different players without doubling up on foreign keys whenever there was a character reference, because there would need to be a user reference as well. Using a flag on the article itself is a much cleaner solution. There is no longer a need to have both character and user FKs. Ersatz-ness is still a special case, but one easily tracked on articles without changing how basic objects of the game relate to each other. Ersatz articles can be treated differently in the stats just as easily while still being subject to character-specific rules like index assignments. --- amanuensis/backend/article.py | 40 +++++++++-------------------------- amanuensis/backend/index.py | 1 - amanuensis/db/models.py | 11 ++++------ tests/backend/test_article.py | 39 ++++++++++------------------------ 4 files changed, 25 insertions(+), 66 deletions(-) diff --git a/amanuensis/backend/article.py b/amanuensis/backend/article.py index 187e9d0..84d2572 100644 --- a/amanuensis/backend/article.py +++ b/amanuensis/backend/article.py @@ -2,8 +2,6 @@ Article query interface """ -from typing import Optional - from sqlalchemy import select from amanuensis.db import * @@ -13,8 +11,8 @@ from amanuensis.errors import ArgumentError, BackendArgumentTypeError def create( db: DbContext, lexicon_id: int, - user_id: int, - character_id: Optional[int], + character_id: int, + ersatz: bool = False, ) -> Article: """ Create a new article in a lexicon. @@ -22,39 +20,21 @@ def create( # Verify argument types are correct if not isinstance(lexicon_id, int): raise BackendArgumentTypeError(int, lexicon_id=lexicon_id) - if not isinstance(user_id, int): - raise BackendArgumentTypeError(int, user_id=user_id) if character_id is not None and not isinstance(character_id, int): raise BackendArgumentTypeError(int, character_id=character_id) - # Check that the user is a member of this lexicon - mem: Membership = db( - select(Membership) - .where(Membership.user_id == user_id) - .where(Membership.lexicon_id == lexicon_id) + # Check that the character belongs to the lexicon + character: Character = db( + select(Character).where(Character.id == character_id) ).scalar_one_or_none() - if not mem: - raise ArgumentError("User is not a member of lexicon") - - # If the character id is provided, check that the user owns the character - # and the character belongs to the lexicon - if character_id is not None: - character: Character = db( - select(Character).where(Character.id == character_id) - ).scalar_one_or_none() - if not character: - raise ArgumentError("Character does not exist") - if character.user.id != user_id: - raise ArgumentError("Character is owned by the wrong player") - if character.lexicon.id != lexicon_id: - raise ArgumentError("Character belongs to the wrong lexicon") - signature = character.signature - else: - signature = "~Ersatz Scrivener" + if not character: + raise ArgumentError("Character does not exist") + if character.lexicon.id != lexicon_id: + raise ArgumentError("Character belongs to the wrong lexicon") + signature = character.signature if not ersatz else "~Ersatz Scrivener" new_article = Article( lexicon_id=lexicon_id, - user_id=user_id, character_id=character_id, title="Article title", body=f"\n\n{signature}", diff --git a/amanuensis/backend/index.py b/amanuensis/backend/index.py index e1a64be..7a8af92 100644 --- a/amanuensis/backend/index.py +++ b/amanuensis/backend/index.py @@ -83,7 +83,6 @@ def get_for_lexicon(db: DbContext, lexicon_id: int) -> Sequence[ArticleIndex]: ).scalars() - def update(db: DbContext, lexicon_id: int, indices: Sequence[ArticleIndex]) -> None: """ Update the indices for a lexicon. Indices are matched by type and pattern. diff --git a/amanuensis/db/models.py b/amanuensis/db/models.py index e972e31..fa99e04 100644 --- a/amanuensis/db/models.py +++ b/amanuensis/db/models.py @@ -99,7 +99,6 @@ class User(ModelBase): memberships = relationship("Membership", back_populates="user") characters = relationship("Character", back_populates="user") - articles = relationship("Article", back_populates="user") posts = relationship("Post", back_populates="user") ######################### @@ -393,11 +392,7 @@ class Article(ModelBase): lexicon_id = Column(Integer, ForeignKey("lexicon.id"), nullable=False) # The character who is the author of this article - # If this is NULL, the article is written by Ersatz Scrivener - character_id = Column(Integer, ForeignKey("character.id"), nullable=True) - - # The user who owns this article - user_id = Column(Integer, ForeignKey("user.id"), nullable=False) + character_id = Column(Integer, ForeignKey("character.id"), nullable=False) # The article to which this is an addendum addendum_to = Column(Integer, ForeignKey("article.id"), nullable=True) @@ -416,6 +411,9 @@ class Article(ModelBase): # The number of times the article has been submitted submit_nonce = Column(Integer, nullable=False, default=0) + # Whether the article is an Ersatz Scrivener article + ersatz = Column(Boolean, nullable=False, default=False) + #################### # History tracking # #################### @@ -449,7 +447,6 @@ class Article(ModelBase): lexicon = relationship("Lexicon", back_populates="articles") character = relationship("Character", back_populates="articles") - user = relationship("User", back_populates="articles") addenda = relationship("Article", backref=backref("parent", remote_side=[id])) diff --git a/tests/backend/test_article.py b/tests/backend/test_article.py index 269a898..41c7864 100644 --- a/tests/backend/test_article.py +++ b/tests/backend/test_article.py @@ -16,43 +16,26 @@ def test_create_article(db: DbContext, make: ObjectFactory): lexicon1: Lexicon = make.lexicon() make.membership(user_id=user1.id, lexicon_id=lexicon1.id) make.membership(user_id=user2.id, lexicon_id=lexicon1.id) - char1_1: Character = make.character(lexicon_id=lexicon1.id, user_id=user1.id) - char1_2: Character = make.character(lexicon_id=lexicon1.id, user_id=user2.id) + char_l1_u1: Character = make.character(lexicon_id=lexicon1.id, user_id=user1.id) + char_l1_u2: Character = make.character(lexicon_id=lexicon1.id, user_id=user2.id) # Create a lexicon that only one user is in lexicon2: Lexicon = make.lexicon() make.membership(user_id=user2.id, lexicon_id=lexicon2.id) - char2_2: Character = make.character(lexicon_id=lexicon2.id, user_id=user2.id) + char_l2_u2: Character = make.character(lexicon_id=lexicon2.id, user_id=user2.id) - # User cannot create article for another user's character + # Characters can't own articles in other lexicons with pytest.raises(ArgumentError): - artiq.create(db, lexicon1.id, user1.id, char1_2.id) + artiq.create(db, lexicon1.id, char_l2_u2.id) with pytest.raises(ArgumentError): - artiq.create(db, lexicon1.id, user2.id, char1_1.id) - - # User cannot create article for their character in the wrong lexicon + artiq.create(db, lexicon2.id, char_l1_u1.id) with pytest.raises(ArgumentError): - artiq.create(db, lexicon1.id, user2.id, char2_2.id) - with pytest.raises(ArgumentError): - artiq.create(db, lexicon2.id, user2.id, char1_2.id) - - # User cannot create article in a lexicon they aren't in - with pytest.raises(ArgumentError): - artiq.create(db, lexicon2.id, user1.id, char1_1.id) - - # User cannot create anonymous articles in a lexicon they aren't in - with pytest.raises(ArgumentError): - artiq.create(db, lexicon2.id, user1.id, character_id=None) + artiq.create(db, lexicon2.id, char_l1_u2.id) # Users can create character-owned articles - assert artiq.create(db, lexicon1.id, user1.id, char1_1.id) - assert artiq.create(db, lexicon1.id, user2.id, char1_2.id) - assert artiq.create(db, lexicon2.id, user2.id, char2_2.id) - - # Users can create anonymous articles - assert artiq.create(db, lexicon1.id, user1.id, character_id=None) - assert artiq.create(db, lexicon1.id, user2.id, character_id=None) - assert artiq.create(db, lexicon2.id, user2.id, character_id=None) + assert artiq.create(db, lexicon1.id, char_l1_u1.id) + assert artiq.create(db, lexicon1.id, char_l1_u2.id) + assert artiq.create(db, lexicon2.id, char_l2_u2.id) def test_article_update_ts(db: DbContext, make: ObjectFactory): @@ -61,7 +44,7 @@ def test_article_update_ts(db: DbContext, make: ObjectFactory): lexicon: Lexicon = make.lexicon() make.membership(user_id=user.id, lexicon_id=lexicon.id) char: Character = make.character(lexicon_id=lexicon.id, user_id=user.id) - article = artiq.create(db, lexicon.id, user.id, char.id) + article = artiq.create(db, lexicon.id, char.id) created = article.last_updated time.sleep(1) # The update timestamp has only second-level precision article.title = "New title, who dis"