Workspace Settings Done with auto-add

This commit is contained in:
dal 2025-07-10 07:01:23 -06:00
parent 99837c8d2c
commit 29e6a6ae7f
No known key found for this signature in database
GPG Key ID: 16F4B0E1E9F61122
30 changed files with 7573 additions and 439 deletions

View File

@ -160,4 +160,10 @@ export async function getWorkspaceSettingsHandler(
### Authentication and User Context
- **Use requireAuth middleware** - Apply to all protected routes
- **Extract user context** - Use `c.get('busterUser')` to get the authenticated user
- **Type as User** - Import `User` type from `@buster/database` for handler parameters
- **Type as User** - Import `User` type from `@buster/database` for handler parameters
## Database Operations
### Soft Delete and Upsert Practices
- In our database, we never hard delete, we always use soft deletes with the `deleted_at` field
- For update operations, we should almost always perform an upsert unless otherwise specified

View File

@ -1,15 +1,15 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import type { Organization, User } from '@buster/database';
import { HTTPException } from 'hono/http-exception';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { addApprovedDomainsHandler } from './add-approved-domains';
import {
createTestUserInDb,
createTestOrganizationInDb,
createTestOrgMemberInDb,
cleanupTestUser,
cleanupTestOrganization,
cleanupTestUser,
createTestOrgMemberInDb,
createTestOrganizationInDb,
createTestUserInDb,
getOrganizationFromDb,
} from './test-db-utils';
import { HTTPException } from 'hono/http-exception';
import type { User, Organization } from '@buster/database';
describe('addApprovedDomainsHandler (integration)', () => {
let testUser: User;
@ -37,9 +37,9 @@ describe('addApprovedDomainsHandler (integration)', () => {
const result = await addApprovedDomainsHandler(request, testUser);
expect(result).toHaveLength(3);
expect(result.map(d => d.domain)).toContain('existing.com');
expect(result.map(d => d.domain)).toContain('new.com');
expect(result.map(d => d.domain)).toContain('another.com');
expect(result.map((d) => d.domain)).toContain('existing.com');
expect(result.map((d) => d.domain)).toContain('new.com');
expect(result.map((d) => d.domain)).toContain('another.com');
// Verify database was updated
const updatedOrg = await getOrganizationFromDb(testOrg.id);
@ -56,8 +56,8 @@ describe('addApprovedDomainsHandler (integration)', () => {
const result = await addApprovedDomainsHandler(request, testUser);
expect(result).toHaveLength(2);
expect(result.map(d => d.domain)).toContain('first.com');
expect(result.map(d => d.domain)).toContain('second.com');
expect(result.map((d) => d.domain)).toContain('first.com');
expect(result.map((d) => d.domain)).toContain('second.com');
const updatedOrg = await getOrganizationFromDb(orgWithNoDomains.id);
expect(updatedOrg?.domains).toEqual(['first.com', 'second.com']);
@ -72,7 +72,7 @@ describe('addApprovedDomainsHandler (integration)', () => {
const result = await addApprovedDomainsHandler(request, testUser);
expect(result).toHaveLength(3);
const domains = result.map(d => d.domain);
const domains = result.map((d) => d.domain);
expect(domains).toContain('existing.com');
expect(domains).toContain('new.com');
expect(domains).toContain('test.io');
@ -85,7 +85,7 @@ describe('addApprovedDomainsHandler (integration)', () => {
const result = await addApprovedDomainsHandler(request, testUser);
expect(result).toHaveLength(2);
const domains = result.map(d => d.domain);
const domains = result.map((d) => d.domain);
expect(domains).toEqual(['existing.com', 'new.com']);
const updatedOrg = await getOrganizationFromDb(testOrg.id);
@ -150,7 +150,7 @@ describe('addApprovedDomainsHandler (integration)', () => {
await createTestOrgMemberInDb(testUser.id, testOrg.id, 'workspace_admin');
const originalUpdatedAt = testOrg.updatedAt;
await new Promise(resolve => setTimeout(resolve, 10)); // Ensure time difference
await new Promise((resolve) => setTimeout(resolve, 10)); // Ensure time difference
const request = { domains: ['new.com'] };
await addApprovedDomainsHandler(request, testUser);
@ -162,4 +162,4 @@ describe('addApprovedDomainsHandler (integration)', () => {
);
});
});
});
});

View File

@ -1,9 +1,9 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { addApprovedDomainsHandler } from './add-approved-domains';
import { createTestUser, createTestOrganization } from './test-fixtures';
import * as securityUtils from './security-utils';
import { DomainService } from './domain-service';
import { db } from '@buster/database';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { addApprovedDomainsHandler } from './add-approved-domains';
import { DomainService } from './domain-service';
import * as securityUtils from './security-utils';
import { createTestOrganization, createTestUser } from './test-fixtures';
// Mock dependencies
vi.mock('./security-utils');
@ -32,22 +32,22 @@ describe('addApprovedDomainsHandler', () => {
domains: ['existing.com'],
});
const mockOrgMembership = { organizationId: 'org-123', role: 'workspace_admin' };
beforeEach(() => {
vi.clearAllMocks();
// Setup default mocks
vi.mocked(securityUtils.validateUserOrganization).mockResolvedValue(mockOrgMembership);
vi.mocked(securityUtils.checkAdminPermissions).mockImplementation(() => {});
vi.mocked(securityUtils.fetchOrganization).mockResolvedValue(mockOrg);
// Setup domain service mocks
vi.mocked(DomainService.prototype.mergeDomains).mockReturnValue(['existing.com', 'new.com']);
vi.mocked(DomainService.prototype.formatDomainsResponse).mockReturnValue([
{ domain: 'existing.com', created_at: mockOrg.createdAt },
{ domain: 'new.com', created_at: mockOrg.createdAt },
]);
// Mock database update
const mockDbChain = {
update: vi.fn().mockReturnThis(),
@ -59,19 +59,22 @@ describe('addApprovedDomainsHandler', () => {
it('should add new domains successfully', async () => {
const request = { domains: ['new.com'] };
const result = await addApprovedDomainsHandler(request, mockUser);
expect(securityUtils.validateUserOrganization).toHaveBeenCalledWith(mockUser.id);
expect(securityUtils.checkAdminPermissions).toHaveBeenCalledWith('workspace_admin');
expect(securityUtils.fetchOrganization).toHaveBeenCalledWith('org-123');
expect(DomainService.prototype.mergeDomains).toHaveBeenCalledWith(['existing.com'], ['new.com']);
expect(DomainService.prototype.mergeDomains).toHaveBeenCalledWith(
['existing.com'],
['new.com']
);
expect(DomainService.prototype.formatDomainsResponse).toHaveBeenCalledWith(
['existing.com', 'new.com'],
mockOrg.createdAt
);
expect(result).toEqual([
{ domain: 'existing.com', created_at: mockOrg.createdAt },
{ domain: 'new.com', created_at: mockOrg.createdAt },
@ -81,15 +84,15 @@ describe('addApprovedDomainsHandler', () => {
it('should handle empty existing domains', async () => {
const orgWithNoDomains = { ...mockOrg, domains: null };
vi.mocked(securityUtils.fetchOrganization).mockResolvedValue(orgWithNoDomains);
vi.mocked(DomainService.prototype.mergeDomains).mockReturnValue(['new.com']);
vi.mocked(DomainService.prototype.formatDomainsResponse).mockReturnValue([
{ domain: 'new.com', created_at: mockOrg.createdAt },
]);
const request = { domains: ['new.com'] };
const result = await addApprovedDomainsHandler(request, mockUser);
expect(DomainService.prototype.mergeDomains).toHaveBeenCalledWith([], ['new.com']);
expect(result).toEqual([{ domain: 'new.com', created_at: mockOrg.createdAt }]);
});
@ -102,9 +105,9 @@ describe('addApprovedDomainsHandler', () => {
where: vi.fn().mockResolvedValue(undefined),
};
vi.mocked(db.update).mockReturnValue(mockDbChain as any);
await addApprovedDomainsHandler(request, mockUser);
expect(db.update).toHaveBeenCalled();
expect(mockDbChain.set).toHaveBeenCalledWith({
domains: ['existing.com', 'new.com'],
@ -116,9 +119,9 @@ describe('addApprovedDomainsHandler', () => {
vi.mocked(securityUtils.validateUserOrganization).mockRejectedValue(
new Error('User not in organization')
);
const request = { domains: ['new.com'] };
await expect(addApprovedDomainsHandler(request, mockUser)).rejects.toThrow(
'User not in organization'
);
@ -128,11 +131,11 @@ describe('addApprovedDomainsHandler', () => {
vi.mocked(securityUtils.checkAdminPermissions).mockImplementation(() => {
throw new Error('Insufficient permissions');
});
const request = { domains: ['new.com'] };
await expect(addApprovedDomainsHandler(request, mockUser)).rejects.toThrow(
'Insufficient permissions'
);
});
});
});

View File

@ -1,14 +1,14 @@
import { type User, and, db, eq, isNull, organizations } from '@buster/database';
import type {
AddApprovedDomainRequest,
AddApprovedDomainsResponse,
} from '@buster/server-shared/security';
import { type User, db, organizations, eq, and, isNull } from '@buster/database';
import {
validateUserOrganization,
fetchOrganization,
checkAdminPermissions
} from './security-utils';
import { DomainService } from './domain-service';
import {
checkAdminPermissions,
fetchOrganization,
validateUserOrganization,
} from './security-utils';
const domainService = new DomainService();
@ -23,7 +23,7 @@ export async function addApprovedDomainsHandler(
// Fetch current organization
const org = await fetchOrganization(userOrg.organizationId);
const currentDomains = org.domains || [];
// Merge domains using domain service
const updatedDomains = domainService.mergeDomains(currentDomains, request.domains);
@ -34,20 +34,12 @@ export async function addApprovedDomainsHandler(
return domainService.formatDomainsResponse(updatedDomains, org.createdAt);
}
async function updateOrganizationDomains(
organizationId: string,
domains: string[]
): Promise<void> {
async function updateOrganizationDomains(organizationId: string, domains: string[]): Promise<void> {
await db
.update(organizations)
.set({
domains,
updatedAt: new Date().toISOString(),
})
.where(
and(
eq(organizations.id, organizationId),
isNull(organizations.deletedAt)
)
);
}
.where(and(eq(organizations.id, organizationId), isNull(organizations.deletedAt)));
}

View File

@ -1,4 +1,4 @@
import { describe, it, expect } from 'vitest';
import { describe, expect, it } from 'vitest';
import { DomainService } from './domain-service';
describe('DomainService', () => {
@ -139,4 +139,4 @@ describe('DomainService', () => {
]);
});
});
});
});

View File

@ -1,25 +1,25 @@
export class DomainService {
normalizeDomains(domains: string[]): string[] {
return domains.map(d => d.toLowerCase().trim());
return domains.map((d) => d.toLowerCase().trim());
}
mergeDomains(currentDomains: string[], newDomains: string[]): string[] {
const normalizedCurrent = this.normalizeDomains(currentDomains);
const normalizedNew = this.normalizeDomains(newDomains);
// Filter out duplicates from new domains
const uniqueNew = normalizedNew.filter(d => !normalizedCurrent.includes(d));
const uniqueNew = normalizedNew.filter((d) => !normalizedCurrent.includes(d));
// Return original current domains plus unique new ones
return [...currentDomains, ...uniqueNew];
}
filterDomains(currentDomains: string[], domainsToRemove: string[]): string[] {
const normalizedToRemove = this.normalizeDomains(domainsToRemove);
// Filter out domains that match (case-insensitive)
return currentDomains.filter(
domain => !normalizedToRemove.includes(domain.toLowerCase().trim())
(domain) => !normalizedToRemove.includes(domain.toLowerCase().trim())
);
}
@ -30,13 +30,13 @@ export class DomainService {
if (!domains || domains.length === 0) {
return [];
}
// Convert PostgreSQL timestamp to ISO string format
const isoCreatedAt = new Date(createdAt).toISOString();
return domains.map(domain => ({
return domains.map((domain) => ({
domain,
created_at: isoCreatedAt,
}));
}
}
}

View File

@ -1,15 +1,15 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { db, eq, organizations } from '@buster/database';
import type { Organization, User } from '@buster/database';
import { HTTPException } from 'hono/http-exception';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { getApprovedDomainsHandler } from './get-approved-domains';
import {
createTestUserInDb,
createTestOrganizationInDb,
createTestOrgMemberInDb,
cleanupTestUser,
cleanupTestOrganization,
cleanupTestUser,
createTestOrgMemberInDb,
createTestOrganizationInDb,
createTestUserInDb,
} from './test-db-utils';
import { HTTPException } from 'hono/http-exception';
import { db, organizations, eq } from '@buster/database';
import type { User, Organization } from '@buster/database';
describe('getApprovedDomainsHandler (integration)', () => {
let testUser: User;
@ -36,8 +36,8 @@ describe('getApprovedDomainsHandler (integration)', () => {
const result = await getApprovedDomainsHandler(testUser);
expect(result).toHaveLength(2);
expect(result.map(d => d.domain)).toContain('example.com');
expect(result.map(d => d.domain)).toContain('test.io');
expect(result.map((d) => d.domain)).toContain('example.com');
expect(result.map((d) => d.domain)).toContain('test.io');
});
it('should return empty array for org with no domains', async () => {
@ -76,8 +76,8 @@ describe('getApprovedDomainsHandler (integration)', () => {
const result = await getApprovedDomainsHandler(roleUser);
expect(result).toHaveLength(2);
expect(result.map(d => d.domain)).toContain('example.com');
expect(result.map(d => d.domain)).toContain('test.io');
expect(result.map((d) => d.domain)).toContain('example.com');
expect(result.map((d) => d.domain)).toContain('test.io');
await cleanupTestUser(roleUser.id);
}
@ -138,7 +138,9 @@ describe('getApprovedDomainsHandler (integration)', () => {
.limit(1);
expect(orgAfter[0]?.domains).toEqual(originalOrg.domains);
expect(new Date(orgAfter[0]?.updatedAt).toISOString()).toEqual(new Date(originalOrg.updatedAt).toISOString());
expect(new Date(orgAfter[0]?.updatedAt).toISOString()).toEqual(
new Date(originalOrg.updatedAt).toISOString()
);
});
it('should return domains in the correct format', async () => {
@ -146,7 +148,7 @@ describe('getApprovedDomainsHandler (integration)', () => {
const result = await getApprovedDomainsHandler(testUser);
result.forEach(domainEntry => {
result.forEach((domainEntry) => {
expect(domainEntry).toHaveProperty('domain');
expect(domainEntry).toHaveProperty('created_at');
expect(typeof domainEntry.domain).toBe('string');
@ -157,4 +159,4 @@ describe('getApprovedDomainsHandler (integration)', () => {
});
});
});
});
});

View File

@ -1,8 +1,8 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { getApprovedDomainsHandler } from './get-approved-domains';
import { createTestUser, createTestOrganization } from './test-fixtures';
import * as securityUtils from './security-utils';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { DomainService } from './domain-service';
import { getApprovedDomainsHandler } from './get-approved-domains';
import * as securityUtils from './security-utils';
import { createTestOrganization, createTestUser } from './test-fixtures';
// Mock dependencies
vi.mock('./security-utils');
@ -19,14 +19,14 @@ describe('getApprovedDomainsHandler', () => {
domains: ['example.com', 'test.io'],
});
const mockOrgMembership = { organizationId: 'org-123', role: 'member' };
beforeEach(() => {
vi.clearAllMocks();
// Setup default mocks
vi.mocked(securityUtils.validateUserOrganization).mockResolvedValue(mockOrgMembership);
vi.mocked(securityUtils.fetchOrganization).mockResolvedValue(mockOrg);
// Setup domain service mocks
vi.mocked(DomainService.prototype.formatDomainsResponse).mockReturnValue([
{ domain: 'example.com', created_at: mockOrg.createdAt },
@ -36,15 +36,15 @@ describe('getApprovedDomainsHandler', () => {
it('should return domains for valid organization', async () => {
const result = await getApprovedDomainsHandler(mockUser);
expect(securityUtils.validateUserOrganization).toHaveBeenCalledWith(mockUser.id);
expect(securityUtils.fetchOrganization).toHaveBeenCalledWith('org-123');
expect(DomainService.prototype.formatDomainsResponse).toHaveBeenCalledWith(
['example.com', 'test.io'],
mockOrg.createdAt
);
expect(result).toEqual([
{ domain: 'example.com', created_at: mockOrg.createdAt },
{ domain: 'test.io', created_at: mockOrg.createdAt },
@ -54,12 +54,15 @@ describe('getApprovedDomainsHandler', () => {
it('should return empty array for org with no domains', async () => {
const orgWithNoDomains = { ...mockOrg, domains: null };
vi.mocked(securityUtils.fetchOrganization).mockResolvedValue(orgWithNoDomains);
vi.mocked(DomainService.prototype.formatDomainsResponse).mockReturnValue([]);
const result = await getApprovedDomainsHandler(mockUser);
expect(DomainService.prototype.formatDomainsResponse).toHaveBeenCalledWith(null, mockOrg.createdAt);
expect(DomainService.prototype.formatDomainsResponse).toHaveBeenCalledWith(
null,
mockOrg.createdAt
);
expect(result).toEqual([]);
});
@ -67,33 +70,29 @@ describe('getApprovedDomainsHandler', () => {
vi.mocked(securityUtils.validateUserOrganization).mockRejectedValue(
new Error('User not in organization')
);
await expect(getApprovedDomainsHandler(mockUser)).rejects.toThrow(
'User not in organization'
);
await expect(getApprovedDomainsHandler(mockUser)).rejects.toThrow('User not in organization');
});
it('should handle organization fetch errors', async () => {
vi.mocked(securityUtils.fetchOrganization).mockRejectedValue(
new Error('Organization not found')
);
await expect(getApprovedDomainsHandler(mockUser)).rejects.toThrow(
'Organization not found'
);
await expect(getApprovedDomainsHandler(mockUser)).rejects.toThrow('Organization not found');
});
it('should not require admin permissions', async () => {
// Test with non-admin role
const nonAdminMembership = { organizationId: 'org-123', role: 'member' };
vi.mocked(securityUtils.validateUserOrganization).mockResolvedValue(nonAdminMembership);
const result = await getApprovedDomainsHandler(mockUser);
// Should still succeed
expect(result).toEqual([
{ domain: 'example.com', created_at: mockOrg.createdAt },
{ domain: 'test.io', created_at: mockOrg.createdAt },
]);
});
});
});

View File

@ -1,13 +1,11 @@
import type { User } from '@buster/database';
import type { GetApprovedDomainsResponse } from '@buster/server-shared/security';
import { type User } from '@buster/database';
import { validateUserOrganization, fetchOrganization } from './security-utils';
import { DomainService } from './domain-service';
import { fetchOrganization, validateUserOrganization } from './security-utils';
const domainService = new DomainService();
export async function getApprovedDomainsHandler(
user: User
): Promise<GetApprovedDomainsResponse> {
export async function getApprovedDomainsHandler(user: User): Promise<GetApprovedDomainsResponse> {
// Validate user organization
const userOrg = await validateUserOrganization(user.id);
@ -16,4 +14,4 @@ export async function getApprovedDomainsHandler(
// Return formatted domains response
return domainService.formatDomainsResponse(org.domains, org.createdAt);
}
}

View File

@ -1,15 +1,15 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { db, eq, organizations } from '@buster/database';
import type { Organization, User } from '@buster/database';
import { HTTPException } from 'hono/http-exception';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { getWorkspaceSettingsHandler } from './get-workspace-settings';
import {
createTestUserInDb,
createTestOrganizationInDb,
createTestOrgMemberInDb,
cleanupTestUser,
cleanupTestOrganization,
cleanupTestUser,
createTestOrgMemberInDb,
createTestOrganizationInDb,
createTestUserInDb,
} from './test-db-utils';
import { HTTPException } from 'hono/http-exception';
import { db, organizations, eq } from '@buster/database';
import type { User, Organization } from '@buster/database';
describe('getWorkspaceSettingsHandler (integration)', () => {
let testUser: User;
@ -160,7 +160,9 @@ describe('getWorkspaceSettingsHandler (integration)', () => {
.where(eq(organizations.id, testOrg.id))
.limit(1);
expect(orgAfter[0]?.restrictNewUserInvitations).toEqual(originalOrg.restrictNewUserInvitations);
expect(orgAfter[0]?.restrictNewUserInvitations).toEqual(
originalOrg.restrictNewUserInvitations
);
expect(orgAfter[0]?.defaultRole).toEqual(originalOrg.defaultRole);
expect(new Date(orgAfter[0]?.updatedAt).toISOString()).toEqual(originalOrg.updatedAt);
});
@ -191,4 +193,4 @@ describe('getWorkspaceSettingsHandler (integration)', () => {
expect(Array.isArray(result.default_datasets)).toBe(true);
});
});
});
});

View File

@ -1,7 +1,7 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { getWorkspaceSettingsHandler } from './get-workspace-settings';
import { createTestUser, createTestOrganization } from './test-fixtures';
import * as securityUtils from './security-utils';
import { createTestOrganization, createTestUser } from './test-fixtures';
import { WorkspaceSettingsService } from './workspace-settings-service';
// Mock dependencies
@ -20,37 +20,46 @@ describe('getWorkspaceSettingsHandler', () => {
defaultRole: 'restricted_querier',
});
const mockOrgMembership = { organizationId: 'org-123', role: 'member' };
const mockDefaultDatasets = [
{ id: 'dataset-1', name: 'Sales Data' },
{ id: 'dataset-2', name: 'Customer Data' },
];
beforeEach(() => {
vi.clearAllMocks();
// Setup default mocks
vi.mocked(securityUtils.validateUserOrganization).mockResolvedValue(mockOrgMembership);
vi.mocked(securityUtils.fetchOrganization).mockResolvedValue(mockOrg);
vi.mocked(securityUtils.fetchDefaultDatasets).mockResolvedValue(mockDefaultDatasets);
// Setup workspace settings service mocks
vi.mocked(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).mockReturnValue({
restrict_new_user_invitations: true,
default_role: 'restricted_querier',
default_datasets: [],
default_datasets: mockDefaultDatasets,
});
});
it('should return settings for valid organization', async () => {
const result = await getWorkspaceSettingsHandler(mockUser);
expect(securityUtils.validateUserOrganization).toHaveBeenCalledWith(mockUser.id);
expect(securityUtils.fetchOrganization).toHaveBeenCalledWith('org-123');
expect(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).toHaveBeenCalledWith({
restrictNewUserInvitations: true,
defaultRole: 'restricted_querier',
});
expect(securityUtils.fetchDefaultDatasets).toHaveBeenCalledWith('org-123');
expect(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).toHaveBeenCalledWith(
{
restrictNewUserInvitations: true,
defaultRole: 'restricted_querier',
defaultDatasets: mockDefaultDatasets,
}
);
expect(result).toEqual({
restrict_new_user_invitations: true,
default_role: 'restricted_querier',
default_datasets: [],
default_datasets: mockDefaultDatasets,
});
});
@ -61,24 +70,27 @@ describe('getWorkspaceSettingsHandler', () => {
defaultRole: 'data_admin',
};
vi.mocked(securityUtils.fetchOrganization).mockResolvedValue(orgWithDifferentSettings);
vi.mocked(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).mockReturnValue({
restrict_new_user_invitations: false,
default_role: 'data_admin',
default_datasets: [],
default_datasets: mockDefaultDatasets,
});
const result = await getWorkspaceSettingsHandler(mockUser);
expect(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).toHaveBeenCalledWith({
restrictNewUserInvitations: false,
defaultRole: 'data_admin',
});
expect(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).toHaveBeenCalledWith(
{
restrictNewUserInvitations: false,
defaultRole: 'data_admin',
defaultDatasets: mockDefaultDatasets,
}
);
expect(result).toEqual({
restrict_new_user_invitations: false,
default_role: 'data_admin',
default_datasets: [],
default_datasets: mockDefaultDatasets,
});
});
@ -86,9 +98,9 @@ describe('getWorkspaceSettingsHandler', () => {
vi.mocked(securityUtils.validateUserOrganization).mockRejectedValue(
new Error('User not in organization')
);
await expect(getWorkspaceSettingsHandler(mockUser)).rejects.toThrow(
'User not in organization'
'Failed to fetch workspace settings'
);
});
@ -96,36 +108,64 @@ describe('getWorkspaceSettingsHandler', () => {
vi.mocked(securityUtils.fetchOrganization).mockRejectedValue(
new Error('Organization not found')
);
await expect(getWorkspaceSettingsHandler(mockUser)).rejects.toThrow(
'Organization not found'
'Failed to fetch workspace settings'
);
});
it('should not require admin permissions', async () => {
// Test with various non-admin roles
const roles = ['querier', 'restricted_querier', 'viewer'];
for (const role of roles) {
vi.clearAllMocks();
const membership = { organizationId: 'org-123', role };
vi.mocked(securityUtils.validateUserOrganization).mockResolvedValue(membership);
vi.mocked(securityUtils.fetchOrganization).mockResolvedValue(mockOrg);
vi.mocked(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).mockReturnValue({
restrict_new_user_invitations: true,
default_role: 'restricted_querier',
default_datasets: [],
});
vi.mocked(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).mockReturnValue(
{
restrict_new_user_invitations: true,
default_role: 'restricted_querier',
default_datasets: mockDefaultDatasets,
}
);
const result = await getWorkspaceSettingsHandler(mockUser);
// Should still succeed without admin permissions
expect(result).toEqual({
restrict_new_user_invitations: true,
default_role: 'restricted_querier',
default_datasets: [],
default_datasets: mockDefaultDatasets,
});
}
});
});
it('should handle empty default datasets', async () => {
vi.mocked(securityUtils.fetchDefaultDatasets).mockResolvedValue([]);
vi.mocked(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).mockReturnValue({
restrict_new_user_invitations: true,
default_role: 'restricted_querier',
default_datasets: [],
});
const result = await getWorkspaceSettingsHandler(mockUser);
expect(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).toHaveBeenCalledWith(
{
restrictNewUserInvitations: true,
defaultRole: 'restricted_querier',
defaultDatasets: [],
}
);
expect(result).toEqual({
restrict_new_user_invitations: true,
default_role: 'restricted_querier',
default_datasets: [],
});
});
});

View File

@ -1,6 +1,11 @@
import type { User } from '@buster/database';
import type { GetWorkspaceSettingsResponse } from '@buster/server-shared/security';
import { type User } from '@buster/database';
import { validateUserOrganization, fetchOrganization } from './security-utils';
import { HTTPException } from 'hono/http-exception';
import {
fetchDefaultDatasets,
fetchOrganization,
validateUserOrganization,
} from './security-utils';
import { WorkspaceSettingsService } from './workspace-settings-service';
const settingsService = new WorkspaceSettingsService();
@ -8,15 +13,35 @@ const settingsService = new WorkspaceSettingsService();
export async function getWorkspaceSettingsHandler(
user: User
): Promise<GetWorkspaceSettingsResponse> {
// Validate user organization
const userOrg = await validateUserOrganization(user.id);
try {
// Validate user organization
const userOrg = await validateUserOrganization(user.id);
// Fetch organization
const org = await fetchOrganization(userOrg.organizationId);
// Fetch organization and default datasets concurrently
const [org, defaultDatasets] = await Promise.all([
fetchOrganization(userOrg.organizationId),
fetchDefaultDatasets(userOrg.organizationId),
]);
// Return formatted settings response
return settingsService.formatWorkspaceSettingsResponse({
restrictNewUserInvitations: org.restrictNewUserInvitations,
defaultRole: org.defaultRole,
});
}
// Return formatted settings response
return settingsService.formatWorkspaceSettingsResponse({
restrictNewUserInvitations: org.restrictNewUserInvitations,
defaultRole: org.defaultRole,
defaultDatasets,
});
} catch (error) {
console.error('Error in getWorkspaceSettingsHandler:', {
userId: user.id,
error: error instanceof Error ? error.message : error,
});
// Re-throw HTTPException as is, wrap other errors
if (error instanceof HTTPException) {
throw error;
}
throw new HTTPException(500, {
message: 'Failed to fetch workspace settings',
});
}
}

View File

@ -1,15 +1,15 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import type { Organization, User } from '@buster/database';
import { HTTPException } from 'hono/http-exception';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import { removeApprovedDomainsHandler } from './remove-approved-domains';
import {
createTestUserInDb,
createTestOrganizationInDb,
createTestOrgMemberInDb,
cleanupTestUser,
cleanupTestOrganization,
cleanupTestUser,
createTestOrgMemberInDb,
createTestOrganizationInDb,
createTestUserInDb,
getOrganizationFromDb,
} from './test-db-utils';
import { HTTPException } from 'hono/http-exception';
import type { User, Organization } from '@buster/database';
describe('removeApprovedDomainsHandler (integration)', () => {
let testUser: User;
@ -37,8 +37,8 @@ describe('removeApprovedDomainsHandler (integration)', () => {
const result = await removeApprovedDomainsHandler(request, testUser);
expect(result).toHaveLength(2);
expect(result.map(d => d.domain)).toContain('keep.com');
expect(result.map(d => d.domain)).toContain('stay.com');
expect(result.map((d) => d.domain)).toContain('keep.com');
expect(result.map((d) => d.domain)).toContain('stay.com');
// Verify database was updated
const updatedOrg = await getOrganizationFromDb(testOrg.id);
@ -52,7 +52,7 @@ describe('removeApprovedDomainsHandler (integration)', () => {
const result = await removeApprovedDomainsHandler(request, testUser);
expect(result).toHaveLength(4); // All original domains remain
const domains = result.map(d => d.domain);
const domains = result.map((d) => d.domain);
expect(domains).toContain('remove1.com');
expect(domains).toContain('remove2.com');
expect(domains).toContain('keep.com');
@ -69,7 +69,7 @@ describe('removeApprovedDomainsHandler (integration)', () => {
const result = await removeApprovedDomainsHandler(request, testUser);
expect(result).toHaveLength(3);
const domains = result.map(d => d.domain);
const domains = result.map((d) => d.domain);
expect(domains).not.toContain('remove1.com');
expect(domains).toContain('remove2.com');
expect(domains).toContain('keep.com');
@ -83,7 +83,7 @@ describe('removeApprovedDomainsHandler (integration)', () => {
const result = await removeApprovedDomainsHandler(request, testUser);
expect(result).toHaveLength(2);
const domains = result.map(d => d.domain);
const domains = result.map((d) => d.domain);
expect(domains).toEqual(['keep.com', 'stay.com']);
});
@ -94,7 +94,7 @@ describe('removeApprovedDomainsHandler (integration)', () => {
const result = await removeApprovedDomainsHandler(request, testUser);
expect(result).toHaveLength(2);
const domains = result.map(d => d.domain);
const domains = result.map((d) => d.domain);
expect(domains).toEqual(['keep.com', 'stay.com']);
});
});
@ -161,7 +161,7 @@ describe('removeApprovedDomainsHandler (integration)', () => {
await createTestOrgMemberInDb(testUser.id, testOrg.id, 'workspace_admin');
const originalUpdatedAt = testOrg.updatedAt;
await new Promise(resolve => setTimeout(resolve, 10)); // Ensure time difference
await new Promise((resolve) => setTimeout(resolve, 10)); // Ensure time difference
const request = { domains: ['remove1.com'] };
await removeApprovedDomainsHandler(request, testUser);
@ -190,4 +190,4 @@ describe('removeApprovedDomainsHandler (integration)', () => {
await cleanupTestOrganization(orgWithFewDomains.id);
});
});
});
});

View File

@ -1,9 +1,9 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { removeApprovedDomainsHandler } from './remove-approved-domains';
import { createTestUser, createTestOrganization } from './test-fixtures';
import * as securityUtils from './security-utils';
import { DomainService } from './domain-service';
import { db } from '@buster/database';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import { DomainService } from './domain-service';
import { removeApprovedDomainsHandler } from './remove-approved-domains';
import * as securityUtils from './security-utils';
import { createTestOrganization, createTestUser } from './test-fixtures';
// Mock dependencies
vi.mock('./security-utils');
@ -32,21 +32,21 @@ describe('removeApprovedDomainsHandler', () => {
domains: ['example.com', 'test.io', 'keep.com'],
});
const mockOrgMembership = { organizationId: 'org-123', role: 'workspace_admin' };
beforeEach(() => {
vi.clearAllMocks();
// Setup default mocks
vi.mocked(securityUtils.validateUserOrganization).mockResolvedValue(mockOrgMembership);
vi.mocked(securityUtils.checkAdminPermissions).mockImplementation(() => {});
vi.mocked(securityUtils.fetchOrganization).mockResolvedValue(mockOrg);
// Setup domain service mocks
vi.mocked(DomainService.prototype.filterDomains).mockReturnValue(['keep.com']);
vi.mocked(DomainService.prototype.formatDomainsResponse).mockReturnValue([
{ domain: 'keep.com', created_at: mockOrg.createdAt },
]);
// Mock database update
const mockDbChain = {
update: vi.fn().mockReturnThis(),
@ -58,13 +58,13 @@ describe('removeApprovedDomainsHandler', () => {
it('should remove specified domains', async () => {
const request = { domains: ['example.com', 'test.io'] };
const result = await removeApprovedDomainsHandler(request, mockUser);
expect(securityUtils.validateUserOrganization).toHaveBeenCalledWith(mockUser.id);
expect(securityUtils.checkAdminPermissions).toHaveBeenCalledWith('workspace_admin');
expect(securityUtils.fetchOrganization).toHaveBeenCalledWith('org-123');
expect(DomainService.prototype.filterDomains).toHaveBeenCalledWith(
['example.com', 'test.io', 'keep.com'],
['example.com', 'test.io']
@ -73,23 +73,25 @@ describe('removeApprovedDomainsHandler', () => {
['keep.com'],
mockOrg.createdAt
);
expect(result).toEqual([
{ domain: 'keep.com', created_at: mockOrg.createdAt },
]);
expect(result).toEqual([{ domain: 'keep.com', created_at: mockOrg.createdAt }]);
});
it('should handle non-existent domain removal gracefully', async () => {
vi.mocked(DomainService.prototype.filterDomains).mockReturnValue(['example.com', 'test.io', 'keep.com']);
vi.mocked(DomainService.prototype.filterDomains).mockReturnValue([
'example.com',
'test.io',
'keep.com',
]);
vi.mocked(DomainService.prototype.formatDomainsResponse).mockReturnValue([
{ domain: 'example.com', created_at: mockOrg.createdAt },
{ domain: 'test.io', created_at: mockOrg.createdAt },
{ domain: 'keep.com', created_at: mockOrg.createdAt },
]);
const request = { domains: ['notfound.com'] };
const result = await removeApprovedDomainsHandler(request, mockUser);
expect(DomainService.prototype.filterDomains).toHaveBeenCalledWith(
['example.com', 'test.io', 'keep.com'],
['notfound.com']
@ -105,9 +107,9 @@ describe('removeApprovedDomainsHandler', () => {
where: vi.fn().mockResolvedValue(undefined),
};
vi.mocked(db.update).mockReturnValue(mockDbChain as any);
await removeApprovedDomainsHandler(request, mockUser);
expect(db.update).toHaveBeenCalled();
expect(mockDbChain.set).toHaveBeenCalledWith({
domains: ['keep.com'],
@ -118,13 +120,13 @@ describe('removeApprovedDomainsHandler', () => {
it('should handle empty domains array', async () => {
const orgWithNoDomains = { ...mockOrg, domains: null };
vi.mocked(securityUtils.fetchOrganization).mockResolvedValue(orgWithNoDomains);
vi.mocked(DomainService.prototype.filterDomains).mockReturnValue([]);
vi.mocked(DomainService.prototype.formatDomainsResponse).mockReturnValue([]);
const request = { domains: ['example.com'] };
const result = await removeApprovedDomainsHandler(request, mockUser);
expect(DomainService.prototype.filterDomains).toHaveBeenCalledWith([], ['example.com']);
expect(result).toEqual([]);
});
@ -133,9 +135,9 @@ describe('removeApprovedDomainsHandler', () => {
vi.mocked(securityUtils.validateUserOrganization).mockRejectedValue(
new Error('User not in organization')
);
const request = { domains: ['example.com'] };
await expect(removeApprovedDomainsHandler(request, mockUser)).rejects.toThrow(
'User not in organization'
);
@ -145,11 +147,11 @@ describe('removeApprovedDomainsHandler', () => {
vi.mocked(securityUtils.checkAdminPermissions).mockImplementation(() => {
throw new Error('Insufficient permissions');
});
const request = { domains: ['example.com'] };
await expect(removeApprovedDomainsHandler(request, mockUser)).rejects.toThrow(
'Insufficient permissions'
);
});
});
});

View File

@ -1,10 +1,13 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { HTTPException } from 'hono/http-exception';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import {
validateUserOrganization,
fetchOrganization,
checkAdminPermissions,
checkWorkspaceAdminPermission,
ensureDefaultPermissionGroup,
fetchDefaultDatasets,
fetchOrganization,
updateDefaultDatasets,
validateUserOrganization,
} from './security-utils';
import { createTestOrganization } from './test-fixtures';
@ -16,11 +19,20 @@ vi.mock('@buster/database', () => ({
from: vi.fn(),
where: vi.fn(),
limit: vi.fn(),
insert: vi.fn(),
update: vi.fn(),
transaction: vi.fn(),
},
organizations: {},
datasets: {},
datasetsToPermissionGroups: {},
permissionGroups: {},
eq: vi.fn(),
and: vi.fn(),
isNull: vi.fn(),
}));
import { getUserOrganizationId, db } from '@buster/database';
import { and, db, eq, getUserOrganizationId, isNull } from '@buster/database';
describe('security-utils', () => {
beforeEach(() => {
@ -110,16 +122,20 @@ describe('security-utils', () => {
it('should reject other roles', () => {
const roles = ['member', 'viewer', 'user', 'guest'];
roles.forEach(role => {
roles.forEach((role) => {
expect(() => checkAdminPermissions(role)).toThrow(HTTPException);
expect(() => checkAdminPermissions(role)).toThrow('Insufficient permissions to manage approved domains');
expect(() => checkAdminPermissions(role)).toThrow(
'Insufficient permissions to manage approved domains'
);
});
});
it('should reject null role', () => {
expect(() => checkAdminPermissions(null)).toThrow(HTTPException);
expect(() => checkAdminPermissions(null)).toThrow('Insufficient permissions to manage approved domains');
expect(() => checkAdminPermissions(null)).toThrow(
'Insufficient permissions to manage approved domains'
);
});
it('should reject undefined role', () => {
@ -134,21 +150,414 @@ describe('security-utils', () => {
it('should reject data_admin role', () => {
expect(() => checkWorkspaceAdminPermission('data_admin')).toThrow(HTTPException);
expect(() => checkWorkspaceAdminPermission('data_admin')).toThrow('Only workspace admins can update workspace settings');
expect(() => checkWorkspaceAdminPermission('data_admin')).toThrow(
'Only workspace admins can update workspace settings'
);
});
it('should reject other roles', () => {
const roles = ['admin', 'member', 'viewer', 'user'];
roles.forEach(role => {
roles.forEach((role) => {
expect(() => checkWorkspaceAdminPermission(role)).toThrow(HTTPException);
expect(() => checkWorkspaceAdminPermission(role)).toThrow('Only workspace admins can update workspace settings');
expect(() => checkWorkspaceAdminPermission(role)).toThrow(
'Only workspace admins can update workspace settings'
);
});
});
it('should reject null role', () => {
expect(() => checkWorkspaceAdminPermission(null)).toThrow(HTTPException);
expect(() => checkWorkspaceAdminPermission(null)).toThrow('Only workspace admins can update workspace settings');
expect(() => checkWorkspaceAdminPermission(null)).toThrow(
'Only workspace admins can update workspace settings'
);
});
});
});
describe('fetchDefaultDatasets', () => {
it('should return datasets from default permission group', async () => {
const mockDatasets = [
{ id: 'dataset-1', name: 'Sales Data' },
{ id: 'dataset-2', name: 'Customer Data' },
];
const mockDbChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
innerJoin: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(mockDatasets),
};
vi.mocked(db.select).mockReturnValue(mockDbChain as any);
const result = await fetchDefaultDatasets('org-123');
expect(result).toEqual(mockDatasets);
expect(mockDbChain.innerJoin).toHaveBeenCalledTimes(2);
});
it('should return empty array when no default datasets', async () => {
const mockDbChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
innerJoin: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue([]),
};
vi.mocked(db.select).mockReturnValue(mockDbChain as any);
const result = await fetchDefaultDatasets('org-123');
expect(result).toEqual([]);
});
});
describe('ensureDefaultPermissionGroup', () => {
it('should return existing permission group ID', async () => {
const mockExistingGroup = [{ id: 'pg-123', name: 'default:org-123' }];
const mockDbChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue(mockExistingGroup),
};
vi.mocked(db.select).mockReturnValue(mockDbChain as any);
const result = await ensureDefaultPermissionGroup('org-123', 'user-123');
expect(result).toBe('pg-123');
expect(db.insert).not.toHaveBeenCalled();
});
it('should create new permission group when not exists', async () => {
const mockDbSelectChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([]),
};
const mockDbInsertChain = {
insert: vi.fn().mockReturnThis(),
values: vi.fn().mockReturnThis(),
returning: vi.fn().mockResolvedValue([{ id: 'pg-new' }]),
};
vi.mocked(db.select).mockReturnValue(mockDbSelectChain as any);
vi.mocked(db.insert).mockReturnValue(mockDbInsertChain as any);
const result = await ensureDefaultPermissionGroup('org-123', 'user-123');
expect(result).toBe('pg-new');
expect(mockDbInsertChain.values).toHaveBeenCalledWith({
name: 'default:org-123',
organizationId: 'org-123',
createdBy: 'user-123',
updatedBy: 'user-123',
});
});
it('should restore soft deleted permission group', async () => {
const mockExistingGroup = [
{
id: 'pg-123',
name: 'default:org-123',
deletedAt: '2024-01-01T00:00:00Z',
},
];
const mockDbSelectChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue(mockExistingGroup),
};
const mockDbUpdateChain = {
update: vi.fn().mockReturnThis(),
set: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(undefined),
};
vi.mocked(db.select).mockReturnValue(mockDbSelectChain as any);
vi.mocked(db.update).mockReturnValue(mockDbUpdateChain as any);
const result = await ensureDefaultPermissionGroup('org-123', 'user-123');
expect(result).toBe('pg-123');
expect(db.update).toHaveBeenCalled();
expect(mockDbUpdateChain.set).toHaveBeenCalledWith({
deletedAt: null,
updatedBy: 'user-123',
updatedAt: expect.any(String),
});
});
it('should throw error when insert fails', async () => {
const mockDbSelectChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([]),
};
const mockDbInsertChain = {
insert: vi.fn().mockReturnThis(),
values: vi.fn().mockReturnThis(),
returning: vi.fn().mockResolvedValue([]),
};
vi.mocked(db.select).mockReturnValue(mockDbSelectChain as any);
vi.mocked(db.insert).mockReturnValue(mockDbInsertChain as any);
await expect(ensureDefaultPermissionGroup('org-123', 'user-123')).rejects.toThrow(
HTTPException
);
await expect(ensureDefaultPermissionGroup('org-123', 'user-123')).rejects.toMatchObject({
status: 500,
message: 'Failed to create default permission group',
});
});
});
describe('updateDefaultDatasets', () => {
it('should update datasets with specific IDs', async () => {
// Mock ensureDefaultPermissionGroup
const mockDbSelectChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([{ id: 'pg-123' }]),
};
// Mock valid datasets query
const mockValidDatasets = [{ id: 'dataset-1' }, { id: 'dataset-2' }];
vi.mocked(db.select)
.mockReturnValueOnce(mockDbSelectChain as any) // For permission group
.mockReturnValueOnce({
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(mockValidDatasets),
} as any); // For dataset validation
// Mock transaction
const mockTx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([]), // No existing records
}),
update: vi.fn().mockReturnValue({
set: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(undefined),
}),
insert: vi.fn().mockReturnValue({
values: vi.fn().mockReturnThis(),
onConflictDoUpdate: vi.fn().mockResolvedValue(undefined),
}),
};
vi.mocked(db.transaction).mockImplementation(async (callback) => {
return callback(mockTx as any);
});
await updateDefaultDatasets('org-123', ['dataset-1', 'dataset-2'], 'user-123');
expect(mockTx.update).toHaveBeenCalled();
expect(mockTx.insert).toHaveBeenCalled();
});
it('should update datasets with "all" keyword', async () => {
// Mock ensureDefaultPermissionGroup
const mockDbSelectChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([{ id: 'pg-123' }]),
};
// Mock fetching all datasets
const mockAllDatasets = [{ id: 'dataset-1' }, { id: 'dataset-2' }, { id: 'dataset-3' }];
vi.mocked(db.select)
.mockReturnValueOnce(mockDbSelectChain as any) // For permission group
.mockReturnValueOnce({
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(mockAllDatasets),
} as any); // For all datasets
// Mock transaction
const mockTx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([]), // No existing records
}),
update: vi.fn().mockReturnValue({
set: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(undefined),
}),
insert: vi.fn().mockReturnValue({
values: vi.fn().mockReturnThis(),
onConflictDoUpdate: vi.fn().mockResolvedValue(undefined),
}),
};
vi.mocked(db.transaction).mockImplementation(async (callback) => {
return callback(mockTx as any);
});
await updateDefaultDatasets('org-123', 'all', 'user-123');
expect(mockTx.insert).toHaveBeenCalled();
});
it('should handle empty dataset array', async () => {
// Mock ensureDefaultPermissionGroup
const mockDbSelectChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([{ id: 'pg-123' }]),
};
// Mock empty dataset validation query
vi.mocked(db.select)
.mockReturnValueOnce(mockDbSelectChain as any) // For permission group
.mockReturnValueOnce({
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue([]),
} as any); // For dataset validation (empty)
// Mock transaction
const mockTx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([]), // No existing records
}),
update: vi.fn().mockReturnValue({
set: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(undefined),
}),
insert: vi.fn().mockReturnValue({
values: vi.fn().mockReturnThis(),
onConflictDoUpdate: vi.fn().mockResolvedValue(undefined),
}),
};
vi.mocked(db.transaction).mockImplementation(async (callback) => {
return callback(mockTx as any);
});
await updateDefaultDatasets('org-123', [], 'user-123');
expect(mockTx.update).toHaveBeenCalled();
expect(mockTx.insert).not.toHaveBeenCalled();
});
it('should filter out invalid dataset IDs', async () => {
// Mock ensureDefaultPermissionGroup
const mockDbSelectChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([{ id: 'pg-123' }]),
};
// Mock valid datasets query
const mockValidDatasets = [{ id: 'dataset-1' }, { id: 'dataset-2' }];
vi.mocked(db.select)
.mockReturnValueOnce(mockDbSelectChain as any) // For permission group
.mockReturnValueOnce({
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(mockValidDatasets),
} as any); // For dataset validation
// Mock transaction
const mockTx = {
select: vi.fn().mockReturnValue({
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([]), // No existing records
}),
update: vi.fn().mockReturnValue({
set: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(undefined),
}),
insert: vi.fn().mockReturnValue({
values: vi.fn().mockReturnThis(),
onConflictDoUpdate: vi.fn().mockResolvedValue(undefined),
}),
};
vi.mocked(db.transaction).mockImplementation(async (callback) => {
return callback(mockTx as any);
});
// Try to update with some invalid dataset IDs
await updateDefaultDatasets('org-123', ['dataset-1', 'dataset-2', 'invalid-id'], 'user-123');
// Should only insert valid datasets
expect(mockTx.insert).toHaveBeenCalled();
// With the new batch upsert logic, insert is called once with all valid datasets
expect(mockTx.insert).toHaveBeenCalledTimes(1);
});
it('should restore soft-deleted dataset associations', async () => {
// Mock ensureDefaultPermissionGroup
const mockDbSelectChain = {
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockReturnThis(),
limit: vi.fn().mockResolvedValue([{ id: 'pg-123' }]),
};
// Mock valid datasets query
const mockValidDatasets = [{ id: 'dataset-1' }, { id: 'dataset-2' }];
vi.mocked(db.select)
.mockReturnValueOnce(mockDbSelectChain as any) // For permission group
.mockReturnValueOnce({
select: vi.fn().mockReturnThis(),
from: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(mockValidDatasets),
} as any); // For dataset validation
// Mock transaction
const mockTx = {
update: vi.fn().mockReturnValue({
set: vi.fn().mockReturnThis(),
where: vi.fn().mockResolvedValue(undefined),
}),
insert: vi.fn().mockReturnValue({
values: vi.fn().mockReturnThis(),
onConflictDoUpdate: vi.fn().mockResolvedValue(undefined),
}),
};
vi.mocked(db.transaction).mockImplementation(async (callback) => {
return callback(mockTx as any);
});
await updateDefaultDatasets('org-123', ['dataset-1', 'dataset-2'], 'user-123');
// Should soft-delete all existing first, then batch upsert
expect(mockTx.update).toHaveBeenCalledTimes(1); // 1 for soft-delete all
expect(mockTx.insert).toHaveBeenCalledTimes(1); // 1 batch upsert for both datasets
// Verify the insert was called with values
expect(mockTx.insert).toHaveBeenCalled();
const insertMock = vi.mocked(mockTx.insert);
expect(insertMock().values).toHaveBeenCalled();
expect(insertMock().values().onConflictDoUpdate).toHaveBeenCalled();
});
});
});

View File

@ -1,6 +1,13 @@
import { HTTPException } from 'hono/http-exception';
import { db, getUserOrganizationId, organizations } from '@buster/database';
import {
datasets,
datasetsToPermissionGroups,
db,
getUserOrganizationId,
organizations,
permissionGroups,
} from '@buster/database';
import { and, eq, isNull } from 'drizzle-orm';
import { HTTPException } from 'hono/http-exception';
export async function validateUserOrganization(userId: string) {
const userOrg = await getUserOrganizationId(userId);
@ -16,12 +23,7 @@ export async function fetchOrganization(organizationId: string) {
const org = await db
.select()
.from(organizations)
.where(
and(
eq(organizations.id, organizationId),
isNull(organizations.deletedAt)
)
)
.where(and(eq(organizations.id, organizationId), isNull(organizations.deletedAt)))
.limit(1);
if (!org.length || !org[0]) {
@ -29,7 +31,7 @@ export async function fetchOrganization(organizationId: string) {
message: 'Organization not found',
});
}
return org[0];
}
@ -47,4 +49,280 @@ export function checkWorkspaceAdminPermission(role: string | null): void {
message: 'Only workspace admins can update workspace settings',
});
}
}
}
export async function fetchDefaultDatasets(organizationId: string) {
const defaultPermissionGroupName = `default:${organizationId}`;
try {
const defaultDatasets = await db
.select({
id: datasets.id,
name: datasets.name,
})
.from(datasets)
.innerJoin(
datasetsToPermissionGroups,
and(
eq(datasets.id, datasetsToPermissionGroups.datasetId),
isNull(datasetsToPermissionGroups.deletedAt)
)
)
.innerJoin(
permissionGroups,
and(
eq(datasetsToPermissionGroups.permissionGroupId, permissionGroups.id),
eq(permissionGroups.name, defaultPermissionGroupName),
eq(permissionGroups.organizationId, organizationId),
isNull(permissionGroups.deletedAt)
)
)
.where(and(eq(datasets.organizationId, organizationId), isNull(datasets.deletedAt)));
return defaultDatasets;
} catch (error) {
console.error('Error fetching default datasets:', {
organizationId,
permissionGroupName: defaultPermissionGroupName,
error: error instanceof Error ? error.message : error,
});
throw new HTTPException(500, {
message: 'Failed to fetch default datasets',
});
}
}
export async function ensureDefaultPermissionGroup(organizationId: string, userId: string) {
const defaultPermissionGroupName = `default:${organizationId}`;
try {
// Check if it exists (including soft deleted ones)
const existingGroup = await db
.select()
.from(permissionGroups)
.where(
and(
eq(permissionGroups.name, defaultPermissionGroupName),
eq(permissionGroups.organizationId, organizationId)
)
)
.limit(1);
if (existingGroup.length > 0 && existingGroup[0]) {
// If it's soft deleted, restore it
if (existingGroup[0].deletedAt) {
try {
await db
.update(permissionGroups)
.set({
deletedAt: null,
updatedBy: userId,
updatedAt: new Date().toISOString(),
})
.where(eq(permissionGroups.id, existingGroup[0].id));
console.info('Restored soft-deleted default permission group:', {
permissionGroupId: existingGroup[0].id,
organizationId,
});
} catch (error) {
console.error('Error restoring default permission group:', {
permissionGroupId: existingGroup[0].id,
organizationId,
error: error instanceof Error ? error.message : error,
});
throw new HTTPException(500, {
message: 'Failed to restore default permission group',
});
}
}
return existingGroup[0].id;
}
// Create if doesn't exist
const newGroup = await db
.insert(permissionGroups)
.values({
name: defaultPermissionGroupName,
organizationId,
createdBy: userId,
updatedBy: userId,
})
.returning();
if (!newGroup[0]) {
console.error('Failed to create default permission group - no data returned:', {
organizationId,
userId,
permissionGroupName: defaultPermissionGroupName,
});
throw new HTTPException(500, {
message: 'Failed to create default permission group',
});
}
console.info('Created new default permission group:', {
permissionGroupId: newGroup[0].id,
organizationId,
});
return newGroup[0].id;
} catch (error) {
// If it's already an HTTPException, re-throw it
if (error instanceof HTTPException) {
throw error;
}
console.error('Error ensuring default permission group:', {
organizationId,
userId,
permissionGroupName: defaultPermissionGroupName,
error: error instanceof Error ? error.message : error,
});
throw new HTTPException(500, {
message: 'Failed to ensure default permission group exists',
});
}
}
export async function updateDefaultDatasets(
organizationId: string,
datasetIds: string[] | 'all',
userId: string
) {
try {
const defaultPermissionGroupId = await ensureDefaultPermissionGroup(organizationId, userId);
// If 'all', fetch all organization datasets
let finalDatasetIds = datasetIds;
if (datasetIds === 'all') {
try {
const allDatasets = await db
.select({ id: datasets.id })
.from(datasets)
.where(and(eq(datasets.organizationId, organizationId), isNull(datasets.deletedAt)));
finalDatasetIds = allDatasets.map((d) => d.id);
console.info('Fetched all organization datasets for default assignment:', {
organizationId,
datasetCount: finalDatasetIds.length,
});
} catch (error) {
console.error('Error fetching all organization datasets:', {
organizationId,
error: error instanceof Error ? error.message : error,
});
throw new HTTPException(500, {
message: 'Failed to fetch organization datasets',
});
}
} else {
// Validate that the provided dataset IDs exist and aren't deleted
try {
const validDatasets = await db
.select({ id: datasets.id })
.from(datasets)
.where(and(eq(datasets.organizationId, organizationId), isNull(datasets.deletedAt)));
const validDatasetIds = new Set(validDatasets.map((d) => d.id));
const invalidIds = datasetIds.filter((id) => !validDatasetIds.has(id));
finalDatasetIds = datasetIds.filter((id) => validDatasetIds.has(id));
if (invalidIds.length > 0) {
console.warn('Some dataset IDs were invalid or deleted:', {
organizationId,
invalidIds,
requestedIds: datasetIds,
validIds: finalDatasetIds,
});
}
} catch (error) {
console.error('Error validating dataset IDs:', {
organizationId,
datasetIds,
error: error instanceof Error ? error.message : error,
});
throw new HTTPException(500, {
message: 'Failed to validate dataset IDs',
});
}
}
// Start a transaction to update datasets atomically
await db.transaction(async (tx) => {
try {
// First, soft delete all existing associations for this permission group
await tx
.update(datasetsToPermissionGroups)
.set({
deletedAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
})
.where(
and(
eq(datasetsToPermissionGroups.permissionGroupId, defaultPermissionGroupId),
isNull(datasetsToPermissionGroups.deletedAt)
)
);
// Add or restore dataset associations using upserts
if (Array.isArray(finalDatasetIds) && finalDatasetIds.length > 0) {
const currentTime = new Date().toISOString();
// Prepare values for batch upsert
const values = finalDatasetIds.map((datasetId) => ({
datasetId,
permissionGroupId: defaultPermissionGroupId,
createdAt: currentTime,
updatedAt: currentTime,
deletedAt: null,
}));
// Perform upsert - if the combination of datasetId and permissionGroupId exists,
// update deletedAt to null and updatedAt to current time
await tx
.insert(datasetsToPermissionGroups)
.values(values)
.onConflictDoUpdate({
target: [
datasetsToPermissionGroups.datasetId,
datasetsToPermissionGroups.permissionGroupId,
],
set: {
deletedAt: null,
updatedAt: currentTime,
},
});
}
console.info('Successfully updated default datasets:', {
organizationId,
permissionGroupId: defaultPermissionGroupId,
datasetCount: Array.isArray(finalDatasetIds) ? finalDatasetIds.length : 0,
operation: datasetIds === 'all' ? 'all' : 'specific',
});
} catch (error) {
console.error('Error in dataset update transaction:', {
organizationId,
permissionGroupId: defaultPermissionGroupId,
error: error instanceof Error ? error.message : error,
});
throw error; // Re-throw to trigger transaction rollback
}
});
} catch (error) {
// If it's already an HTTPException, re-throw it
if (error instanceof HTTPException) {
throw error;
}
console.error('Error updating default datasets:', {
organizationId,
userId,
datasetIds,
error: error instanceof Error ? error.message : error,
});
throw new HTTPException(500, {
message: 'Failed to update default datasets',
});
}
}

View File

@ -1,7 +1,7 @@
import { db, organizations, usersToOrganizations, users } from '@buster/database';
import { eq, and, isNull } from 'drizzle-orm';
import type { User, Organization } from '@buster/database';
import { randomUUID } from 'crypto';
import { db, organizations, users, usersToOrganizations } from '@buster/database';
import type { Organization, User } from '@buster/database';
import { and, eq, isNull } from 'drizzle-orm';
export async function createTestUserInDb(userData: Partial<User> = {}): Promise<User> {
const id = randomUUID();
@ -44,7 +44,7 @@ export async function createTestOrganizationInDb(
export async function createTestOrgMemberInDb(
userId: string,
organizationId: string,
role: string = 'querier'
role = 'querier'
): Promise<void> {
const member = {
userId,
@ -63,7 +63,7 @@ export async function createTestOrgMemberInDb(
export async function cleanupTestUser(userId: string): Promise<void> {
// Delete organization memberships
await db.delete(usersToOrganizations).where(eq(usersToOrganizations.userId, userId));
// Delete user
await db.delete(users).where(eq(users.id, userId));
}
@ -71,7 +71,7 @@ export async function cleanupTestUser(userId: string): Promise<void> {
export async function cleanupTestOrganization(orgId: string): Promise<void> {
// Delete organization memberships
await db.delete(usersToOrganizations).where(eq(usersToOrganizations.organizationId, orgId));
// Delete organization
await db.delete(organizations).where(eq(organizations.id, orgId));
}
@ -80,13 +80,8 @@ export async function getOrganizationFromDb(orgId: string): Promise<Organization
const result = await db
.select()
.from(organizations)
.where(
and(
eq(organizations.id, orgId),
isNull(organizations.deletedAt)
)
)
.where(and(eq(organizations.id, orgId), isNull(organizations.deletedAt)))
.limit(1);
return result[0] as Organization || null;
}
return (result[0] as Organization) || null;
}

View File

@ -1,4 +1,4 @@
import type { User, Organization, OrganizationMember } from '@buster/database';
import type { Organization, OrganizationMember, User } from '@buster/database';
export function createTestUser(overrides?: Partial<User>): User {
const id = `test-user-${Math.random().toString(36).substring(7)}`;
@ -33,7 +33,7 @@ export function createTestOrganization(overrides?: Partial<Organization>): Organ
export function createTestOrgMember(
userId: string,
organizationId: string,
role: string = 'querier'
role = 'querier'
): { organizationId: string; role: string } {
return {
organizationId,
@ -71,4 +71,4 @@ export function createMockDb() {
mockSet,
mockReturning,
};
}
}

View File

@ -1,15 +1,15 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { updateWorkspaceSettingsHandler } from './update-workspace-settings';
import type { Organization, User } from '@buster/database';
import { HTTPException } from 'hono/http-exception';
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
import {
createTestUserInDb,
createTestOrganizationInDb,
createTestOrgMemberInDb,
cleanupTestUser,
cleanupTestOrganization,
cleanupTestUser,
createTestOrgMemberInDb,
createTestOrganizationInDb,
createTestUserInDb,
getOrganizationFromDb,
} from './test-db-utils';
import { HTTPException } from 'hono/http-exception';
import type { User, Organization } from '@buster/database';
import { updateWorkspaceSettingsHandler } from './update-workspace-settings';
describe('updateWorkspaceSettingsHandler (integration)', () => {
let testUser: User;
@ -121,7 +121,9 @@ describe('updateWorkspaceSettingsHandler (integration)', () => {
const request = { restrict_new_user_invitations: true };
await expect(updateWorkspaceSettingsHandler(request, roleUser)).rejects.toThrow(HTTPException);
await expect(updateWorkspaceSettingsHandler(request, roleUser)).rejects.toThrow(
HTTPException
);
await expect(updateWorkspaceSettingsHandler(request, roleUser)).rejects.toMatchObject({
status: 403,
message: 'Only workspace admins can update workspace settings',
@ -144,7 +146,9 @@ describe('updateWorkspaceSettingsHandler (integration)', () => {
const request = { restrict_new_user_invitations: true };
// When organization is deleted, user effectively has no organization
await expect(updateWorkspaceSettingsHandler(request, testUser)).rejects.toThrow(HTTPException);
await expect(updateWorkspaceSettingsHandler(request, testUser)).rejects.toThrow(
HTTPException
);
await expect(updateWorkspaceSettingsHandler(request, testUser)).rejects.toMatchObject({
status: 403,
message: 'User is not associated with an organization',
@ -154,7 +158,9 @@ describe('updateWorkspaceSettingsHandler (integration)', () => {
it('should return 403 for user without organization', async () => {
const request = { restrict_new_user_invitations: true };
await expect(updateWorkspaceSettingsHandler(request, testUser)).rejects.toThrow(HTTPException);
await expect(updateWorkspaceSettingsHandler(request, testUser)).rejects.toThrow(
HTTPException
);
await expect(updateWorkspaceSettingsHandler(request, testUser)).rejects.toMatchObject({
status: 403,
message: 'User is not associated with an organization',
@ -197,7 +203,7 @@ describe('updateWorkspaceSettingsHandler (integration)', () => {
await createTestOrgMemberInDb(testUser.id, testOrg.id, 'workspace_admin');
const originalUpdatedAt = testOrg.updatedAt;
await new Promise(resolve => setTimeout(resolve, 10)); // Ensure time difference
await new Promise((resolve) => setTimeout(resolve, 10)); // Ensure time difference
const request = { restrict_new_user_invitations: true };
await updateWorkspaceSettingsHandler(request, testUser);
@ -229,4 +235,4 @@ describe('updateWorkspaceSettingsHandler (integration)', () => {
);
});
});
});
});

View File

@ -1,9 +1,9 @@
import { describe, it, expect, vi, beforeEach } from 'vitest';
import { updateWorkspaceSettingsHandler } from './update-workspace-settings';
import { createTestUser, createTestOrganization } from './test-fixtures';
import * as securityUtils from './security-utils';
import { WorkspaceSettingsService } from './workspace-settings-service';
import { db } from '@buster/database';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as securityUtils from './security-utils';
import { createTestOrganization, createTestUser } from './test-fixtures';
import { updateWorkspaceSettingsHandler } from './update-workspace-settings';
import { WorkspaceSettingsService } from './workspace-settings-service';
// Mock dependencies
vi.mock('./security-utils');
@ -33,17 +33,21 @@ describe('updateWorkspaceSettingsHandler', () => {
defaultRole: 'restricted_querier',
});
const mockOrgMembership = { organizationId: 'org-123', role: 'workspace_admin' };
const mockDefaultDatasets = [
{ id: 'dataset-1', name: 'Sales Data' },
{ id: 'dataset-2', name: 'Customer Data' },
];
beforeEach(() => {
vi.clearAllMocks();
// Setup default mocks
vi.mocked(securityUtils.validateUserOrganization).mockResolvedValue(mockOrgMembership);
vi.mocked(securityUtils.checkWorkspaceAdminPermission).mockImplementation(() => {});
vi.mocked(securityUtils.fetchOrganization)
.mockResolvedValueOnce(mockOrg) // First call for initial fetch
.mockResolvedValueOnce({ ...mockOrg, restrictNewUserInvitations: true }); // Second call after update
vi.mocked(securityUtils.fetchOrganization).mockResolvedValue(mockOrg);
vi.mocked(securityUtils.updateDefaultDatasets).mockResolvedValue(undefined);
vi.mocked(securityUtils.fetchDefaultDatasets).mockResolvedValue(mockDefaultDatasets);
// Setup workspace settings service mocks
vi.mocked(WorkspaceSettingsService.prototype.buildUpdateData).mockReturnValue({
updatedAt: '2024-01-01T00:00:00Z',
@ -52,9 +56,9 @@ describe('updateWorkspaceSettingsHandler', () => {
vi.mocked(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).mockReturnValue({
restrict_new_user_invitations: true,
default_role: 'restricted_querier',
default_datasets: [],
default_datasets: mockDefaultDatasets,
});
// Mock database update
const mockDbChain = {
update: vi.fn().mockReturnThis(),
@ -69,7 +73,7 @@ describe('updateWorkspaceSettingsHandler', () => {
restrict_new_user_invitations: true,
default_role: 'data_admin',
};
vi.mocked(WorkspaceSettingsService.prototype.buildUpdateData).mockReturnValue({
updatedAt: '2024-01-01T00:00:00Z',
restrictNewUserInvitations: true,
@ -78,33 +82,33 @@ describe('updateWorkspaceSettingsHandler', () => {
vi.mocked(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).mockReturnValue({
restrict_new_user_invitations: true,
default_role: 'data_admin',
default_datasets: [],
default_datasets: mockDefaultDatasets,
});
const result = await updateWorkspaceSettingsHandler(request, mockUser);
expect(securityUtils.validateUserOrganization).toHaveBeenCalledWith(mockUser.id);
expect(securityUtils.checkWorkspaceAdminPermission).toHaveBeenCalledWith('workspace_admin');
expect(WorkspaceSettingsService.prototype.buildUpdateData).toHaveBeenCalledWith(request);
expect(result).toEqual({
restrict_new_user_invitations: true,
default_role: 'data_admin',
default_datasets: [],
default_datasets: mockDefaultDatasets,
});
});
it('should handle partial updates correctly', async () => {
const request = { restrict_new_user_invitations: true };
const result = await updateWorkspaceSettingsHandler(request, mockUser);
expect(WorkspaceSettingsService.prototype.buildUpdateData).toHaveBeenCalledWith(request);
expect(result).toEqual({
restrict_new_user_invitations: true,
default_role: 'restricted_querier',
default_datasets: [],
default_datasets: mockDefaultDatasets,
});
});
@ -116,7 +120,7 @@ describe('updateWorkspaceSettingsHandler', () => {
where: vi.fn().mockResolvedValue(undefined),
};
vi.mocked(db.update).mockReturnValue(mockDbChain as any);
vi.mocked(WorkspaceSettingsService.prototype.buildUpdateData).mockReturnValue({
updatedAt: '2024-01-01T00:00:00Z',
defaultRole: 'data_admin',
@ -124,11 +128,11 @@ describe('updateWorkspaceSettingsHandler', () => {
vi.mocked(WorkspaceSettingsService.prototype.formatWorkspaceSettingsResponse).mockReturnValue({
restrict_new_user_invitations: false,
default_role: 'data_admin',
default_datasets: [],
default_datasets: mockDefaultDatasets,
});
await updateWorkspaceSettingsHandler(request, mockUser);
expect(db.update).toHaveBeenCalled();
expect(mockDbChain.set).toHaveBeenCalledWith({
updatedAt: '2024-01-01T00:00:00Z',
@ -138,9 +142,9 @@ describe('updateWorkspaceSettingsHandler', () => {
it('should fetch updated organization after update', async () => {
const request = { restrict_new_user_invitations: true };
await updateWorkspaceSettingsHandler(request, mockUser);
expect(securityUtils.fetchOrganization).toHaveBeenCalledTimes(1);
expect(securityUtils.fetchOrganization).toHaveBeenCalledWith('org-123');
});
@ -149,11 +153,11 @@ describe('updateWorkspaceSettingsHandler', () => {
vi.mocked(securityUtils.validateUserOrganization).mockRejectedValue(
new Error('User not in organization')
);
const request = { restrict_new_user_invitations: true };
await expect(updateWorkspaceSettingsHandler(request, mockUser)).rejects.toThrow(
'User not in organization'
'Failed to update workspace settings'
);
});
@ -161,11 +165,11 @@ describe('updateWorkspaceSettingsHandler', () => {
vi.mocked(securityUtils.checkWorkspaceAdminPermission).mockImplementation(() => {
throw new Error('Only workspace admins can update settings');
});
const request = { restrict_new_user_invitations: true };
await expect(updateWorkspaceSettingsHandler(request, mockUser)).rejects.toThrow(
'Only workspace admins can update settings'
'Failed to update workspace settings'
);
});
@ -177,11 +181,58 @@ describe('updateWorkspaceSettingsHandler', () => {
throw new Error('Only workspace admins can update settings');
}
});
const request = { restrict_new_user_invitations: true };
await expect(updateWorkspaceSettingsHandler(request, mockUser)).rejects.toThrow(
'Only workspace admins can update settings'
'Failed to update workspace settings'
);
});
});
it('should update default datasets with specific IDs', async () => {
const request = {
default_datasets_ids: ['dataset-1', 'dataset-2'],
};
await updateWorkspaceSettingsHandler(request, mockUser);
expect(securityUtils.updateDefaultDatasets).toHaveBeenCalledWith(
'org-123',
['dataset-1', 'dataset-2'],
mockUser.id
);
expect(securityUtils.fetchDefaultDatasets).toHaveBeenCalledWith('org-123');
});
it('should update default datasets with "all" keyword', async () => {
const request = {
default_datasets_ids: ['all'],
};
await updateWorkspaceSettingsHandler(request, mockUser);
expect(securityUtils.updateDefaultDatasets).toHaveBeenCalledWith('org-123', 'all', mockUser.id);
});
it('should not update default datasets when not provided', async () => {
const request = {
restrict_new_user_invitations: true,
};
await updateWorkspaceSettingsHandler(request, mockUser);
expect(securityUtils.updateDefaultDatasets).not.toHaveBeenCalled();
// But should still fetch them for the response
expect(securityUtils.fetchDefaultDatasets).toHaveBeenCalledWith('org-123');
});
it('should handle empty default datasets array', async () => {
const request = {
default_datasets_ids: [],
};
await updateWorkspaceSettingsHandler(request, mockUser);
expect(securityUtils.updateDefaultDatasets).toHaveBeenCalledWith('org-123', [], mockUser.id);
});
});

View File

@ -1,12 +1,15 @@
import { type User, and, db, eq, isNull, organizations } from '@buster/database';
import type {
UpdateWorkspaceSettingsRequest,
UpdateWorkspaceSettingsResponse,
} from '@buster/server-shared/security';
import { type User, db, organizations, eq, and, isNull } from '@buster/database';
import {
validateUserOrganization,
fetchOrganization,
checkWorkspaceAdminPermission
import { HTTPException } from 'hono/http-exception';
import {
checkWorkspaceAdminPermission,
fetchDefaultDatasets,
fetchOrganization,
updateDefaultDatasets,
validateUserOrganization,
} from './security-utils';
import { WorkspaceSettingsService } from './workspace-settings-service';
@ -16,24 +19,72 @@ export async function updateWorkspaceSettingsHandler(
request: UpdateWorkspaceSettingsRequest,
user: User
): Promise<UpdateWorkspaceSettingsResponse> {
// Validate user organization and permissions
const userOrg = await validateUserOrganization(user.id);
checkWorkspaceAdminPermission(userOrg.role);
try {
// Validate user organization and permissions
const userOrg = await validateUserOrganization(user.id);
checkWorkspaceAdminPermission(userOrg.role);
// Build update data
const updateData = settingsService.buildUpdateData(request);
// Build update data
const updateData = settingsService.buildUpdateData(request);
// Update organization settings
await updateOrganizationSettings(userOrg.organizationId, updateData);
// Update organization settings and default datasets concurrently
const updatePromises: Promise<void>[] = [
updateOrganizationSettings(userOrg.organizationId, updateData),
];
// Fetch updated organization
const updatedOrg = await fetchOrganization(userOrg.organizationId);
// Update default datasets if provided
if (request.default_datasets_ids !== undefined) {
updatePromises.push(
updateDefaultDatasets(
userOrg.organizationId,
request.default_datasets_ids.includes('all')
? 'all'
: request.default_datasets_ids.filter((id): id is string => id !== 'all'),
user.id
)
);
}
// Return formatted response
return settingsService.formatWorkspaceSettingsResponse({
restrictNewUserInvitations: updatedOrg.restrictNewUserInvitations,
defaultRole: updatedOrg.defaultRole,
});
try {
await Promise.all(updatePromises);
} catch (error) {
console.error('Error during concurrent updates:', {
userId: user.id,
organizationId: userOrg.organizationId,
hasDatasetUpdate: request.default_datasets_ids !== undefined,
error: error instanceof Error ? error.message : error,
});
throw error; // Re-throw to be caught by outer try-catch
}
// Fetch updated organization and default datasets
const [updatedOrg, defaultDatasets] = await Promise.all([
fetchOrganization(userOrg.organizationId),
fetchDefaultDatasets(userOrg.organizationId),
]);
// Return formatted response
return settingsService.formatWorkspaceSettingsResponse({
restrictNewUserInvitations: updatedOrg.restrictNewUserInvitations,
defaultRole: updatedOrg.defaultRole,
defaultDatasets,
});
} catch (error) {
console.error('Error in updateWorkspaceSettingsHandler:', {
userId: user.id,
request,
error: error instanceof Error ? error.message : error,
});
// Re-throw HTTPException as is, wrap other errors
if (error instanceof HTTPException) {
throw error;
}
throw new HTTPException(500, {
message: 'Failed to update workspace settings',
});
}
}
async function updateOrganizationSettings(
@ -44,13 +95,24 @@ async function updateOrganizationSettings(
defaultRole?: string;
}
): Promise<void> {
await db
.update(organizations)
.set(updateData)
.where(
and(
eq(organizations.id, organizationId),
isNull(organizations.deletedAt)
)
);
}
try {
await db
.update(organizations)
.set(updateData)
.where(and(eq(organizations.id, organizationId), isNull(organizations.deletedAt)));
console.info('Updated organization settings:', {
organizationId,
fields: Object.keys(updateData).filter((k) => k !== 'updatedAt'),
});
} catch (error) {
console.error('Error updating organization settings:', {
organizationId,
updateData,
error: error instanceof Error ? error.message : error,
});
throw new HTTPException(500, {
message: 'Failed to update organization settings',
});
}
}

View File

@ -1,6 +1,6 @@
import { describe, it, expect } from 'vitest';
import { WorkspaceSettingsService } from './workspace-settings-service';
import type { UpdateWorkspaceSettingsRequest } from '@buster/server-shared/security';
import { describe, expect, it } from 'vitest';
import { WorkspaceSettingsService } from './workspace-settings-service';
describe('WorkspaceSettingsService', () => {
const service = new WorkspaceSettingsService();
@ -37,15 +37,19 @@ describe('WorkspaceSettingsService', () => {
restrictNewUserInvitations: false,
defaultRole: 'member',
};
expect(service.formatWorkspaceSettingsResponse(settingsTrue).restrict_new_user_invitations).toBe(true);
expect(service.formatWorkspaceSettingsResponse(settingsFalse).restrict_new_user_invitations).toBe(false);
expect(
service.formatWorkspaceSettingsResponse(settingsTrue).restrict_new_user_invitations
).toBe(true);
expect(
service.formatWorkspaceSettingsResponse(settingsFalse).restrict_new_user_invitations
).toBe(false);
});
it('should handle all string values correctly', () => {
const roles = ['member', 'admin', 'workspace_admin', 'data_admin'];
roles.forEach(role => {
roles.forEach((role) => {
const settings = {
restrictNewUserInvitations: false,
defaultRole: role,
@ -60,7 +64,7 @@ describe('WorkspaceSettingsService', () => {
it('should include updatedAt timestamp', () => {
const request: UpdateWorkspaceSettingsRequest = {};
const result = service.buildUpdateData(request);
expect(result.updatedAt).toBeDefined();
expect(new Date(result.updatedAt).toISOString()).toBe(result.updatedAt);
});
@ -70,7 +74,7 @@ describe('WorkspaceSettingsService', () => {
restrict_new_user_invitations: true,
};
const result = service.buildUpdateData(request);
expect(result).toHaveProperty('restrictNewUserInvitations', true);
expect(result).not.toHaveProperty('defaultRole');
});
@ -80,7 +84,7 @@ describe('WorkspaceSettingsService', () => {
default_role: 'admin',
};
const result = service.buildUpdateData(request);
expect(result).toHaveProperty('defaultRole', 'admin');
expect(result).not.toHaveProperty('restrictNewUserInvitations');
});
@ -91,7 +95,7 @@ describe('WorkspaceSettingsService', () => {
default_role: 'member',
};
const result = service.buildUpdateData(request);
expect(result).toHaveProperty('restrictNewUserInvitations', false);
expect(result).toHaveProperty('defaultRole', 'member');
expect(result).toHaveProperty('updatedAt');
@ -103,7 +107,7 @@ describe('WorkspaceSettingsService', () => {
default_role: undefined,
};
const result = service.buildUpdateData(request);
expect(result).not.toHaveProperty('restrictNewUserInvitations');
expect(result).not.toHaveProperty('defaultRole');
expect(result).toHaveProperty('updatedAt');
@ -114,8 +118,8 @@ describe('WorkspaceSettingsService', () => {
restrict_new_user_invitations: false,
};
const result = service.buildUpdateData(request);
expect(result.restrictNewUserInvitations).toBe(false);
});
});
});
});

View File

@ -1,17 +1,24 @@
import type {
import type {
GetWorkspaceSettingsResponse,
UpdateWorkspaceSettingsRequest
UpdateWorkspaceSettingsRequest,
} from '@buster/server-shared/security';
export class WorkspaceSettingsService {
formatWorkspaceSettingsResponse(settings: {
restrictNewUserInvitations: boolean;
defaultRole: string;
defaultRole:
| 'workspace_admin'
| 'data_admin'
| 'querier'
| 'restricted_querier'
| 'viewer'
| 'none';
defaultDatasets?: Array<{ id: string; name: string }>;
}): GetWorkspaceSettingsResponse {
return {
restrict_new_user_invitations: settings.restrictNewUserInvitations,
default_role: settings.defaultRole,
default_datasets: [], // TODO: Implement when datasets are available
default_datasets: settings.defaultDatasets || [],
};
}
@ -38,4 +45,4 @@ export class WorkspaceSettingsService {
return updateData;
}
}
}

View File

@ -9,6 +9,7 @@
"ci:check": "pnpm run check && pnpm run typecheck",
"db:check": "pnpm --filter @buster/database run db:check",
"db:generate": "pnpm --filter @buster/database run db:generate",
"db:generate:custom": "pnpm --filter @buster/database run db:generate:custom",
"db:init": "pnpm run --filter @buster/database db:init",
"db:introspect": "pnpm --filter @buster/database run db:introspect",
"db:migrate": "pnpm --filter @buster/database run db:migrate",

View File

@ -1,37 +1,39 @@
import { describe, expect, test, vi } from 'vitest';
import type { CoreMessage } from 'ai';
import { describe, expect, test, vi } from 'vitest';
import { formatFollowUpMessageStepExecution } from '../../../src/steps/post-processing/format-follow-up-message-step';
// Mock the agent and its dependencies
vi.mock('@mastra/core', () => ({
Agent: vi.fn().mockImplementation(() => ({
generate: vi.fn().mockResolvedValue({
toolCalls: [{
toolName: 'generateUpdateMessage',
args: {
update_message: 'Test update message',
title: 'Update Title'
}
}]
})
toolCalls: [
{
toolName: 'generateUpdateMessage',
args: {
update_message: 'Test update message',
title: 'Update Title',
},
},
],
}),
})),
createStep: vi.fn((config) => config)
createStep: vi.fn((config) => config),
}));
vi.mock('braintrust', () => ({
wrapTraced: vi.fn((fn) => fn)
wrapTraced: vi.fn((fn) => fn),
}));
vi.mock('../../../src/utils/models/anthropic-cached', () => ({
anthropicCachedModel: vi.fn(() => 'mocked-model')
anthropicCachedModel: vi.fn(() => 'mocked-model'),
}));
vi.mock('../../../src/utils/standardizeMessages', () => ({
standardizeMessages: vi.fn((msg) => [{ role: 'user', content: msg }])
standardizeMessages: vi.fn((msg) => [{ role: 'user', content: msg }]),
}));
vi.mock('../../../src/tools/post-processing/generate-update-message', () => ({
generateUpdateMessage: {}
generateUpdateMessage: {},
}));
describe('Format Follow-up Message Step Unit Tests', () => {
@ -40,7 +42,7 @@ describe('Format Follow-up Message Step Unit Tests', () => {
{ role: 'user', content: 'Initial query about sales' },
{ role: 'assistant', content: 'Sales data analysis complete' },
{ role: 'user', content: 'Can you filter by last 6 months?' },
{ role: 'assistant', content: 'Filtered data shown' }
{ role: 'assistant', content: 'Filtered data shown' },
];
const inputData = {
@ -61,9 +63,9 @@ describe('Format Follow-up Message Step Unit Tests', () => {
descriptiveTitle: 'Time Period Filter',
classification: 'timePeriodInterpretation' as const,
explanation: 'Assumed last 6 months means from today',
label: 'major' as const
}
]
label: 'major' as const,
},
],
};
const result = await formatFollowUpMessageStepExecution({ inputData });
@ -93,9 +95,9 @@ describe('Format Follow-up Message Step Unit Tests', () => {
descriptiveTitle: 'Data Aggregation',
classification: 'aggregation' as const,
explanation: 'Used SUM for totals',
label: 'major' as const
}
]
label: 'major' as const,
},
],
};
const result = await formatFollowUpMessageStepExecution({ inputData });
@ -124,9 +126,9 @@ describe('Format Follow-up Message Step Unit Tests', () => {
descriptiveTitle: 'Metric Definition',
classification: 'metricDefinition' as const,
explanation: 'Used standard revenue metric',
label: 'major' as const
}
]
label: 'major' as const,
},
],
};
const result = await formatFollowUpMessageStepExecution({ inputData });
@ -141,7 +143,7 @@ describe('Format Follow-up Message Step Unit Tests', () => {
const inputData = {
conversationHistory: [
{ role: 'user' as const, content: 'Follow-up question' },
{ role: 'assistant' as const, content: 'Follow-up answer' }
{ role: 'assistant' as const, content: 'Follow-up answer' },
],
userName: 'Marcus Wright',
messageId: 'msg-fu-999',
@ -157,15 +159,15 @@ describe('Format Follow-up Message Step Unit Tests', () => {
descriptiveTitle: 'Formatting Choice',
classification: 'dataFormat' as const,
explanation: 'Used comma separation',
label: 'minor' as const
label: 'minor' as const,
},
{
descriptiveTitle: 'Time Zone',
classification: 'timePeriodInterpretation' as const,
explanation: 'Used UTC',
label: 'timeRelated' as const
}
]
label: 'timeRelated' as const,
},
],
};
const result = await formatFollowUpMessageStepExecution({ inputData });
@ -180,7 +182,7 @@ describe('Format Follow-up Message Step Unit Tests', () => {
const mockConversationHistory: CoreMessage[] = [
{ role: 'user', content: 'Show me customer segments' },
{ role: 'assistant', content: 'Here are the segments' },
{ role: 'user', content: 'Filter by enterprise only' }
{ role: 'user', content: 'Filter by enterprise only' },
];
const inputData = {
@ -201,15 +203,15 @@ describe('Format Follow-up Message Step Unit Tests', () => {
descriptiveTitle: 'Enterprise Definition',
classification: 'segmentDefinition' as const,
explanation: 'Defined enterprise as >$1M revenue',
label: 'major' as const
label: 'major' as const,
},
{
descriptiveTitle: 'Customer Status',
classification: 'dataQuality' as const,
explanation: 'Included only active customers',
label: 'major' as const
}
]
label: 'major' as const,
},
],
};
const result = await formatFollowUpMessageStepExecution({ inputData });
@ -219,4 +221,4 @@ describe('Format Follow-up Message Step Unit Tests', () => {
expect(result.message).toBe('Test update message');
expect(result.conversationHistory).toEqual(mockConversationHistory);
});
});
});

View File

@ -1,44 +1,46 @@
import { describe, expect, test, vi } from 'vitest';
import type { CoreMessage } from 'ai';
import { describe, expect, test, vi } from 'vitest';
import { formatInitialMessageStepExecution } from '../../../src/steps/post-processing/format-initial-message-step';
// Mock the agent and its dependencies
vi.mock('@mastra/core', () => ({
Agent: vi.fn().mockImplementation(() => ({
generate: vi.fn().mockResolvedValue({
toolCalls: [{
toolName: 'generateSummary',
args: {
summary_message: 'Test summary message',
title: 'Test Title'
}
}]
})
toolCalls: [
{
toolName: 'generateSummary',
args: {
summary_message: 'Test summary message',
title: 'Test Title',
},
},
],
}),
})),
createStep: vi.fn((config) => config)
createStep: vi.fn((config) => config),
}));
vi.mock('braintrust', () => ({
wrapTraced: vi.fn((fn) => fn)
wrapTraced: vi.fn((fn) => fn),
}));
vi.mock('../../../src/utils/models/anthropic-cached', () => ({
anthropicCachedModel: vi.fn(() => 'mocked-model')
anthropicCachedModel: vi.fn(() => 'mocked-model'),
}));
vi.mock('../../../src/utils/standardizeMessages', () => ({
standardizeMessages: vi.fn((msg) => [{ role: 'user', content: msg }])
standardizeMessages: vi.fn((msg) => [{ role: 'user', content: msg }]),
}));
vi.mock('../../../src/tools/post-processing/generate-summary', () => ({
generateSummary: {}
generateSummary: {},
}));
describe('Format Initial Message Step Unit Tests', () => {
test('should include chat history in context message when present', async () => {
const mockConversationHistory: CoreMessage[] = [
{ role: 'user', content: 'What is the total revenue?' },
{ role: 'assistant', content: 'The total revenue is $1M' }
{ role: 'assistant', content: 'The total revenue is $1M' },
];
const inputData = {
@ -59,9 +61,9 @@ describe('Format Initial Message Step Unit Tests', () => {
descriptiveTitle: 'Revenue Calculation',
classification: 'metricDefinition' as const,
explanation: 'Assumed revenue includes all sources',
label: 'major' as const
}
]
label: 'major' as const,
},
],
};
const result = await formatInitialMessageStepExecution({ inputData });
@ -90,9 +92,9 @@ describe('Format Initial Message Step Unit Tests', () => {
descriptiveTitle: 'Customer Count',
classification: 'dataQuality' as const,
explanation: 'Included all customer statuses',
label: 'major' as const
}
]
label: 'major' as const,
},
],
};
const result = await formatInitialMessageStepExecution({ inputData });
@ -120,9 +122,9 @@ describe('Format Initial Message Step Unit Tests', () => {
descriptiveTitle: 'Date Range',
classification: 'timePeriodInterpretation' as const,
explanation: 'Assumed current month',
label: 'major' as const
}
]
label: 'major' as const,
},
],
};
const result = await formatInitialMessageStepExecution({ inputData });
@ -136,7 +138,7 @@ describe('Format Initial Message Step Unit Tests', () => {
const inputData = {
conversationHistory: [
{ role: 'user' as const, content: 'Hello' },
{ role: 'assistant' as const, content: 'Hi there!' }
{ role: 'assistant' as const, content: 'Hi there!' },
],
userName: 'Alice Cooper',
messageId: 'msg-999',
@ -152,9 +154,9 @@ describe('Format Initial Message Step Unit Tests', () => {
descriptiveTitle: 'Minor Detail',
classification: 'dataFormat' as const,
explanation: 'Used standard format',
label: 'minor' as const
}
]
label: 'minor' as const,
},
],
};
const result = await formatInitialMessageStepExecution({ inputData });
@ -163,4 +165,4 @@ describe('Format Initial Message Step Unit Tests', () => {
expect(result.summaryMessage).toBeUndefined();
expect(result.summaryTitle).toBeUndefined();
});
});
});

View File

@ -0,0 +1,62 @@
-- Custom SQL migration file, put your code below! --
CREATE OR REPLACE FUNCTION public.auto_add_user_to_organizations()
RETURNS TRIGGER
LANGUAGE plpgsql
AS $$
DECLARE
user_domain text;
org_record public.organizations%ROWTYPE;
BEGIN
IF EXISTS (SELECT 1 FROM public.users_to_organizations WHERE public.users_to_organizations.user_id = NEW.id) THEN
RETURN NEW;
END IF;
user_domain := split_part(NEW.email, '@', 2);
FOR org_record IN
SELECT * FROM public.organizations
WHERE user_domain = ANY(public.organizations.domains)
LOOP
INSERT INTO public.users_to_organizations (
user_id,
organization_id,
role,
sharing_setting,
edit_sql,
upload_csv,
export_assets,
email_slack_enabled,
created_at,
updated_at,
deleted_at,
created_by,
updated_by,
deleted_by,
status
) VALUES (
NEW.id,
org_record.id,
org_record.default_role,
'none'::public.sharing_setting_enum,
false,
false,
false,
false,
now(),
now(),
null,
NEW.id,
NEW.id,
null,
'active'::public.user_organization_status_enum
);
END LOOP;
RETURN NEW;
END;
$$;
CREATE TRIGGER auto_add_to_orgs_trigger
AFTER INSERT ON public.users
FOR EACH ROW
EXECUTE FUNCTION public.auto_add_user_to_organizations();

File diff suppressed because it is too large Load Diff

View File

@ -540,6 +540,13 @@
"when": 1752091186136,
"tag": "0076_tired_madripoor",
"breakpoints": true
},
{
"idx": 77,
"version": "7",
"when": 1752150366202,
"tag": "0077_short_marvex",
"breakpoints": true
}
]
}

View File

@ -43,18 +43,7 @@ export const UpdateWorkspaceSettingsRequestSchema = z.object({
restrict_new_user_invitations: z.boolean().optional(),
default_role: OrganizationRoleSchema.optional(),
// this can either be a uuid or "all"
default_datasets_ids: z
.array(
z.union([
z
.string()
.regex(
/^[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[1-5][0-9a-fA-F]{3}-[89abAB][0-9a-fA-F]{3}-[0-9a-fA-F]{12}$/
),
z.literal('all'),
])
)
.optional(),
default_datasets_ids: z.array(z.union([z.uuid(), z.literal('all')])).optional(),
});
export type UpdateWorkspaceSettingsRequest = z.infer<typeof UpdateWorkspaceSettingsRequestSchema>;