fix: one module answers to ./tenantService (#1049)

src/context/TenantContext.jsx

@@ -82,6 +82,25 @@ function nameFor(id) {
   return found ? found.name : DEFAULT_TENANT_NAME;
 }
 
+/**
+ * Whatever is stored under the tenant key, unvalidated, or null.
+ *
+ * Separate from `readStoredTenant()` because the API-driven paths match the
+ * stored id against the tenant list the server returned, which is not the same
+ * set as `KNOWN_TENANTS` — validating against the static list there would
+ * discard a legitimate server-side workspace id.
+ *
+ * @returns {string|null}
+ */
+function readRawStoredTenant() {
+  try {
+    return localStorage.getItem(STORAGE_KEY);
+  } catch {
+    // Private browsing, or site data blocked. No stored choice is readable.
+    return null;
+  }
+}
+
 /**
  * The tenant the visitor previously chose, or the default.
  *
@@ -90,12 +109,8 @@ function nameFor(id) {
  * @returns {string}
  */
 function readStoredTenant() {
-  try {
-    const stored = localStorage.getItem(STORAGE_KEY);
-    return isKnownTenant(stored) ? stored : DEFAULT_TENANT_ID;
-  } catch {
-    return DEFAULT_TENANT_ID;
-  }
+  const stored = readRawStoredTenant();
+  return isKnownTenant(stored) ? stored : DEFAULT_TENANT_ID;
 }
 
 /**
@@ -141,9 +156,12 @@ export function TenantProvider({ children }) {
       const data = await fetchUserTenants();
       setTenants(data);
 
-      // If we have API tenants and no current tenant is set, use the first one
+      // If we have API tenants and no current tenant is set, use the first one.
+      // Read through readStoredTenant() rather than touching localStorage
+      // directly: a browser with site data blocked throws on the bare call, and
+      // this one runs inside an async callback where nothing catches it (#843).
       if (data.length > 0 && !currentTenant) {
-        const savedTenantId = localStorage.getItem(STORAGE_KEY);
+        const savedTenantId = readRawStoredTenant();
         const savedTenant = savedTenantId
           ? data.find(t => t.id === savedTenantId) || data[0]
           : data[0];
@@ -227,7 +245,7 @@ export function TenantProvider({ children }) {
   }, [tenantId, currentTenant]);
 
   useEffect(() => {
-    const activeId = localStorage.getItem(STORAGE_KEY);
+    const activeId = readRawStoredTenant();
     fetchTenants().then(() => {
       if (activeId && tenants.length > 0) {
         const saved = tenants.find((t) => t.id === activeId);

src/services/historicalDataService.tenantScoping.test.js

@@ -0,0 +1,87 @@
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+
+/**
+ * Cover for #1049.
+ *
+ * `historicalDataService` calls `getTenantScopedDbName()` and
+ * `getTenantScopedStoreName()` at module scope:
+ *
+ *   const SCOPED_DB_NAME = getTenantScopedDbName(DB_NAME);
+ *
+ * so when `./tenantService` resolved to a module without those exports, the
+ * import threw `TypeError: getTenantScopedDbName is not a function` before any
+ * of the module's own code ran. Every other test in this directory failed at
+ * collection, which reads as "the suite is broken" rather than "this one import
+ * is wrong" — hence a test whose subject is the import itself.
+ */
+describe('historicalDataService — tenant-scoped storage (#1049)', () => {
+  beforeEach(() => {
+    vi.resetModules();
+    localStorage.clear();
+  });
+
+  afterEach(() => {
+    vi.restoreAllMocks();
+  });
+
+  it('imports without throwing', async () => {
+    await expect(import('./historicalDataService')).resolves.toBeDefined();
+  });
+
+  it('exports the functions its consumers import', async () => {
+    const module = await import('./historicalDataService');
+
+    for (const name of [
+      'openDB',
+      'getCachedData',
+      'setCachedData',
+      'pruneCache',
+      'fetchHistoricalData',
+      'formatHistoricalCSV',
+      'getDelimiterForLocale',
+      'HISTORY_CACHE_TTL',
+    ]) {
+      expect(module[name], `expected historicalDataService to export ${name}`).toBeDefined();
+    }
+  });
+
+  it('opens a database name scoped to the default tenant', async () => {
+    const opened = [];
+    vi.stubGlobal('indexedDB', {
+      open: (name) => {
+        opened.push(name);
+        const request = { onsuccess: null, onerror: null, onupgradeneeded: null, result: null, error: new Error('stub') };
+        setTimeout(() => request.onerror?.({ target: request }), 0);
+        return request;
+      },
+    });
+
+    const { openDB } = await import('./historicalDataService');
+    await expect(openDB()).rejects.toBeTruthy();
+
+    expect(opened).toEqual(['PollutionHubDB__default']);
+    vi.unstubAllGlobals();
+  });
+
+  it('opens a database name scoped to the selected tenant', async () => {
+    localStorage.setItem('pch_tenant_id', 'mumbai-municipal');
+
+    const opened = [];
+    vi.stubGlobal('indexedDB', {
+      open: (name) => {
+        opened.push(name);
+        const request = { onsuccess: null, onerror: null, onupgradeneeded: null, result: null, error: new Error('stub') };
+        setTimeout(() => request.onerror?.({ target: request }), 0);
+        return request;
+      },
+    });
+
+    const { openDB } = await import('./historicalDataService');
+    await expect(openDB()).rejects.toBeTruthy();
+
+    // The whole point of #759: two organisations on one browser profile must not
+    // share a cache. A shared database name is the failure that scoping prevents.
+    expect(opened).toEqual(['PollutionHubDB__mumbai-municipal']);
+    vi.unstubAllGlobals();
+  });
+});

src/services/tenantService.client.test.js

@@ -0,0 +1,163 @@
+import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
+import {
+  fetchUserTenants,
+  updateTenantSettings,
+  inviteTenantMember,
+  removeTenantMember,
+  getTenantScopedDbName,
+} from './tenantService';
+
+/**
+ * Cover for #1049.
+ *
+ * The point of these is less the individual assertions than the import at the
+ * top: `./tenantService` has to resolve to a module that exports both halves.
+ * While `tenantService.js` shadowed `tenantService.ts` exactly one of the two
+ * groups was reachable at a time, and which one depended on the resolver.
+ */
+
+/** @param {any} body @param {{ok?: boolean, status?: number}} [init] */
+function jsonResponse(body, { ok = true, status = 200 } = {}) {
+  return {
+    ok,
+    status,
+    json: async () => body,
+  };
+}
+
+describe('tenantService — one module, both halves (#1049)', () => {
+  beforeEach(() => {
+    localStorage.clear();
+    vi.stubGlobal('fetch', vi.fn());
+  });
+
+  afterEach(() => {
+    vi.unstubAllGlobals();
+    vi.restoreAllMocks();
+  });
+
+  it('exposes the scoping helpers and the REST client from the same specifier', async () => {
+    expect(typeof getTenantScopedDbName).toBe('function');
+    expect(typeof fetchUserTenants).toBe('function');
+    expect(typeof updateTenantSettings).toBe('function');
+    expect(typeof inviteTenantMember).toBe('function');
+    expect(typeof removeTenantMember).toBe('function');
+  });
+
+  describe('fetchUserTenants', () => {
+    it('returns the parsed workspace list', async () => {
+      fetch.mockResolvedValue(jsonResponse([{ id: 'a', name: 'A' }]));
+
+      await expect(fetchUserTenants()).resolves.toEqual([{ id: 'a', name: 'A' }]);
+      expect(fetch).toHaveBeenCalledWith(
+        expect.stringContaining('/tenants'),
+        expect.objectContaining({ method: 'GET' })
+      );
+    });
+
+    it('omits the Authorization header entirely when no token is stored', async () => {
+      fetch.mockResolvedValue(jsonResponse([]));
+
+      await fetchUserTenants();
+
+      const { headers } = fetch.mock.calls[0][1];
+      // Not `Bearer null`: an absent token has to read as an anonymous request,
+      // not as a request presenting the string "null" as a credential.
+      expect(headers).not.toHaveProperty('Authorization');
+    });
+
+    it('sends the stored token when there is one', async () => {
+      localStorage.setItem('token', 'abc123');
+      fetch.mockResolvedValue(jsonResponse([]));
+
+      await fetchUserTenants();
+
+      expect(fetch.mock.calls[0][1].headers.Authorization).toBe('Bearer abc123');
+    });
+
+    it('does not throw when localStorage is unreadable', async () => {
+      vi.spyOn(Storage.prototype, 'getItem').mockImplementation(() => {
+        throw new DOMException('The operation is insecure.', 'SecurityError');
+      });
+      fetch.mockResolvedValue(jsonResponse([]));
+
+      await expect(fetchUserTenants()).resolves.toEqual([]);
+    });
+
+    it('surfaces the server message on a failed response', async () => {
+      fetch.mockResolvedValue(
+        jsonResponse({ message: 'Workspace access revoked' }, { ok: false, status: 403 })
+      );
+
+      await expect(fetchUserTenants()).rejects.toThrow('Workspace access revoked');
+    });
+
+    it('falls back to the status when the error body is not JSON', async () => {
+      fetch.mockResolvedValue({
+        ok: false,
+        status: 502,
+        json: async () => {
+          throw new SyntaxError('Unexpected token < in JSON');
+        },
+      });
+
+      await expect(fetchUserTenants()).rejects.toThrow('HTTP 502');
+    });
+  });
+
+  describe('updateTenantSettings', () => {
+    it('PATCHes the settings under a `settings` key', async () => {
+      fetch.mockResolvedValue(jsonResponse({ id: 't1', settings: { theme: 'dark' } }));
+
+      await updateTenantSettings('t1', { theme: 'dark' });
+
+      const [url, init] = fetch.mock.calls[0];
+      expect(url).toContain('/tenants/t1/settings');
+      expect(init.method).toBe('PATCH');
+      expect(JSON.parse(init.body)).toEqual({ settings: { theme: 'dark' } });
+    });
+
+    it('encodes the tenant id into the path', async () => {
+      fetch.mockResolvedValue(jsonResponse({}));
+
+      await updateTenantSettings('a/b', {});
+
+      expect(fetch.mock.calls[0][0]).toContain('/tenants/a%2Fb/settings');
+    });
+  });
+
+  describe('inviteTenantMember', () => {
+    it('POSTs the email and role', async () => {
+      fetch.mockResolvedValue(jsonResponse({ id: 'm1' }));
+
+      await inviteTenantMember('t1', 'someone@example.com', 'MEMBER');
+
+      const [, init] = fetch.mock.calls[0];
+      expect(init.method).toBe('POST');
+      expect(JSON.parse(init.body)).toEqual({
+        email: 'someone@example.com',
+        role: 'MEMBER',
+      });
+    });
+  });
+
+  describe('removeTenantMember', () => {
+    it('DELETEs the membership and resolves with nothing', async () => {
+      fetch.mockResolvedValue({ ok: true, status: 204, json: async () => undefined });
+
+      await expect(removeTenantMember('t1', 'm1')).resolves.toBeUndefined();
+      expect(fetch.mock.calls[0][0]).toContain('/tenants/t1/members/m1');
+      expect(fetch.mock.calls[0][1].method).toBe('DELETE');
+    });
+
+    it('rejects with the server message when the removal is refused', async () => {
+      fetch.mockResolvedValue(
+        jsonResponse({ message: 'Cannot remove the last admin' }, { ok: false, status: 409 })
+      );
+
+      await expect(removeTenantMember('t1', 'm1')).rejects.toThrow(
+        'Cannot remove the last admin'
+      );
+    });
+  });
+});