diff --git a/zeeguu/core/classroom/__init__.py b/zeeguu/core/classroom/__init__.py new file mode 100644 index 000000000..f827a7705 --- /dev/null +++ b/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", +] diff --git a/zeeguu/core/classroom/queries.py b/zeeguu/core/classroom/queries.py new file mode 100644 index 000000000..2a6af68d1 --- /dev/null +++ b/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)) diff --git a/zeeguu/core/classroom/rules.py b/zeeguu/core/classroom/rules.py new file mode 100644 index 000000000..261b772bb --- /dev/null +++ b/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) diff --git a/zeeguu/core/model/user.py b/zeeguu/core/model/user.py index ecfdc416c..0a1a5e48a 100644 --- a/zeeguu/core/model/user.py +++ b/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 diff --git a/zeeguu/core/test/test_classroom_visibility_rules.py b/zeeguu/core/test/test_classroom_visibility_rules.py new file mode 100644 index 000000000..f3e3304d8 --- /dev/null +++ b/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)) diff --git a/zeeguu/core/user_feature_toggles.py b/zeeguu/core/user_feature_toggles.py index 83b2c818a..63d1f6f44 100644 --- a/zeeguu/core/user_feature_toggles.py +++ b/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):