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..2219f63f4 100644 --- a/src/articles/ClassroomArticles.js +++ b/src/articles/ClassroomArticles.js @@ -83,6 +83,16 @@ export default function ClassroomArticles() { ); } + // 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 0647f19f4..37def7e96 100644 --- a/src/teacher/myClassesPage/cohortsPage/ClassTexts.js +++ b/src/teacher/myClassesPage/cohortsPage/ClassTexts.js @@ -44,6 +44,21 @@ 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. + // + // 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); + const nothingShared = texts.length === 0; const emptyForEveryone = nothingShared && cohort.only_classroom_texts; @@ -69,6 +84,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. )} 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); + }); +});