The classroom's rules, in one file — and a table that checks them

zeeguu/core/classroom/__init__.py

@@ -0,0 +1,18 @@
+from zeeguu.core.classroom.queries import (
+    classes_of,
+    classroom_of,
+    hidden_from,
+    sees_only_class_texts,
+    texts_of,
+)
+from zeeguu.core.classroom.rules import app_is_reduced_to_classroom, student_can_read
+
+__all__ = [
+    "app_is_reduced_to_classroom",
+    "classes_of",
+    "classroom_of",
+    "hidden_from",
+    "sees_only_class_texts",
+    "student_can_read",
+    "texts_of",
+]

zeeguu/core/classroom/queries.py

@@ -0,0 +1,68 @@
+"""Finding things in the classroom.
+
+Everything here is implementation: it walks the model, applies the rules from
+classroom/rules.py, and hands back the answer. No policy is decided in this
+file -- a condition about what a student may see belongs next door.
+"""
+
+from zeeguu.core.classroom.rules import app_is_reduced_to_classroom, student_can_read
+
+
+def classes_of(user):
+    """The classes a user is enrolled in.
+
+    Enrolment, not ownership: a teacher who owns a class is not in it unless
+    they joined it like a student.
+    """
+    from zeeguu.core.model.cohort import Cohort
+
+    return [Cohort.find(membership.cohort_id) for membership in user.cohorts]
+
+
+def texts_of(cohort):
+    """Every text a teacher has shared with this class, in any language."""
+    from zeeguu.core.model.cohort_article_map import CohortArticleMap
+
+    return CohortArticleMap.get_articles_for_cohort(cohort)
+
+
+def classroom_of(student):
+    """The texts a student sees, each with the classes it reached them through.
+
+    One merged list: a student in several classes has one classroom, not one
+    per class, and a text shared with two of them appears once naming both.
+
+    Returns [(Article, [Cohort])], in the order the classes were joined.
+    """
+    reached_through = {}
+    order = []
+
+    for cohort in classes_of(student):
+        for text in texts_of(cohort):
+            if not student_can_read(student, text):
+                continue
+            if text.id not in reached_through:
+                order.append(text)
+                reached_through[text.id] = []
+            reached_through[text.id].append(cohort)
+
+    return [(text, reached_through[text.id]) for text in order]
+
+
+def hidden_from(student):
+    """The texts in the student's classes that they cannot open.
+
+    The reason a class can look full to its teacher and empty to half of its
+    students.
+    """
+    return [
+        text
+        for cohort in classes_of(student)
+        for text in texts_of(cohort)
+        if not student_can_read(student, text)
+    ]
+
+
+def sees_only_class_texts(user):
+    """Whether the app is reduced to the classroom for this user."""
+    return app_is_reduced_to_classroom(user, classes_of(user))

zeeguu/core/classroom/rules.py

@@ -0,0 +1,44 @@
+"""The classroom's rules. Nothing else lives here.
+
+Each function below is a policy: it answers "is this allowed?" from values it
+is given, and touches no database. That is the point of the file -- you can
+read it end to end in under a minute, and there is nowhere for behaviour to
+hide, because there is no traversal, no query and no I/O.
+
+Anything that has to *find* things -- which classes a student is in, which
+texts are in them -- is a query and lives in classroom/queries.py. Those call
+these; these never call those.
+
+test_classroom_visibility_rules.py states the same rules as a table of worlds
+and expected outcomes.
+"""
+
+
+def student_can_read(student, text):
+    """A student reads only in the language they are currently learning.
+
+    The student's own setting decides, not the class's -- which is why a Greek
+    learner in an English class sees an empty classroom, and why a class can
+    hold texts that nobody in it can open.
+    """
+    if student.learned_language is None:
+        return False
+    return text.language_id == student.learned_language_id
+
+
+def app_is_reduced_to_classroom(user, classes):
+    """Whether this user gets only the classroom: no feed, no search, no inbox.
+
+    A teacher sets it per class. A student in several classes is restricted if
+    any one of them asks for it -- the strictest class wins, and the way out is
+    to leave that class.
+
+    Teachers are never restricted, even in a class that asks for it: they need
+    the feed and the search to find the texts they are going to share. This is
+    why a teacher clicking "Student Site" gets the full app, and cannot see
+    what their own students see.
+    """
+    if user.isTeacher():
+        return False
+
+    return any(cohort.only_classroom_texts for cohort in classes)

zeeguu/core/model/user.py

@@ -551,48 +551,31 @@ def add_user_to_cohort(self, cohort, session):
         session.commit()
 
     def cohort_articles_for_user(self):
-        """The class texts this student can see, each tagged with its class.
+        """The classroom, as the API sends it.
 
-        A student in more than one class gets one merged list, so each text
-        says which class it came from -- otherwise the list is a pile with no
-        provenance. The tag is only useful to a student with several classes;
-        the client drops it for everyone else.
+        The rules for what a student sees live in zeeguu.core.classroom; this
+        only turns the answer into article infos and tags each one with the
+        classes it came from.
         """
-        from zeeguu.core.model import Cohort, CohortArticleMap, UserArticle
+        from zeeguu.core.classroom import classroom_of
+        from zeeguu.core.model import UserArticle
 
-        all_articles = []
-        classes_by_article = {}
         try:
-            # Filter articles by the user's learned language: a student only
-            # ever reads in the language they are currently learning, so a
-            # class taught in another one has nothing to show them.
-            user_language_id = (
-                self.learned_language_id if self.learned_language else None
-            )
-
-            for c in self.cohorts:
-                cohort = Cohort.find(c.cohort_id)
-                for article in CohortArticleMap.get_articles_for_cohort(cohort):
-                    if article.language_id != user_language_id:
-                        continue
-                    # A text shared with two of this student's classes appears
-                    # once, tagged with both.
-                    if article.id not in classes_by_article:
-                        all_articles.append(article)
-                        classes_by_article[article.id] = []
-                    classes_by_article[article.id].append(
-                        {"id": cohort.id, "name": cohort.name}
-                    )
-
-            infos = UserArticle.article_infos(
-                self, all_articles, select_appropriate=False
-            )
-            for info in infos:
-                info["from_classes"] = classes_by_article.get(info["id"], [])
-            return infos
-        except NoResultFound as e:
+            visible = classroom_of(self)
+        except NoResultFound:
             return []
 
+        infos = UserArticle.article_infos(
+            self, [text for text, _ in visible], select_appropriate=False
+        )
+        classes_by_text = {
+            text.id: [{"id": c.id, "name": c.name} for c in cohorts]
+            for text, cohorts in visible
+        }
+        for info in infos:
+            info["from_classes"] = classes_by_text.get(info["id"], [])
+        return infos
+
     def isTeacher(self):
         from zeeguu.core.model import Teacher
 

zeeguu/core/test/test_classroom_visibility_rules.py

@@ -0,0 +1,123 @@
+"""What a student may see in their classroom, as a table.
+
+Every row is one world, and what a student in it sees. These rules are spread
+across cohort_articles_for_user, _classroom_only and the article filters; no
+one file states them, which is why they keep being got wrong. State them here
+and let the real code answer.
+
+Read a row as:
+
+    a student LEARNING this language, enrolled in these CLASSES,
+    sees SEES texts, and their app is in MODE.
+
+In CLASSES, the value is the language of each text in that class, so
+"en en da" is a class holding two English texts and one Danish. A class name
+ending in ! has "students see only the texts I share" switched on.
+"""
+
+from unittest import TestCase
+
+import zeeguu.core
+from zeeguu.core.model.cohort import Cohort
+from zeeguu.core.model.cohort_article_map import CohortArticleMap
+from zeeguu.core.model.teacher import Teacher
+from zeeguu.core.test.model_test_mixin import ModelTestMixIn
+from zeeguu.core.test.rules.article_rule import ArticleRule
+from zeeguu.core.test.rules.language_rule import LanguageRule
+from zeeguu.core.test.rules.user_rule import UserRule
+from zeeguu.core.user_feature_toggles import is_feature_enabled_for_user
+
+db_session = zeeguu.core.model.db.session
+
+FULL_APP = "full app"
+CLASSROOM_ONLY = "classroom only"
+
+#        learning  classes                                sees  mode
+RULES = [
+    ("the ordinary case",
+         "da",     {"Danish Class": "da da da"},            3,   FULL_APP),
+
+    ("a class in another language is invisible, not empty",
+         "da",     {"English Class": "en en"},              0,   FULL_APP),
+
+    ("a class may hold texts the student cannot see",
+         "da",     {"Mixed Class": "da en en"},             1,   FULL_APP),
+
+    ("two classes merge into one list",
+         "da",     {"A": "da", "B": "da"},                  2,   FULL_APP),
+
+    ("a text in two of the student's classes is listed once",
+         "da",     {"A": "shared", "B": "shared"},          1,   FULL_APP),
+
+    ("one restricted class restricts the whole app",
+         "da",     {"A": "da da", "B!": "da"},              3,   CLASSROOM_ONLY),
+
+    ("Jack's bug: restricted, and every text is in another language",
+         "de",     {"CUT!": "en en en en en"},              0,   CLASSROOM_ONLY),
+
+    ("a student with no language set sees nothing",
+         None,     {"Danish Class": "da da"},               0,   FULL_APP),
+]
+
+
+class ClassroomVisibilityRulesTest(ModelTestMixIn, TestCase):
+    def _language(self, code):
+        return LanguageRule.get_or_create_language(code)
+
+    _next_code = 0
+
+    def _world(self, learning, classes):
+        student = UserRule().user
+        if learning:
+            student.set_learned_language(learning, session=db_session)
+        else:
+            student.learned_language = None
+
+        shared_article = None
+        for name, text_languages in classes.items():
+            # invite codes are unique across the whole table, and class names
+            # repeat between rows
+            ClassroomVisibilityRulesTest._next_code += 1
+            cohort = Cohort(f"code-{ClassroomVisibilityRulesTest._next_code}",
+                            name.rstrip("!"),
+                            self._language("da"), 10,
+                            only_classroom_texts=name.endswith("!"))
+            db_session.add(cohort)
+            student.add_user_to_cohort(cohort, db_session)
+
+            for code in text_languages.split():
+                if code == "shared":
+                    # the same article, deliberately, in both classes
+                    if shared_article is None:
+                        shared_article = ArticleRule().article
+                        shared_article.language = self._language("da")
+                    article = shared_article
+                else:
+                    article = ArticleRule().article
+                    article.language = self._language(code)
+                db_session.add(article)
+                db_session.add(CohortArticleMap(cohort, article, None))
+        db_session.commit()
+        return student
+
+    def test_every_rule(self):
+        for label, learning, classes, sees, mode in RULES:
+            with self.subTest(label):
+                student = self._world(learning, classes)
+
+                self.assertEqual(
+                    sees, len(student.cohort_articles_for_user()),
+                    f"{label}: wrong number of texts visible")
+
+                restricted = is_feature_enabled_for_user("classroom_only", student)
+                self.assertEqual(
+                    mode, CLASSROOM_ONLY if restricted else FULL_APP,
+                    f"{label}: wrong mode")
+
+    def test_a_teacher_is_never_restricted(self):
+        # Stated separately because it is about who you are, not what you see.
+        teacher = self._world("da", {"A!": "da"})
+        db_session.add(Teacher(teacher))
+        db_session.commit()
+
+        self.assertFalse(is_feature_enabled_for_user("classroom_only", teacher))

zeeguu/core/user_feature_toggles.py

@@ -73,7 +73,13 @@ def _new_topics(user):
 
 def _tiago_exercises(user):
     right_user = user.invitation_code == "Tiago" or user.id == 534 or user.id == 4022
-    right_language = user.learned_language.code in ["da"]
+    # Guarded: features_for_user runs on every /user_details, so an account
+    # without a learned language would take the whole call down. No such
+    # account exists today, but the model allows one and the classroom code
+    # already handles it.
+    right_language = (
+        user.learned_language is not None and user.learned_language.code in ["da"]
+    )
     return right_user and right_language
 
 
@@ -122,26 +128,12 @@ def _always_open_externally(user):
 def _classroom_only(user):
     """The student sees only the texts their teacher shares with the class.
 
-    Set per class by the teacher (Cohort.only_classroom_texts). A student in
-    several classes is restricted if any one of them asks for it: the strictest
-    class wins, and the way out is to leave that class.
-
-    Teachers are excluded even when they are members of such a cohort -- they
-    need the feed and search to find the texts they are going to share.
+    The rule itself is in zeeguu.core.classroom, next to the rest of the
+    classroom's behaviour.
     """
-    if user.isTeacher():
-        return False
+    from zeeguu.core.classroom import sees_only_class_texts
 
-    cohort_ids = [user_cohort.cohort_id for user_cohort in user.cohorts]
-    if not cohort_ids:
-        return False
-
-    return (
-        Cohort.query.filter(Cohort.id.in_(cohort_ids))
-        .filter(Cohort.only_classroom_texts.is_(True))
-        .count()
-        > 0
-    )
+    return sees_only_class_texts(user)
 
 
 def _verbal_flashcards(user):