From a679e45d7f8fa01f46df0fb9e8800d658a343f7c Mon Sep 17 00:00:00 2001 From: Mircea Lungu Date: Mon, 31 Aug 2026 09:50:23 +0300 Subject: [PATCH 1/3] Show a student which class each text belongs to A student in more than one class gets one merged classroom list, and until now it was a pile of texts with no way to tell whose lesson was whose. Each text now carries its class as a tag in the metadata strip, suppressed for the 2,484 students who have exactly one class and would see the same answer on every row. ArticlePreview renders two layouts and both have their own MetaStrip, so the tags are built once next to the other tag slots rather than inside either branch. Putting them in only one is a silent no-op for the classroom, which is exactly what happened first time. The class page also tells the teacher when it holds texts in a language its students are not learning -- those texts are invisible to the whole class, and the teacher is the only person who can do anything about it. Co-Authored-By: Claude Opus 5 --- src/articles/ArticlePreview.js | 14 +++++++++++ src/articles/ClassroomArticles.js | 2 ++ .../myClassesPage/cohortsPage/ClassTexts.js | 24 +++++++++++++++++++ 3 files changed, 40 insertions(+) diff --git a/src/articles/ArticlePreview.js b/src/articles/ArticlePreview.js index e07200939..271c4ca5c 100644 --- a/src/articles/ArticlePreview.js +++ b/src/articles/ArticlePreview.js @@ -39,6 +39,9 @@ export default function ArticlePreview({ onUnhideArticle, isHiddenView = false, inSavedView = false, + // A student in several classes gets one merged classroom list, so each text + // says which class it belongs to. + showClassNames = false, // Someone is being shown this card rather than browsing it -- a teacher // previewing their class's feed. The card must look the same but do nothing // on the teacher's own account, so the personal controls come off. @@ -289,6 +292,15 @@ export default function ArticlePreview({ // tags; elsewhere publish time sits at the tail. `dontShowPublishingTime` // suppresses publish time only — the saved-time path is the replacement, // so it isn't gated on the same flag. + // Which class a text came from. Only for a student in more than one class: + // with a single class it is the same answer on every row, and the tab itself + // already says it. Computed here because both layouts below render a strip. + const classTags = showClassNames + ? (article.from_classes || []).map((each) => ( + {each.name} + )) + : null; + let savedTag = null; let publishedTimeSlot = null; if (inSavedView && article.personal_copy_saved_at) { @@ -384,6 +396,7 @@ export default function ArticlePreview({ // layouts so the meta/image markup isn't duplicated across the two. const metaStrip = ( + {classTags} {article.topics_list && article.topics_list.map(([topicTitle]) => {topicTitle})} {article.matched_searches && @@ -625,6 +638,7 @@ export default function ArticlePreview({ Saved · source · time. State badges (Simplified/Saved) get a subtle accent color; source/time stay muted. All on one row, small. */} + {classTags} {article.topics_list && article.topics_list.map(([topicTitle]) => {topicTitle})} {article.matched_searches && diff --git a/src/articles/ClassroomArticles.js b/src/articles/ClassroomArticles.js index 0e8a374b4..b48ba7cbc 100644 --- a/src/articles/ClassroomArticles.js +++ b/src/articles/ClassroomArticles.js @@ -83,6 +83,8 @@ export default function ClassroomArticles() { ); } + const inMoreThanOneClass = (student?.cohorts?.length || 0) > 1; + return ( <>
diff --git a/src/teacher/myClassesPage/cohortsPage/ClassTexts.js b/src/teacher/myClassesPage/cohortsPage/ClassTexts.js index 0647f19f4..32978873b 100644 --- a/src/teacher/myClassesPage/cohortsPage/ClassTexts.js +++ b/src/teacher/myClassesPage/cohortsPage/ClassTexts.js @@ -44,6 +44,15 @@ export default function ClassTexts() { const { cohort, texts, student_count } = overview; + // A text in another language can be shared with this class, and then no + // student ever sees it: they only get the texts in the language they are + // learning. Seven classes are in this state today. The teacher is the one + // who can fix it, so the count goes to them rather than to the students. + const offLanguage = (overview.texts_by_language || []).filter( + (each) => each.code !== cohort.language, + ); + const offLanguageCount = offLanguage.reduce((total, each) => total + each.count, 0); + const nothingShared = texts.length === 0; const emptyForEveryone = nothingShared && cohort.only_classroom_texts; @@ -69,6 +78,21 @@ export default function ClassTexts() { )} + {offLanguageCount > 0 && ( + + + {offLanguageCount === 1 ? "One text here is" : `${offLanguageCount} texts here are`} not + in {languageName(cohort.language)} —{" "} + {offLanguage + .map((each) => `${each.count} in ${languageName(each.code)}`) + .join(", ")} + . Your students only ever see the texts in the language they are + learning, so nobody in this class can open{" "} + {offLanguageCount === 1 ? "it" : "them"}. + + + )} + {nothingShared && !emptyForEveryone && ( No texts shared with this class yet. )} From 586e7254e1b771191313652265ce94a2e5218b4b Mon Sep 17 00:00:00 2001 From: Mircea Lungu Date: Mon, 31 Aug 2026 13:39:03 +0300 Subject: [PATCH 2/3] Don't warn a class with no language that all its texts are hidden Review follow-ups. - `cohort.language` is null for a class whose language was never set, and every language then failed the "is this off-language?" test, so the banner claimed the whole list was unreachable and left a hole in its own sentence where the language name should be. Both halves were false: the student filter compares the *article's* language to the *student's* learned language and never consults the class's. Six such classes exist; none holds a text yet, which is the only reason this was not already visible. - The classroom waited for getStudent before choosing an empty-state message but not before rendering the list, so the class tags -- which need to know whether the student has more than one class -- rendered absent and then appeared when that request landed. It waits in both paths now. Co-Authored-By: Claude Opus 5 --- src/articles/ClassroomArticles.js | 10 +++++++++- src/teacher/myClassesPage/cohortsPage/ClassTexts.js | 8 +++++++- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/src/articles/ClassroomArticles.js b/src/articles/ClassroomArticles.js index b48ba7cbc..2219f63f4 100644 --- a/src/articles/ClassroomArticles.js +++ b/src/articles/ClassroomArticles.js @@ -83,7 +83,15 @@ export default function ClassroomArticles() { ); } - const inMoreThanOneClass = (student?.cohorts?.length || 0) > 1; + // The class tags need to know how many classes this student has, and that + // arrives on a separate request. Waiting is better than rendering the list + // untagged and having the tags pop in when getStudent lands -- the empty + // path above waits for the same reason. + if (student === null) { + return ; + } + + const inMoreThanOneClass = student.cohorts.length > 1; return ( <> diff --git a/src/teacher/myClassesPage/cohortsPage/ClassTexts.js b/src/teacher/myClassesPage/cohortsPage/ClassTexts.js index 32978873b..37def7e96 100644 --- a/src/teacher/myClassesPage/cohortsPage/ClassTexts.js +++ b/src/teacher/myClassesPage/cohortsPage/ClassTexts.js @@ -48,7 +48,13 @@ export default function ClassTexts() { // student ever sees it: they only get the texts in the language they are // learning. Seven classes are in this state today. The teacher is the one // who can fix it, so the count goes to them rather than to the students. - const offLanguage = (overview.texts_by_language || []).filter( + // + // Only meaningful once the class has a language: without one there is + // nothing for a text to be "off", and treating null as a language would + // declare every text unreachable -- both alarming and untrue, since the + // student filter compares the article's language to the student's own and + // never consults the class's. + const offLanguage = (cohort.language ? overview.texts_by_language || [] : []).filter( (each) => each.code !== cohort.language, ); const offLanguageCount = offLanguage.reduce((total, each) => total + each.count, 0); From 81b029e267d0f1db2406c8d8d91e305d25e1bb47 Mon Sep 17 00:00:00 2001 From: Mircea Lungu Date: Mon, 31 Aug 2026 13:46:55 +0300 Subject: [PATCH 3/3] Compare cohort ids as strings where a route param meets a payload The cohort id is a number in every payload now (api#719), but useParams() always hands back a string, so the two places that matched one against the other were relying on the API's odd stringification to work at all. Adds the regression this closes: a class shared during a session (ids from /cohorts_info) and the same class after a reload (ids from /teacher_texts) used to build two separate filter chips. Co-Authored-By: Claude Opus 5 --- .../cohortsPage/StudentsActivityOverview.js | 6 +++++- .../readingPage/StudentReadingInsights.js | 5 ++++- test/teacher/textFilters.test.js | 16 ++++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/teacher/myClassesPage/cohortsPage/StudentsActivityOverview.js b/src/teacher/myClassesPage/cohortsPage/StudentsActivityOverview.js index f81a9ba7a..a311578bd 100644 --- a/src/teacher/myClassesPage/cohortsPage/StudentsActivityOverview.js +++ b/src/teacher/myClassesPage/cohortsPage/StudentsActivityOverview.js @@ -32,7 +32,11 @@ export default function StudentsActivityOverview() { //Extracting the cohort data for the page title - for showing "no students" guidance and for deleting students from the cohort. useEffect(() => { api.getCohortsInfo((res) => { - const currentCohortArray = res.filter((cohort) => cohort.id === cohortID); + // cohortID comes from the route, so it is a string; the payload's id is + // a number. Compare as strings rather than relying on either shape. + const currentCohortArray = res.filter( + (cohort) => String(cohort.id) === String(cohortID), + ); setCohort(currentCohortArray[0]); }); //eslint-disable-next-line diff --git a/src/teacher/myClassesPage/readingPage/StudentReadingInsights.js b/src/teacher/myClassesPage/readingPage/StudentReadingInsights.js index 45a49d1aa..2c2152688 100644 --- a/src/teacher/myClassesPage/readingPage/StudentReadingInsights.js +++ b/src/teacher/myClassesPage/readingPage/StudentReadingInsights.js @@ -25,7 +25,10 @@ export default function StudentReadingInsights() { useEffect(() => { api.getCohortsInfo((cohortInfo) => { - let currentCohort = cohortInfo.find((each) => each.id === cohortID); + // Route params are strings; the payload's cohort id is a number. + let currentCohort = cohortInfo.find( + (each) => String(each.id) === String(cohortID), + ); setCohortLang(currentCohort.language_name); }); api.getStudentInfo( diff --git a/test/teacher/textFilters.test.js b/test/teacher/textFilters.test.js index 1ba39986a..afc9eb571 100644 --- a/test/teacher/textFilters.test.js +++ b/test/teacher/textFilters.test.js @@ -68,3 +68,19 @@ describe("sharedClassesOf", () => { expect(sharedClassesOf({ cohorts: ["nana"] })).toEqual([]); }); }); + +describe("cohort ids arriving from two endpoints", () => { + // /teacher_texts sends shared_with with numeric ids; the share dialog builds + // its objects from /cohorts_info. Those used to be stringified, so a text + // shared in this session and the same text after a reload produced two + // different chips for one class. + test("a class shared just now and the same class after a reload are one chip", () => { + const justShared = { id: 3, shared_with: [{ id: 7, name: "nana" }] }; + const afterReload = { id: 4, shared_with: [{ id: 7, name: "nana" }] }; + + const filters = buildClassFilters([justShared, afterReload]); + + expect(filters.filter((each) => each.name === "nana")).toHaveLength(1); + expect(filters.find((each) => each.name === "nana").count).toBe(2); + }); +});