Throw proper error on duplicate emailing domain (#22790)
Adding an emailing domain that already exists blew up with a raw QueryFailedError and the client just saw a generic "An error occurred". The unique index on domain is global, so the workspace-scoped existence check never caught rows owned by another workspace. Now the check is unscoped and throws an EmailingDomainException mapped to CONFLICT with a proper user-facing message, in both the createEmailingDomain mutation and the email group channel flow. Also dropped the hardcoded catch-all snackbar on the new channel page so server messages actually reach the user. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22790?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+7
-15
@@ -10,7 +10,6 @@ import { Section } from 'twenty-ui/layout';
|
||||
import { useCreateEmailGroupChannel } from '@/settings/accounts/hooks/useCreateEmailGroupChannel';
|
||||
import { SaveAndCancelButtons } from '@/settings/components/SaveAndCancelButtons/SaveAndCancelButtons';
|
||||
import { SettingsPageContainer } from '@/settings/components/SettingsPageContainer';
|
||||
import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar';
|
||||
import { SettingsTextInput } from '@/ui/input/components/SettingsTextInput';
|
||||
import { SettingsPageLayout } from '@/settings/components/layout/SettingsPageLayout';
|
||||
import { useNavigateSettings } from '~/hooks/useNavigateSettings';
|
||||
@@ -18,7 +17,6 @@ import { useNavigateSettings } from '~/hooks/useNavigateSettings';
|
||||
export const SettingsAccountsNewEmailGroupChannel = () => {
|
||||
const { t } = useLingui();
|
||||
const navigate = useNavigateSettings();
|
||||
const { enqueueErrorSnackBar } = useSnackBar();
|
||||
const { createEmailGroupChannel, loading } = useCreateEmailGroupChannel();
|
||||
|
||||
const [handle, setHandle] = useState('');
|
||||
@@ -27,22 +25,16 @@ export const SettingsAccountsNewEmailGroupChannel = () => {
|
||||
const canSave = isHandleValidEmail && !loading;
|
||||
|
||||
const handleSave = useCallback(async () => {
|
||||
try {
|
||||
const result = await createEmailGroupChannel(handle);
|
||||
const messageChannelId =
|
||||
result.data?.createEmailGroupChannel.messageChannel.id;
|
||||
const result = await createEmailGroupChannel(handle);
|
||||
const messageChannelId =
|
||||
result.data?.createEmailGroupChannel.messageChannel.id;
|
||||
|
||||
if (messageChannelId) {
|
||||
navigate(SettingsPath.EmailGroupChannelDetail, {
|
||||
messageChannelId,
|
||||
});
|
||||
}
|
||||
} catch {
|
||||
enqueueErrorSnackBar({
|
||||
message: t`Failed to create email channel. Email channels may not be configured on this server.`,
|
||||
if (messageChannelId) {
|
||||
navigate(SettingsPath.EmailGroupChannelDetail, {
|
||||
messageChannelId,
|
||||
});
|
||||
}
|
||||
}, [createEmailGroupChannel, handle, navigate, enqueueErrorSnackBar, t]);
|
||||
}, [createEmailGroupChannel, handle, navigate]);
|
||||
|
||||
return (
|
||||
<SettingsPageLayout
|
||||
|
||||
+9
-1
@@ -10,6 +10,7 @@ import { CREATE_EMAIL_GROUP_CHANNEL } from '@/settings/accounts/graphql/mutation
|
||||
import { GET_MY_CONNECTED_ACCOUNTS } from '@/settings/accounts/graphql/queries/getMyConnectedAccounts';
|
||||
import { GET_MY_MESSAGE_CHANNELS } from '@/settings/accounts/graphql/queries/getMyMessageChannels';
|
||||
import { GET_ALL_EMAILING_DOMAINS } from '@/settings/emailing-domains/graphql/queries/getAllEmailingDomains';
|
||||
import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar';
|
||||
|
||||
type CreateEmailGroupChannelResult = {
|
||||
createEmailGroupChannel: {
|
||||
@@ -33,6 +34,8 @@ type CreateEmailGroupChannelVariables = {
|
||||
};
|
||||
|
||||
export const useCreateEmailGroupChannel = () => {
|
||||
const { enqueueErrorSnackBar } = useSnackBar();
|
||||
|
||||
const [mutate, { loading, error }] = useMutation<
|
||||
CreateEmailGroupChannelResult,
|
||||
CreateEmailGroupChannelVariables
|
||||
@@ -45,7 +48,12 @@ export const useCreateEmailGroupChannel = () => {
|
||||
});
|
||||
|
||||
const createEmailGroupChannel = (handle: string) =>
|
||||
mutate({ variables: { input: { handle } } });
|
||||
mutate({
|
||||
variables: { input: { handle } },
|
||||
onError: (mutationError) => {
|
||||
enqueueErrorSnackBar({ apolloError: mutationError });
|
||||
},
|
||||
});
|
||||
|
||||
return { createEmailGroupChannel, loading, error };
|
||||
};
|
||||
|
||||
@@ -50,6 +50,9 @@ const WORKSPACE_SCOPED_EXEMPTIONS = new Set<string>([
|
||||
// Only injection lives in a frozen historical upgrade-version-command
|
||||
// directory that CI's mutation-guard refuses to let us edit.
|
||||
'DataSourceEntity',
|
||||
// The domain column is globally unique across workspaces, so duplicate
|
||||
// preflight checks must query cross-workspace; writes stay on the wrapper.
|
||||
'EmailingDomainEntity',
|
||||
]);
|
||||
|
||||
// Everything else must use @InjectWorkspaceScopedRepository.
|
||||
|
||||
+5
-1
@@ -8,6 +8,7 @@ import { MetadataResolver } from 'src/engine/api/graphql/graphql-config/decorato
|
||||
import { CreateEmailingDomainInput } from 'src/engine/core-modules/emailing-domain/dtos/create-emailing-domain.input';
|
||||
import { EmailingDomainDTO } from 'src/engine/core-modules/emailing-domain/dtos/emailing-domain.dto';
|
||||
import { EmailGroupAccessGraphqlApiExceptionFilter } from 'src/engine/core-modules/emailing-domain/filters/email-group-access-graphql-api-exception.filter';
|
||||
import { EmailingDomainGraphqlApiExceptionFilter } from 'src/engine/core-modules/emailing-domain/filters/emailing-domain-graphql-api-exception.filter';
|
||||
import { EmailGroupAccessService } from 'src/engine/core-modules/emailing-domain/services/email-group-access.service';
|
||||
import { EmailingDomainService } from 'src/engine/core-modules/emailing-domain/services/emailing-domain.service';
|
||||
import { ResolverValidationPipe } from 'src/engine/core-modules/graphql/pipes/resolver-validation.pipe';
|
||||
@@ -25,7 +26,10 @@ import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
|
||||
FeatureFlagGuard,
|
||||
SettingsPermissionGuard(PermissionFlagType.WORKSPACE),
|
||||
)
|
||||
@UseFilters(EmailGroupAccessGraphqlApiExceptionFilter)
|
||||
@UseFilters(
|
||||
EmailGroupAccessGraphqlApiExceptionFilter,
|
||||
EmailingDomainGraphqlApiExceptionFilter,
|
||||
)
|
||||
@UsePipes(ResolverValidationPipe)
|
||||
@MetadataResolver(() => EmailingDomainDTO)
|
||||
export class EmailingDomainResolver {
|
||||
|
||||
+34
@@ -0,0 +1,34 @@
|
||||
import { type MessageDescriptor } from '@lingui/core';
|
||||
import { msg } from '@lingui/core/macro';
|
||||
import { assertUnreachable } from 'twenty-shared/utils';
|
||||
|
||||
import { CustomException } from 'src/utils/custom-exception';
|
||||
|
||||
export enum EmailingDomainExceptionCode {
|
||||
EMAILING_DOMAIN_ALREADY_REGISTERED = 'EMAILING_DOMAIN_ALREADY_REGISTERED',
|
||||
}
|
||||
|
||||
const getEmailingDomainExceptionUserFriendlyMessage = (
|
||||
code: EmailingDomainExceptionCode,
|
||||
) => {
|
||||
switch (code) {
|
||||
case EmailingDomainExceptionCode.EMAILING_DOMAIN_ALREADY_REGISTERED:
|
||||
return msg`This domain is already registered.`;
|
||||
default:
|
||||
assertUnreachable(code);
|
||||
}
|
||||
};
|
||||
|
||||
export class EmailingDomainException extends CustomException<EmailingDomainExceptionCode> {
|
||||
constructor(
|
||||
message: string,
|
||||
code: EmailingDomainExceptionCode,
|
||||
{ userFriendlyMessage }: { userFriendlyMessage?: MessageDescriptor } = {},
|
||||
) {
|
||||
super(message, code, {
|
||||
userFriendlyMessage:
|
||||
userFriendlyMessage ??
|
||||
getEmailingDomainExceptionUserFriendlyMessage(code),
|
||||
});
|
||||
}
|
||||
}
|
||||
+22
@@ -0,0 +1,22 @@
|
||||
import { Catch, type ExceptionFilter } from '@nestjs/common';
|
||||
|
||||
import { assertUnreachable } from 'twenty-shared/utils';
|
||||
|
||||
import {
|
||||
EmailingDomainException,
|
||||
EmailingDomainExceptionCode,
|
||||
} from 'src/engine/core-modules/emailing-domain/exceptions/emailing-domain.exception';
|
||||
import { ConflictError } from 'src/engine/core-modules/graphql/utils/graphql-errors.util';
|
||||
|
||||
@Catch(EmailingDomainException)
|
||||
export class EmailingDomainGraphqlApiExceptionFilter implements ExceptionFilter {
|
||||
catch(exception: EmailingDomainException) {
|
||||
switch (exception.code) {
|
||||
case EmailingDomainExceptionCode.EMAILING_DOMAIN_ALREADY_REGISTERED:
|
||||
throw new ConflictError(exception);
|
||||
default: {
|
||||
assertUnreachable(exception.code);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
+18
-9
@@ -1,7 +1,10 @@
|
||||
import { Injectable, Logger } from '@nestjs/common';
|
||||
import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { Repository } from 'typeorm';
|
||||
|
||||
import {
|
||||
EmailingDomainDriverException,
|
||||
EmailingDomainDriverExceptionCode,
|
||||
@@ -9,6 +12,10 @@ import {
|
||||
import { EmailingDomainDriverFactory } from 'src/engine/core-modules/emailing-domain/drivers/emailing-domain-driver.factory';
|
||||
import { EmailingDomainStatus } from 'src/engine/core-modules/emailing-domain/drivers/types/emailing-domain-status.type';
|
||||
import { EmailingDomainEntity } from 'src/engine/core-modules/emailing-domain/emailing-domain.entity';
|
||||
import {
|
||||
EmailingDomainException,
|
||||
EmailingDomainExceptionCode,
|
||||
} from 'src/engine/core-modules/emailing-domain/exceptions/emailing-domain.exception';
|
||||
import { UnsubscribeHostnameService } from 'src/engine/core-modules/emailing-domain/services/unsubscribe-hostname.service';
|
||||
import { WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity';
|
||||
import { InjectWorkspaceScopedRepository } from 'src/engine/twenty-orm/workspace-scoped-repository/inject-workspace-scoped-repository.decorator';
|
||||
@@ -21,6 +28,10 @@ export class EmailingDomainService {
|
||||
constructor(
|
||||
@InjectWorkspaceScopedRepository(EmailingDomainEntity)
|
||||
private readonly emailingDomainRepository: WorkspaceScopedRepository<EmailingDomainEntity>,
|
||||
// Domain is globally unique across workspaces, so existence checks need
|
||||
// an unscoped repository
|
||||
@InjectRepository(EmailingDomainEntity)
|
||||
private readonly globalEmailingDomainRepository: Repository<EmailingDomainEntity>,
|
||||
private readonly emailingDomainDriverFactory: EmailingDomainDriverFactory,
|
||||
private readonly unsubscribeHostnameService: UnsubscribeHostnameService,
|
||||
) {}
|
||||
@@ -29,17 +40,15 @@ export class EmailingDomainService {
|
||||
domain: string,
|
||||
workspaceId: string,
|
||||
): Promise<EmailingDomainEntity> {
|
||||
const existingEmailingDomain = await this.emailingDomainRepository.findOne(
|
||||
workspaceId,
|
||||
{
|
||||
const existingEmailingDomain =
|
||||
await this.globalEmailingDomainRepository.findOne({
|
||||
where: { domain },
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
if (existingEmailingDomain) {
|
||||
throw new EmailingDomainDriverException(
|
||||
'Emailing domain already exists for this workspace',
|
||||
EmailingDomainDriverExceptionCode.CONFIGURATION_ERROR,
|
||||
if (isDefined(existingEmailingDomain)) {
|
||||
throw new EmailingDomainException(
|
||||
'Emailing domain is already registered',
|
||||
EmailingDomainExceptionCode.EMAILING_DOMAIN_ALREADY_REGISTERED,
|
||||
);
|
||||
}
|
||||
|
||||
|
||||
+15
@@ -1,6 +1,11 @@
|
||||
import { assertUnreachable } from 'twenty-shared/utils';
|
||||
|
||||
import {
|
||||
EmailingDomainException,
|
||||
EmailingDomainExceptionCode,
|
||||
} from 'src/engine/core-modules/emailing-domain/exceptions/emailing-domain.exception';
|
||||
import {
|
||||
ConflictError,
|
||||
ForbiddenError,
|
||||
InternalServerError,
|
||||
NotFoundError,
|
||||
@@ -32,6 +37,16 @@ export const messageChannelGraphqlApiExceptionHandler = (error: Error) => {
|
||||
}
|
||||
}
|
||||
|
||||
if (error instanceof EmailingDomainException) {
|
||||
switch (error.code) {
|
||||
case EmailingDomainExceptionCode.EMAILING_DOMAIN_ALREADY_REGISTERED:
|
||||
throw new ConflictError(error);
|
||||
default: {
|
||||
return assertUnreachable(error.code);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if (error instanceof ConnectedAccountException) {
|
||||
switch (error.code) {
|
||||
case ConnectedAccountExceptionCode.CONNECTED_ACCOUNT_OWNERSHIP_VIOLATION:
|
||||
|
||||
Reference in New Issue
Block a user