hide users removed from the class

src/clue/components/clue-app-header.tsx

@@ -82,7 +82,7 @@ export const ClueAppHeaderComponent: React.FC<IProps> = observer(function ClueAp
   };
 
   const renderGroup = (group: GroupModelType) => {
-    const groupUsers = group.users.slice();
+    const groupUsers = group.activeUsers.slice();
     const userIndex = groupUsers.findIndex((groupUser) => groupUser.id === user.id);
     // Put the main user first to match 4-up colors
     if (userIndex > -1) {

src/clue/components/teacher/teacher-student-tab.tsx

@@ -32,7 +32,7 @@ export class TeacherStudentTabComponent extends BaseComponent<IProps, IState> {
     const { groupId } = this.props;
     const group = groups.getGroupById(groupId);
     // TODO: if no group prop then get list of all users in class
-    const users = group ? group.users : [];
+    const users = group ? group.activeUsers : [];
     return (
       <div className="teacher-student-tab">
         {this.renderUsers(users)}

src/components/four-up.test.tsx

@@ -77,6 +77,7 @@ describe("Four Up Component", () => {
     const clazz = ClassModel.create({
       name: "Test Class",
       classHash: "test",
+      timestamp: 4000,
       users: {
         1: {
           type: "student",
@@ -86,14 +87,6 @@ describe("Four Up Component", () => {
           fullName: "User 1",
           initials: "U1"
         },
-        2: {
-          type: "student",
-          id: "2",
-          firstName: "User",
-          lastName: "2",
-          fullName: "User 2",
-          initials: "U2"
-        },
         3: {
           type: "student",
           id: "3",
@@ -105,23 +98,37 @@ describe("Four Up Component", () => {
       }
     });
 
+    // TODO: add a test of how removed users are not shown
     const group = GroupModel.create({
       id: "1",
       users: [
         GroupUserModel.create({
           id: "1",
           connectedTimestamp: 1,
         }),
+        // This user doesn't exist in the class and they their connectedTimestamp is
+        // after the timestamp of the class. The code treats this student as being
+        // added from the class. So it should be shown with the initials "**". At
+        // runtime this will trigger a refresh of the class so the "**" should
+        // quickly be replaced with the real initials.
         GroupUserModel.create({
           id: "2",
-          connectedTimestamp: 1,
-          disconnectedTimestamp: 2,
+          connectedTimestamp: 10000,
+          disconnectedTimestamp: 10001,
         }),
         GroupUserModel.create({
           id: "3",
           connectedTimestamp: 3,
           disconnectedTimestamp: 2,
         }),
+        // This user doesn't exist in the class and they their connectedTimestamp is
+        // before the timestamp of the class. The code treats this student as being
+        // removed from the class. So it shouldn't be shown.
+        GroupUserModel.create({
+          id: "4",
+          connectedTimestamp: 4,
+          disconnectedTimestamp: 3,
+        }),
       ],
     });
     const groups = GroupsModel.create({
@@ -136,15 +143,16 @@ describe("Four Up Component", () => {
     const { container } = render(<FourUpComponent group={realGroup} stores={stores}/>);
     // A canvas will be rendered unless an "unshared document" message is displayed.
     // User 2 has no document, so it will display an "unshared document" message.
-    // User 1 has a shared document, User 3 is the main user, and there is no fourth user. All of those show canvases.
+    // User 1 has a shared document, User 3 is the main user, and there is no active fourth user.
+    // All of those show canvases.
     expect(screen.queryAllByTestId("canvas")).toHaveLength(3);
     // Users 1, 2 and 3 should be labelled
     const memberList = container.querySelectorAll(".member");
     expect(memberList).toHaveLength(3);
     // First member is the current user, followed by group members
     expect(memberList[0].textContent).toBe("U3");
     expect(memberList[1].textContent).toBe("U1");
-    expect(memberList[2].textContent).toBe("U2");
+    expect(memberList[2].textContent).toBe("**");
 
     // TODO: figure out how to add coverage for window mouse events setup by the splitter handlers
     userEvent.click(screen.getByTestId("4up-horizontal-splitter"));

src/components/four-up.tsx

@@ -80,7 +80,7 @@ export function getFocusedGroupUser(group: GroupModelType| undefined, openDocId:
     mode: DocumentViewMode | undefined) {
   if (!openDocId || !group) return undefined;
 
-  return group?.users.find(obj => {
+  return group?.activeUsers.find(obj => {
     const userDoc = getUserDocument(obj, mode);
     return userDoc?.key === openDocId;
   });
@@ -144,7 +144,7 @@ export class FourUpComponent extends BaseComponent<IProps, IState> {
   private getFocusedGroupUser() {
     const {group} = this.props;
     const docKey = this.getFocusedUserDocKey();
-    return group.users.find(obj => docKey && this.getGroupUserDoc(obj)?.key === docKey);
+    return group.activeUsers.find(obj => docKey && this.getGroupUserDoc(obj)?.key === docKey);
   }
 
   private getGroupUserDoc(groupUser?: GroupUserModelType) {

src/components/group/group-chooser.tsx

@@ -74,7 +74,7 @@ export class GroupChooserComponent extends BaseComponent<IProps, IState> {
   private renderChooseExistingGroup() {
     const {groups} = this.stores;
     const groupElements = groups.allGroups.map((group) => {
-      const users = group.users.map((user) => {
+      const users = group.activeUsers.map((user) => {
         const className = `user ${user.connected ? "connected" : "disconnected"}`;
         const title = `${user.name}: ${user.connected ? "connected" : "disconnected"}`;
         return <span key={user.id} className={className} title={title}>{user.initials}</span>;
@@ -121,7 +121,7 @@ export class GroupChooserComponent extends BaseComponent<IProps, IState> {
   private handleChooseExistingGroup = (group: GroupModelType) => {
     return (e: React.MouseEvent<HTMLElement>) => {
       e.preventDefault();
-      if (group.users.length >= 4) {
+      if (group.activeUsers.length >= 4) {
         this.setState({error: "Sorry, that group is full with four students"});
       }
       else {

src/lib/db.ts

@@ -518,7 +518,7 @@ export class DB {
       this.createDocument({ type: ProblemPublication, content }).then(({document, metadata}) => {
         const publicationRef = this.firebase.ref(this.firebase.getProblemPublicationsPath(user)).push();
         const userGroup = groups.getGroupById(user.currentGroupId);
-        const groupUserConnections: DBGroupUserConnections | undefined = userGroup && userGroup.users
+        const groupUserConnections: DBGroupUserConnections | undefined = userGroup && userGroup.activeUsers
           .filter(groupUser => groupUser.id !== user.id)
           .reduce((allUsers: DBGroupUserConnections, groupUser) => {
             allUsers[groupUser.id] = groupUser.connected;

src/models/stores/groups.test.ts

@@ -141,6 +141,7 @@ describe("Groups model", () => {
     expect(groups.allGroups.length).toEqual(1);
     const group = groups.allGroups[0];
     expect(group.users.length).toEqual(3);
+    expect(group.activeUsers.length).toEqual(2);
     expect(group.users[0].id).toEqual("1");
     expect(group.users[0].name).toEqual("Test User");
     expect(group.users[0].initials).toEqual("TU");

src/models/stores/groups.ts

@@ -13,7 +13,7 @@ export interface IGroupsEnvironment {
 }
 
 export enum GroupUserState {
-  Active,
+  Found,
   New,
   Removed,
 }
@@ -41,7 +41,7 @@ export const GroupUserModel = types
     // user joins the class after the current user has started CLUE.
     get state(): GroupUserState {
       if (self.classUser) {
-        return GroupUserState.Active;
+        return GroupUserState.Found;
       }
       // We add a little padding here. Perhaps the group user is being
       // created right as the class info is downloaded. So maybe the connectedTimestamp is
@@ -102,19 +102,22 @@ export const GroupModel = types
   .views(self => ({
     get environment() {
       return getEnv(self) as IGroupsEnvironment;
+    },
+    get activeUsers() {
+      return self.users.filter(user => user.state !== GroupUserState.Removed);
     }
   }))
   .views((self) => ({
     getUserById(id?: string) {
-      return self.users.find(user => user.id === id);
+      return self.activeUsers.find(user => user.id === id);
     },
     get displayId() {
       const maxChars = 3;
       return self.id.length > maxChars ? self.id.slice(self.id.length - maxChars) : self.id;
     },
     // This will put the current user first if they are in this group
     get sortedUsers() {
-      const sortedUsers = [...self.users];
+      const sortedUsers = [...self.activeUsers];
       const userId = self.environment.user?.id;
       return sortedUsers.sort((a, b) => {
         if (a.id === userId) return -1;
@@ -137,6 +140,8 @@ export const GroupsModel = types
     get groupsByUser() {
       const groupsByUser: Record<string, GroupModelType> = {};
       self.allGroups.forEach((group) => {
+        // We don't use activeUsers here incase some code is trying to
+        // track down more info about a removed user
         group.users.forEach((groupUser) => {
           groupsByUser[groupUser.id] = group;
         });
@@ -205,15 +210,6 @@ export const GroupsModel = types
         groupsMapSnapshot[groupId] = {id: groupId, users: groupUserSnapshots};
       });
       applySnapshot(self.groupsMap, groupsMapSnapshot);
-
-      if (self.needToRefreshClass) {
-        // TODO: Request classInfo from the portal then pass it to updateFromPortal.
-        // This might be better to move somewhere else.
-        // const classInfo = {} as ClassInfo;
-        // if (classInfo) {
-        //   clazz.updateFromPortal(classInfo);
-        // }
-      }
     }
   }));