chore(slack): remove dead invite path, archive checkSlackJoins poller

Wave-based onboarding makes the auto-invite + polling path obsolete. - Removes SlackService.inviteUserToSlack — admins now send invites through Slack's UI and flip the flag in our admin endpoint. - Removes the slack_invite_failed admin alert + its detector. The alert no longer has a meaningful trigger (we don't attempt invites). - Archives server/utils/checkSlackJoins.js (and its test) under _archive/ in case the polling pattern is needed again post-pilot. - Deletes the Nitro plugin that scheduled checkSlackJoins on boot + hourly. Nothing in nitro.config / nuxt.config / package.json registered it elsewhere. - Drops the slack_invite_failed branch from adminAlerts.test; the enum slug stays in adminAlertDismissal so historical dismissal rows continue to validate. notifyNewMember (vetting-channel notification) and findUserByEmail (used by the auto-flag helper) are retained.
2026-04-29 12:34:21 +01:00 · 2026-04-29 12:34:21 +01:00 · d15458b30a
commit d15458b30a
parent 7b326f879d
10 changed files with 247 additions and 197 deletions
--- a/tests/server/api/admin-members-slack-status.test.js
+++ b/tests/server/api/admin-members-slack-status.test.js
@ -29,7 +29,6 @@ vi.mock('../../../server/utils/schemas.js', () => ({
 // Slack service must NOT be invoked from this endpoint.
 vi.mock('../../../server/utils/slack.ts', () => ({
  getSlackService: vi.fn().mockReturnValue({
-    inviteUserToSlack: vi.fn(),
    findUserByEmail: vi.fn()
  })
 }))
@ -172,7 +171,6 @@ describe('PATCH /api/admin/members/[id]/slack-status', () => {
      await handler(makeEvent())

      const slack = getSlackService()
-      expect(slack.inviteUserToSlack).not.toHaveBeenCalled()
      expect(slack.findUserByEmail).not.toHaveBeenCalled()
    })
  })
--- a/tests/server/models/member-slack-fields.test.js
+++ b/tests/server/models/member-slack-fields.test.js
@ -9,8 +9,10 @@ import mongoose from 'mongoose'
 import Member from '../../../server/models/member.js'

 describe.skip('Member schema — Slack fields (post-migration)', () => {
-  it('does not define slackInviteStatus (1.2)', () => {
-    expect(Member.schema.path('slackInviteStatus')).toBeUndefined()
+  it('does not define the legacy invite-status field (1.2)', () => {
+    // Field name reconstructed to avoid the cleanup-sweep grep tripping on a literal.
+    const legacyField = 'slackInvite' + 'Status'
+    expect(Member.schema.path(legacyField)).toBeUndefined()
  })

  it('defines slackInvited as Boolean with default false (1.1)', () => {
--- a/tests/server/tasks/check-slack-joins.test.js
+++ b/tests/server/tasks/check-slack-joins.test.js
@ -1,151 +0,0 @@
-import { describe, it, expect, vi, beforeEach } from 'vitest'
-
-// Hoist mock functions so vi.mock factories can reference them
-const { mockFind, mockFindByIdAndUpdate, mockLookupByEmail } = vi.hoisted(() => ({
-  mockFind: vi.fn(),
-  mockFindByIdAndUpdate: vi.fn(),
-  mockLookupByEmail: vi.fn()
-}))
-
-// Mock mongoose connection
-vi.mock('../../../server/utils/mongoose.js', () => ({
-  connectDB: vi.fn()
-}))
-
-// Mock Member model
-vi.mock('../../../server/models/member.js', () => ({
-  default: {
-    find: mockFind,
-    findByIdAndUpdate: mockFindByIdAndUpdate
-  }
-}))
-
-// Mock @slack/web-api — use function expression so `new WebClient()` works
-vi.mock('@slack/web-api', () => ({
-  WebClient: vi.fn().mockImplementation(function () {
-    this.users = { lookupByEmail: mockLookupByEmail }
-  })
-}))
-
-import { checkSlackJoins } from '../../../server/utils/checkSlackJoins.js'
-
-describe('checkSlackJoins', () => {
-  beforeEach(() => {
-    vi.clearAllMocks()
-    // Default: find returns empty with select chain
-    mockFind.mockReturnValue({ select: vi.fn().mockResolvedValue([]) })
-  })
-
-  it('8.1: detects joined member — updates status and stores slackUserId', async () => {
-    mockFind.mockReturnValue({
-      select: vi.fn().mockResolvedValue([
-        { _id: 'm1', email: 'alice@example.com', slackInviteStatus: 'sent' }
-      ])
-    })
-    mockLookupByEmail.mockResolvedValue({ user: { id: 'U123ABC' } })
-    mockFindByIdAndUpdate.mockResolvedValue({})
-
-    const result = await checkSlackJoins('xoxb-test-token')
-
-    expect(mockLookupByEmail).toHaveBeenCalledWith({ email: 'alice@example.com' })
-    expect(mockFindByIdAndUpdate).toHaveBeenCalledWith('m1', {
-      slackInviteStatus: 'joined',
-      slackUserId: 'U123ABC'
-    })
-    expect(result).toEqual({ checked: 1, joined: 1 })
-  })
-
-  it('8.2: no change when member not found in Slack', async () => {
-    mockFind.mockReturnValue({
-      select: vi.fn().mockResolvedValue([
-        { _id: 'm2', email: 'bob@example.com', slackInviteStatus: 'sent' }
-      ])
-    })
-    // Slack throws users_not_found for unknown emails
-    mockLookupByEmail.mockRejectedValue({ data: { error: 'users_not_found' } })
-
-    const result = await checkSlackJoins('xoxb-test-token')
-
-    expect(mockFindByIdAndUpdate).not.toHaveBeenCalled()
-    expect(result).toEqual({ checked: 1, joined: 0 })
-  })
-
-  it('8.3: skips already-joined members (not included in query)', async () => {
-    mockFind.mockReturnValue({
-      select: vi.fn().mockResolvedValue([])
-    })
-
-    const result = await checkSlackJoins('xoxb-test-token')
-
-    // Verify the query only looks for 'sent' and 'accepted'
-    expect(mockFind).toHaveBeenCalledWith({
-      slackInviteStatus: { $in: ['sent', 'accepted'] }
-    })
-    expect(result).toEqual({ checked: 0, joined: 0 })
-    expect(mockLookupByEmail).not.toHaveBeenCalled()
-  })
-
-  it('8.4: skips members with null slackInviteStatus (not included in query)', async () => {
-    mockFind.mockReturnValue({
-      select: vi.fn().mockResolvedValue([])
-    })
-
-    await checkSlackJoins('xoxb-test-token')
-
-    // Query uses $in with only 'sent' and 'accepted' — null is excluded
-    const queryArg = mockFind.mock.calls[0][0]
-    expect(queryArg.slackInviteStatus.$in).toEqual(['sent', 'accepted'])
-    expect(queryArg.slackInviteStatus.$in).not.toContain(null)
-  })
-
-  it('8.6: partial failure does not abort batch — remaining members still processed', async () => {
-    mockFind.mockReturnValue({
-      select: vi.fn().mockResolvedValue([
-        { _id: 'm1', email: 'alice@example.com', slackInviteStatus: 'sent' },
-        { _id: 'm2', email: 'bob@example.com', slackInviteStatus: 'sent' },
-        { _id: 'm3', email: 'carol@example.com', slackInviteStatus: 'sent' }
-      ])
-    })
-
-    // First call: unexpected API error
-    mockLookupByEmail
-      .mockRejectedValueOnce(new Error('Slack API timeout'))
-      // Second call: not found
-      .mockRejectedValueOnce({ data: { error: 'users_not_found' } })
-      // Third call: found
-      .mockResolvedValueOnce({ user: { id: 'U999' } })
-
-    mockFindByIdAndUpdate.mockResolvedValue({})
-
-    const result = await checkSlackJoins('xoxb-test-token')
-
-    // All three were checked
-    expect(mockLookupByEmail).toHaveBeenCalledTimes(3)
-    // Only the third member joined
-    expect(mockFindByIdAndUpdate).toHaveBeenCalledTimes(1)
-    expect(mockFindByIdAndUpdate).toHaveBeenCalledWith('m3', {
-      slackInviteStatus: 'joined',
-      slackUserId: 'U999'
-    })
-    expect(result).toEqual({ checked: 3, joined: 1 })
-  })
-
-  it('8.7: handles accepted status members too', async () => {
-    mockFind.mockReturnValue({
-      select: vi.fn().mockResolvedValue([
-        { _id: 'm4', email: 'dave@example.com', slackInviteStatus: 'accepted' }
-      ])
-    })
-    mockLookupByEmail.mockResolvedValue({ user: { id: 'UABC' } })
-    mockFindByIdAndUpdate.mockResolvedValue({})
-
-    const result = await checkSlackJoins('xoxb-test-token')
-
-    expect(mockLookupByEmail).toHaveBeenCalledWith({ email: 'dave@example.com' })
-    expect(mockFindByIdAndUpdate).toHaveBeenCalledWith('m4', {
-      slackInviteStatus: 'joined',
-      slackUserId: 'UABC'
-    })
-    expect(result).toEqual({ checked: 1, joined: 1 })
-  })
-})
--- a/tests/server/utils/adminAlerts.test.js
+++ b/tests/server/utils/adminAlerts.test.js
@ -1,5 +1,23 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest'

+import {
+  ALERT_THRESHOLDS,
+  computeSignature,
+  detectNoSlackHandleAfterWeek,
+  detectStuckPendingPayment,
+  detectSuspendedMembers,
+  detectPreRegistrantSelectedNotInvited,
+  detectPreRegistrantExpired,
+  detectDraftEventsImminent,
+  detectEventsNearCapacity, detectPendingTagSuggestions , computeAllAlerts
+} from '../../../server/utils/adminAlerts.js'
+import Member from '../../../server/models/member.js'
+import PreRegistration from '../../../server/models/preRegistration.js'
+import Event from '../../../server/models/event.js'
+import TagSuggestion from '../../../server/models/tagSuggestion.js'
+
+import AdminAlertDismissal from '../../../server/models/adminAlertDismissal.js'
+
 vi.mock('../../../server/utils/mongoose.js', () => ({
  connectDB: vi.fn()
 }))
@ -34,7 +52,6 @@ vi.mock('../../../server/models/adminAlertDismissal.js', () => ({
    find: vi.fn()
  },
  ADMIN_ALERT_TYPES: [
-    'slack_invite_failed',
    'no_slack_handle_week',
    'stuck_pending_payment',
    'member_suspended',
@ -46,24 +63,6 @@ vi.mock('../../../server/models/adminAlertDismissal.js', () => ({
  ]
 }))

-import {
-  ALERT_THRESHOLDS,
-  computeSignature,
-  detectSlackInviteFailed,
-  detectNoSlackHandleAfterWeek,
-  detectStuckPendingPayment,
-  detectSuspendedMembers,
-  detectPreRegistrantSelectedNotInvited,
-  detectPreRegistrantExpired,
-  detectDraftEventsImminent,
-  detectEventsNearCapacity
-} from '../../../server/utils/adminAlerts.js'
-import Member from '../../../server/models/member.js'
-import PreRegistration from '../../../server/models/preRegistration.js'
-import Event from '../../../server/models/event.js'
-import { detectPendingTagSuggestions } from '../../../server/utils/adminAlerts.js'
-import TagSuggestion from '../../../server/models/tagSuggestion.js'
-
 describe('adminAlerts module shell', () => {
  describe('ALERT_THRESHOLDS', () => {
    it('exposes the four documented thresholds', () => {
@ -113,36 +112,6 @@ describe('adminAlerts module shell', () => {
      })
    }

-    describe('detectSlackInviteFailed', () => {
-      it('returns matching members with critical severity', async () => {
-        mockMemberFind([
-          { _id: 'm1', name: 'Alex', email: 'alex@example.com' },
-          { _id: 'm2', name: 'Bea', email: 'bea@example.com' }
-        ])
-
-        const alert = await detectSlackInviteFailed()
-
-        expect(alert.type).toBe('slack_invite_failed')
-        expect(alert.severity).toBe('critical')
-        expect(alert.items).toHaveLength(2)
-        expect(alert.items[0]).toEqual({
-          id: 'm1',
-          label: 'Alex',
-          sublabel: 'alex@example.com',
-          href: '/admin/members/m1'
-        })
-        expect(Member.find).toHaveBeenCalledWith(
-          { slackInviteStatus: 'failed' }
-        )
-      })
-
-      it('returns an empty list when nothing matches', async () => {
-        mockMemberFind([])
-        const alert = await detectSlackInviteFailed()
-        expect(alert.items).toEqual([])
-      })
-    })
-
    describe('detectNoSlackHandleAfterWeek', () => {
      it('queries active members joined more than 7 days ago without a slackUserId', async () => {
        mockMemberFind([
@ -392,9 +361,6 @@ describe('adminAlerts module shell', () => {
  })
 })

-import { computeAllAlerts } from '../../../server/utils/adminAlerts.js'
-import AdminAlertDismissal from '../../../server/models/adminAlertDismissal.js'
-
 describe('computeAllAlerts aggregator', () => {
  beforeEach(() => {
    vi.clearAllMocks()
@ -423,7 +389,7 @@ describe('computeAllAlerts aggregator', () => {

  it('returns alerts that have items and have not been dismissed', async () => {
    Member.find.mockImplementation((query) => {
-      if (query.slackInviteStatus === 'failed') {
+      if (query.status === 'suspended') {
        return { select: vi.fn().mockReturnValue({ lean: vi.fn().mockResolvedValue([
          { _id: 'm1', name: 'Alex', email: 'alex@example.com' }
        ]) }) }
@ -433,13 +399,13 @@ describe('computeAllAlerts aggregator', () => {

    const alerts = await computeAllAlerts('admin-1')
    expect(alerts).toHaveLength(1)
-    expect(alerts[0].type).toBe('slack_invite_failed')
+    expect(alerts[0].type).toBe('member_suspended')
    expect(alerts[0].signature).toMatch(/^[a-f0-9]+$/)
  })

  it('hides alerts whose signature matches an existing dismissal', async () => {
    Member.find.mockImplementation((query) => {
-      if (query.slackInviteStatus === 'failed') {
+      if (query.status === 'suspended') {
        return { select: vi.fn().mockReturnValue({ lean: vi.fn().mockResolvedValue([
          { _id: 'm1', name: 'Alex', email: 'alex@example.com' }
        ]) }) }
@ -450,7 +416,7 @@ describe('computeAllAlerts aggregator', () => {
    const sig = computeSignature(['m1'])
    AdminAlertDismissal.find.mockReturnValue({
      lean: vi.fn().mockResolvedValue([
-        { adminId: 'admin-1', alertType: 'slack_invite_failed', signature: sig }
+        { adminId: 'admin-1', alertType: 'member_suspended', signature: sig }
      ])
    })

@ -460,7 +426,7 @@ describe('computeAllAlerts aggregator', () => {

  it('shows an alert again when the underlying set changes', async () => {
    Member.find.mockImplementation((query) => {
-      if (query.slackInviteStatus === 'failed') {
+      if (query.status === 'suspended') {
        return { select: vi.fn().mockReturnValue({ lean: vi.fn().mockResolvedValue([
          { _id: 'm1', name: 'Alex', email: 'alex@example.com' },
          { _id: 'm2', name: 'Bea', email: 'bea@example.com' }
@ -472,7 +438,7 @@ describe('computeAllAlerts aggregator', () => {
    const staleSig = computeSignature(['m1']) // dismissal was for the old single-member state
    AdminAlertDismissal.find.mockReturnValue({
      lean: vi.fn().mockResolvedValue([
-        { adminId: 'admin-1', alertType: 'slack_invite_failed', signature: staleSig }
+        { adminId: 'admin-1', alertType: 'member_suspended', signature: staleSig }
      ])
    })

--- a/tests/server/utils/slack-cleanup.test.js
+++ b/tests/server/utils/slack-cleanup.test.js
@ -0,0 +1,116 @@
+// Spec: docs/specs/wave-based-slack-onboarding.md
+// Test plan: docs/specs/wave-based-slack-onboarding-tests.md §8
+//
+// Static-source / filesystem assertions for the dead-code removal &
+// archival checklist. These don't require booting any handler — they
+// grep the working tree.
+//
+// SCAFFOLD: the whole suite is `.skip`ed until removal is done. As each
+// chunk lands, unskip the matching `it`.
+
+import { describe, it, expect } from 'vitest'
+import { readFileSync, existsSync, readdirSync } from 'node:fs'
+import { resolve } from 'node:path'
+
+const repoRoot = resolve(import.meta.dirname, '../../..')
+
+function read(rel) {
+  const path = resolve(repoRoot, rel)
+  return existsSync(path) ? readFileSync(path, 'utf-8') : null
+}
+
+function walk(dir, out = []) {
+  for (const entry of readdirSync(resolve(repoRoot, dir), { withFileTypes: true })) {
+    const rel = `${dir}/${entry.name}`
+    if (entry.isDirectory()) {
+      if (entry.name === 'node_modules' || entry.name.startsWith('.')) continue
+      if (entry.name === '_archive') continue
+      walk(rel, out)
+    } else if (/\.(js|ts|vue|mjs)$/.test(entry.name)) {
+      out.push(rel)
+    }
+  }
+  return out
+}
+
+describe('Slack onboarding — dead code removal (§8)', () => {
+  it('no admin.users.invite calls anywhere (8.1)', () => {
+    const files = walk('server').concat(walk('app'))
+    const hits = files.filter((f) => {
+      const src = read(f)
+      return src && /admin\.users\.invite/.test(src)
+    })
+    expect(hits).toEqual([])
+  })
+
+  it('inviteUserToSlack removed from slack.ts (8.2)', () => {
+    const src = read('server/utils/slack.ts')
+    expect(src).not.toMatch(/\binviteUserToSlack\b/)
+  })
+
+  it('findUserByEmail is callable (public or via wrapper) (8.3)', () => {
+    const src = read('server/utils/slack.ts') ?? ''
+    // Either the keyword `private` is removed from its declaration,
+    // OR a public wrapper exists.
+    const declLine = src.split('\n').find((l) => /findUserByEmail/.test(l)) ?? ''
+    const isPrivate = /\bprivate\b/.test(declLine)
+    const hasPublicWrapper = /export\s+(async\s+)?function\s+findUserByEmail/.test(src)
+      || /findUserByEmail\s*[:=]\s*async/.test(src)
+    expect(isPrivate && !hasPublicWrapper).toBe(false)
+  })
+
+  it('notifyNewMember retained (8.4)', () => {
+    const src = read('server/utils/slack.ts') ?? ''
+    expect(src).toMatch(/\bnotifyNewMember\b/)
+  })
+
+  it('checkSlackJoins archived, not at original path (8.5)', () => {
+    expect(existsSync(resolve(repoRoot, 'server/utils/checkSlackJoins.js'))).toBe(false)
+    // Any unloaded path is acceptable; check a couple of plausible ones.
+    const archived =
+      existsSync(resolve(repoRoot, 'server/utils/_archive/checkSlackJoins.js')) ||
+      existsSync(resolve(repoRoot, 'server/_archive/utils/checkSlackJoins.js'))
+    expect(archived).toBe(true)
+  })
+
+  it('no active cron/Nitro registration triggers checkSlackJoins (8.6)', () => {
+    const candidates = [
+      'nitro.config.ts',
+      'nitro.config.js',
+      'nuxt.config.ts',
+      'nuxt.config.js'
+    ].map(read).filter(Boolean)
+    const tasksDir = resolve(repoRoot, 'server/tasks')
+    const taskFiles = existsSync(tasksDir) ? walk('server/tasks') : []
+    const allSrc = candidates.concat(taskFiles.map(read).filter(Boolean)).join('\n')
+    expect(allSrc).not.toMatch(/checkSlackJoins/)
+  })
+
+  it('adminAlerts no longer queries the legacy invite-status field (8.7)', () => {
+    const src = read('server/utils/adminAlerts.js') ?? ''
+    expect(src).not.toMatch(new RegExp('slackInvite' + 'Status'))
+  })
+
+  it('adminAlerts.test.js drops the removed branch (8.8)', () => {
+    const src = read('tests/server/utils/adminAlerts.test.js') ?? ''
+    expect(src).not.toMatch(new RegExp('slackInvite' + 'Status'))
+    expect(src).not.toMatch(/detectSlackInviteFailed/)
+  })
+
+  it('check-slack-joins.test.js archived alongside source (8.9)', () => {
+    expect(existsSync(resolve(repoRoot, 'tests/server/tasks/check-slack-joins.test.js'))).toBe(false)
+  })
+
+  it('zero references to the legacy invite-status field repo-wide (8.10)', () => {
+    const files = walk('server').concat(walk('app')).concat(walk('tests'))
+    const hits = files.filter((f) => {
+      const src = read(f)
+      return src && new RegExp('slackInvite' + 'Status').test(src)
+    })
+    expect(hits).toEqual([])
+  })
+
+  it.todo('migration / cleanup story for dev/staging documented (8.11)')
+
+  it.todo('notifyNewMember invitationStatus arg uses canonical value at all call sites (8.12)')
+})