Lane 1: roster visibility rules + hide-yourself privacy flag (gaps 04, 05, 08)

api/src/controllers/member.controller.ts

@@ -6,7 +6,7 @@ import validator from 'validator';
 import * as badwords from 'badwords-list';
 
 import { sendPasswordResetEmail, sendPasswordResetUnknownEmail } from '../libs';
-import { MemberService, HomeService, PlaceService } from '../services';
+import { MemberService, HomeService, PlaceService, MemberDataService } from '../services';
 import { SessionInfo } from 'session-info.interface';
 import {parseInt} from 'lodash';
 
@@ -31,7 +31,8 @@ class MemberController {
   constructor(
     private memberService: MemberService, 
     private homeService: HomeService,
-    private placeService: PlaceService) {}
+    private placeService: PlaceService,
+    private memberDataService: MemberDataService) {}
 
   public async getAdminLevel(request: Request, response: Response): Promise<object> {
     const session = this.memberService.decryptSession(request, response);
@@ -462,29 +463,91 @@ class MemberController {
     }
   }
 
-  public async getOnlineUsers(request: Request, response: Response): Promise<any> {
+  /**
+   * Reads the caller's hide-yourself flag.
+   *
+   * One boolean, because one checkbox (IMS) is the whole privacy model in the original --
+   * no per-buddy blocking, no appear-offline-to-some, no ignore list.
+   */
+  public async getPrivacy(request: Request, response: Response): Promise<void> {
     const session = this.memberService.decryptSession(request, response);
     if (!session) return;
     try {
-      const returnUsers = [];
-      const users = await this.memberService.getOnlineUsers();
-      for (const user of users) {
-        const hasHome = await this.homeService.getHome(user.id);
-        const accessLevel = await this.memberService.getAccessLevel(user.id);
-        if(hasHome){
-          user.hasHome = true;
-        } else {
-          user.hasHome = false;
-        }
-        if(accessLevel && accessLevel === 'security'){
-          user.security = true;
-        } else {
-          user.security = false;
-        }
-        user.id = null;
-        returnUsers.push(user);
+      response.status(200).json({ hidden: await this.memberDataService.isHidden(session.id) });
+    } catch (error) {
+      console.log(error);
+      response.status(400).json({ error: 'Error reading privacy setting' });
+    }
+  }
+
+  /** Sets the caller's hide-yourself flag. Only ever affects the caller's own record. */
+  public async updatePrivacy(request: Request, response: Response): Promise<void> {
+    const session = this.memberService.decryptSession(request, response);
+    if (!session) return;
+    const { hidden } = request.body;
+    if (typeof hidden !== 'boolean') {
+      response.status(400).json({ error: 'hidden must be a boolean' });
+      return;
+    }
+    try {
+      await this.memberDataService.setHidden(session.id, hidden);
+      response.status(200).json({ message: 'success', hidden });
+    } catch (error) {
+      console.log(error);
+      response.status(400).json({ error: 'Error updating privacy setting' });
+    }
+  }
+
+  /**
+   * The online roster.
+   *
+   * Serves visitors as well as members, which is why it uses peekSession rather than
+   * decryptSession -- the latter writes a 400 when no token is present, so a visitor would
+   * get an error instead of the count they are meant to see.
+   *
+   * A visitor receives `{ count, returnUsers: null }`: message/count.html in the original
+   * emits the roster link only when NNM != "Visitor", so an unauthenticated caller learns
+   * how many people are online and nothing more. `null` rather than `[]` so the client can
+   * tell "not permitted to see" from "nobody online".
+   */
+  public async getOnlineUsers(request: Request, response: Response): Promise<any> {
+    const session = this.memberService.peekSession(request);
+    try {
+      const roster = await this.memberService.getRoster(session ? session.id : null);
+
+      if (roster.entries === null) {
+        response.status(200).json({ count: roster.count, returnUsers: null });
+        return;
       }
-      response.status(200).json({ returnUsers });
+
+      // Batched rather than one getHome per entry, matching getDirectory below. The roster
+      // is every visibly-online member, so the per-entry version issued two queries per
+      // person on every poll.
+      const memberIdsWithHome = await this.homeService.findMemberIdsWithHome(
+        roster.entries.map(entry => entry.id),
+      );
+      // getAccessLevel is still one call per entry -- it fans out into canAdmin, canLeader
+      // and a role lookup, so batching it means batching those. Resolved in parallel here
+      // rather than in series; a real batch is a separate change.
+      const accessLevels = await Promise.all(
+        roster.entries.map(entry => this.memberService.getAccessLevel(entry.id)),
+      );
+      const returnUsers = roster.entries.map((entry, i) => ({
+        username: entry.username,
+        hasHome: memberIdsWithHome.has(entry.id),
+        // getAccessLevel returns string[], so the previous `accessLevel === 'security'`
+        // compared an array to a string and was never true -- the flag has been dead since
+        // it was added. Every other consumer (App.vue, admin.vue, the admin panels) already
+        // uses .includes(), which is the correct test.
+        security: accessLevels[i].includes('security'),
+        // Buddies render bold and the viewer's own name renders as plain text rather
+        // than a link; the client owns that markup, so it gets the flags.
+        isBuddy: entry.isBuddy,
+        isSelf: entry.isSelf,
+        // Preserved from the previous shape: member ids are not exposed here.
+        id: null,
+      }));
+      response.status(200).json({ count: roster.count, returnUsers });
     } catch (error) {
       console.log(error);
       response.status(400).json({ error });
@@ -613,4 +676,7 @@ class MemberController {
 const memberService = Container.get(MemberService);
 const homeService = Container.get(HomeService);
 const placeService = Container.get(PlaceService);
-export const memberController = new MemberController(memberService, homeService, placeService);
+const memberDataService = Container.get(MemberDataService);
+export const memberController = new MemberController(
+  memberService, homeService, placeService, memberDataService,
+);

api/src/repositories/member-data/member-data.repository.ts

@@ -56,6 +56,28 @@ export class MemberDataRepository {
     }, {} as Record<string, string | null>);
   }
 
+  /**
+   * One attribute across many members, as memberId -> value.
+   *
+   * Batched deliberately: the roster needs every online member's privacy flag at once, and
+   * fetching that per member would add an N+1 to an endpoint that already has two. Members
+   * with the attribute unset are simply absent from the map.
+   */
+  public async getForMembers(
+    memberIds: number[],
+    name: string,
+  ): Promise<Map<number, string | null>> {
+    const result = new Map<number, string | null>();
+    if (!memberIds.length) return result;
+
+    const rows = await this.db.knex('member_data')
+      .select('member_id', 'value')
+      .whereIn('member_id', memberIds)
+      .andWhere('name', name);
+    for (const row of rows) result.set(row.member_id, row.value);
+    return result;
+  }
+
   /**
    * Sets an attribute, replacing any existing value.
    *

api/src/routes/member.routes.ts

@@ -28,6 +28,12 @@ memberRoutes.get('/session', (request, response) => memberController.session(req
 memberRoutes.post('/update_password', (request, response) =>
   memberController.updatePassword(request, response),
 );
+memberRoutes.get('/get_privacy', (request, response) =>
+  memberController.getPrivacy(request, response),
+);
+memberRoutes.post('/update_privacy', (request, response) =>
+  memberController.updatePrivacy(request, response),
+);
 memberRoutes.post('/update_role', (request, response) =>
   memberController.updatePrimaryRoleId(request, response),
 );

api/src/services/index.ts

@@ -9,6 +9,8 @@ export * from './home/home.service';
 export * from './hood/hood.service';
 export * from './mall/mall.service';
 export * from './member/member.service';
+export * from './member-data/member-data.service';
+export * from './roster/roster.service';
 export * from './message/message.service';
 export * from './object/object.service';
 export * from './object-instance/object-instance.service';

api/src/services/member-data/member-data.service.spec.ts

@@ -0,0 +1,109 @@
+import { Container } from 'typedi';
+import { createSpyObj } from 'jest-createspyobj';
+
+import { MemberDataService } from './member-data.service';
+import { MemberDataRepository } from '../../repositories';
+
+describe('MemberDataService', () => {
+  const MEMBER = 11;
+  let memberDataRepository: jest.Mocked<MemberDataRepository>;
+  let service: MemberDataService;
+
+  beforeEach(() => {
+    memberDataRepository = createSpyObj(MemberDataRepository);
+    Container.reset();
+    Container.set(MemberDataRepository, memberDataRepository);
+    service = Container.get(MemberDataService);
+  });
+
+  describe('isHidden', () => {
+    it('is true only for the exact stored value "1"', async () => {
+      memberDataRepository.get.mockResolvedValue('1');
+      expect(await service.isHidden(MEMBER)).toBe(true);
+    });
+    it('is false when unset', async () => {
+      memberDataRepository.get.mockResolvedValue(null);
+      expect(await service.isHidden(MEMBER)).toBe(false);
+    });
+    /** A stray truthy value must not read as hidden -- '0' is the disabled state. */
+    it('is false for "0"', async () => {
+      memberDataRepository.get.mockResolvedValue('0');
+      expect(await service.isHidden(MEMBER)).toBe(false);
+    });
+    it('is false without a member id, and does not query', async () => {
+      expect(await service.isHidden(0)).toBe(false);
+      expect(memberDataRepository.get).not.toHaveBeenCalled();
+    });
+  });
+
+  describe('setHidden', () => {
+    it('stores "1" when hiding', async () => {
+      await service.setHidden(MEMBER, true);
+      expect(memberDataRepository.set).toHaveBeenCalledWith(MEMBER, 'IMS', '1');
+    });
+    /** null so the repository deletes the row: "unset" keeps one representation. */
+    it('clears the attribute when unhiding', async () => {
+      await service.setHidden(MEMBER, false);
+      expect(memberDataRepository.set).toHaveBeenCalledWith(MEMBER, 'IMS', null);
+    });
+  });
+
+  describe('getBuddySlots', () => {
+    it('always returns exactly ten slots', async () => {
+      memberDataRepository.getByPrefix.mockResolvedValue({});
+      const slots = await service.getBuddySlots(MEMBER);
+      expect(slots).toHaveLength(10);
+      expect(slots.every(s => s === null)).toBe(true);
+    });
+
+    /** The slot INDEX is part of the model: a gap must not shift later slots down. */
+    it('keeps slots sparse rather than compacting them', async () => {
+      memberDataRepository.getByPrefix.mockResolvedValue({ BU0: 'a', BU3: 'b', BU9: 'c' });
+      const slots = await service.getBuddySlots(MEMBER);
+      expect(slots[0]).toBe('a');
+      expect(slots[1]).toBeNull();
+      expect(slots[3]).toBe('b');
+      expect(slots[9]).toBe('c');
+    });
+
+    /** Guards against an out-of-range stored name corrupting the array. */
+    it('ignores names outside the ten-slot range', async () => {
+      memberDataRepository.getByPrefix.mockResolvedValue({ BU0: 'a', BU10: 'x', BUxx: 'y' });
+      const slots = await service.getBuddySlots(MEMBER);
+      expect(slots).toHaveLength(10);
+      expect(slots.filter(Boolean)).toEqual(['a']);
+    });
+
+    /**
+     * The suffix must be exactly one digit, checked as text. Number() is far more permissive
+     * than the slot names are: '' is 0, and '01' and '1e0' are both 1. getByPrefix matches on
+     * prefix, so any of these can arrive from stored data or a future non-slot BU* attribute.
+     */
+    it('rejects prefix matches that are not single-digit slot names', async () => {
+      memberDataRepository.getByPrefix.mockResolvedValue({
+        BU: 'bare prefix, Number("") is 0',
+        BU01: 'leading zero, Number() reads 1',
+        BU1e0: 'exponent, Number() reads 1',
+        BU2: 'the only real slot here',
+      });
+      const slots = await service.getBuddySlots(MEMBER);
+      expect(slots[0]).toBeNull();
+      expect(slots[1]).toBeNull();
+      expect(slots[2]).toBe('the only real slot here');
+      expect(slots.filter(Boolean)).toEqual(['the only real slot here']);
+    });
+  });
+
+  describe('getBuddyNameSet', () => {
+    it('lowercases for case-insensitive nickname matching', async () => {
+      memberDataRepository.getByPrefix.mockResolvedValue({ BU0: 'HawK', BU1: 'scott99' });
+      const set = await service.getBuddyNameSet(MEMBER);
+      expect(set.has('hawk')).toBe(true);
+      expect(set.has('scott99')).toBe(true);
+    });
+    it('drops empty slots', async () => {
+      memberDataRepository.getByPrefix.mockResolvedValue({ BU0: 'a', BU1: null });
+      expect((await service.getBuddyNameSet(MEMBER)).size).toBe(1);
+    });
+  });
+});

api/src/services/member-data/member-data.service.ts

@@ -0,0 +1,87 @@
+import { Service } from 'typedi';
+
+import { MemberDataRepository } from '../../repositories';
+
+/**
+ * Named per-member attributes, with the CS 4.x attribute names kept in one place.
+ *
+ * Callers should use these methods rather than passing raw attribute names around, so
+ * what 'IMS' and 'BU3' mean is defined here and nowhere else.
+ */
+@Service()
+export class MemberDataService {
+  /**
+   * The hide-yourself privacy flag.
+   *
+   * A single checkbox in the original (message/config.html) and that is the ENTIRE privacy
+   * model -- no per-buddy blocking, no appear-offline-to-some, no ignore list. The
+   * simplicity is the design, so resist growing it.
+   */
+  public static readonly HIDDEN = 'IMS';
+
+  /** Buddy slot prefix. Exactly ten slots, BU0..BU9, holding nicknames. */
+  public static readonly BUDDY_PREFIX = 'BU';
+  public static readonly BUDDY_SLOTS = 10;
+
+  constructor(private memberDataRepository: MemberDataRepository) {}
+
+  /** True if the member has chosen to appear offline. */
+  public async isHidden(memberId: number): Promise<boolean> {
+    if (!memberId) return false;
+    return (await this.memberDataRepository.get(memberId, MemberDataService.HIDDEN)) === '1';
+  }
+
+  /** Sets or clears the hide-yourself flag. */
+  public async setHidden(memberId: number, hidden: boolean): Promise<void> {
+    await this.memberDataRepository.set(
+      memberId,
+      MemberDataService.HIDDEN,
+      hidden ? '1' : null,
+    );
+  }
+
+  /**
+   * The member's buddy nicknames, by slot index.
+   *
+   * Sparse on purpose: slot 3 being empty does not shift slots 4..9 down, because the slot
+   * index is part of the original's model. Index is the array position; empty slots are
+   * null.
+   *
+   * Read-only here. Managing the list (adding, removing, the "buddy entered" notification)
+   * is a separate task.
+   */
+  public async getBuddySlots(memberId: number): Promise<(string | null)[]> {
+    const slots: (string | null)[] = new Array(MemberDataService.BUDDY_SLOTS).fill(null);
+    if (!memberId) return slots;
+
+    const stored = await this.memberDataRepository
+      .getByPrefix(memberId, MemberDataService.BUDDY_PREFIX);
+    for (const [name, value] of Object.entries(stored)) {
+      const suffix = name.slice(MemberDataService.BUDDY_PREFIX.length);
+      // Exactly one digit, matched as text before any numeric conversion. A prefix match is
+      // not a slot name: Number('') is 0, so a bare 'BU' would otherwise land in slot 0, and
+      // Number() also accepts 'BU01' and 'BU1e0' as 1, colliding with BU1. Any future BU*
+      // attribute that is not a slot would be silently read as one too.
+      if (!/^[0-9]$/.test(suffix)) continue;
+      const index = Number(suffix);
+      if (index < MemberDataService.BUDDY_SLOTS) {
+        slots[index] = value;
+      }
+    }
+    return slots;
+  }
+
+  /**
+   * The member's buddy nicknames lowercased, for membership tests.
+   *
+   * Buddies are stored by NICKNAME, not id, so comparison has to be case-insensitive --
+   * the original's own field set includes NNK, a lowercased nickname, precisely because
+   * nicknames are matched case-insensitively.
+   */
+  public async getBuddyNameSet(memberId: number): Promise<Set<string>> {
+    const slots = await this.getBuddySlots(memberId);
+    return new Set(
+      slots.filter((name): name is string => !!name).map(name => name.toLowerCase()),
+    );
+  }
+}