Skip to content

Commit

Permalink
Remove usage of UrlGeneratorHelper (#1165)
Browse files Browse the repository at this point in the history 
Removes the usage of `UrlGeneratorHelper` in `alerts.controller.spec.ts`. With its removal, it is now easier to catch potential issues around the URL generation that we return to the clients (i.e.: by decoupling the assertions from the implementation).
  • Loading branch information
@fmrsabino
fmrsabino authored Feb 20, 2024
1 parent 2baa536 commit 8b91327
Showing 1 changed file with 44 additions and 95 deletions.
139 changes: 44 additions & 95 deletions src/routes/alerts/alerts.controller.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ import {
multiSendEncoder,
multiSendTransactionsEncoder,
} from '@/domain/contracts/contracts/__tests__/multi-send-encoder.builder';
import { UrlGeneratorHelper } from '@/domain/alerts/urls/url-generator.helper';
import { accountBuilder } from '@/domain/account/entities/__tests__/account.builder';
import { EmailAddress } from '@/domain/account/entities/account.entity';
import { subscriptionBuilder } from '@/domain/account/entities/__tests__/subscription.builder';
Expand Down Expand Up @@ -76,7 +75,6 @@ describe('Alerts (Unit)', () => {
let configurationService: jest.MockedObjectDeep<IConfigurationService>;
let emailApi: jest.MockedObjectDeep<IEmailApi>;
let accountDataSource: jest.MockedObjectDeep<IAccountDataSource>;
let urlGenerator: UrlGeneratorHelper;

const accountRecoverySubscription = subscriptionBuilder()
.with('key', 'account_recovery')
Expand Down Expand Up @@ -120,7 +118,6 @@ describe('Alerts (Unit)', () => {
safeConfigUrl = configurationService.get('safeConfig.baseUri');
signingKey = configurationService.getOrThrow('alerts.signingKey');
emailApi = moduleFixture.get(IEmailApi);
urlGenerator = moduleFixture.get(UrlGeneratorHelper);
accountDataSource = moduleFixture.get(IAccountDataSource);
networkService = moduleFixture.get(NetworkService);
webAppBaseUri = configurationService.getOrThrow('safeWebApp.baseUri');
Expand Down Expand Up @@ -228,17 +225,14 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(1, {
subject: 'Recovery attempt',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: [...safe.owners, owner].map((address) => {
return {
address,
explorerUrl: urlGenerator.addressToExplorerUrl({
chain,
explorerUrl: chain.blockExplorerUriTemplate.address.replace(
'{{address}}',
address,
}),
),
};
}),
threshold: threshold.toString(),
Expand Down Expand Up @@ -339,17 +333,14 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(1, {
subject: 'Recovery attempt',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: [owners[0], owners[2]].map((address) => {
return {
address,
explorerUrl: urlGenerator.addressToExplorerUrl({
chain,
explorerUrl: chain.blockExplorerUriTemplate.address.replace(
'{{address}}',
address,
}),
),
};
}),
threshold: threshold.toString(),
Expand Down Expand Up @@ -449,17 +440,14 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(1, {
subject: 'Recovery attempt',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: [owners[0], newOwner, owners[2]].map((address) => {
return {
address,
explorerUrl: urlGenerator.addressToExplorerUrl({
chain,
explorerUrl: chain.blockExplorerUriTemplate.address.replace(
'{{address}}',
address,
}),
),
};
}),
threshold: safe.threshold.toString(),
Expand Down Expand Up @@ -549,17 +537,14 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(1, {
subject: 'Recovery attempt',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: safe.owners.map((address) => {
return {
address,
explorerUrl: urlGenerator.addressToExplorerUrl({
chain,
explorerUrl: chain.blockExplorerUriTemplate.address.replace(
'{{address}}',
address,
}),
),
};
}),
threshold: threshold.toString(),
Expand Down Expand Up @@ -680,21 +665,18 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(1, {
subject: 'Recovery attempt',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: [
owners[1],
owners[2],
addOwnerWithThreshold.build().owner,
].map((address) => {
return {
address,
explorerUrl: urlGenerator.addressToExplorerUrl({
chain,
explorerUrl: chain.blockExplorerUriTemplate.address.replace(
'{{address}}',
address,
}),
),
};
}),
threshold: removeOwner.build().threshold.toString(),
Expand Down Expand Up @@ -783,17 +765,14 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(1, {
subject: 'Recovery attempt',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: [...safe.owners, owner].map((address) => {
return {
address,
explorerUrl: urlGenerator.addressToExplorerUrl({
chain,
explorerUrl: chain.blockExplorerUriTemplate.address.replace(
'{{address}}',
address,
}),
),
};
}),
threshold: threshold.toString(),
Expand All @@ -807,17 +786,14 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(2, {
subject: 'Recovery attempt',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: [...safe.owners, owner].map((address) => {
return {
address,
explorerUrl: urlGenerator.addressToExplorerUrl({
chain,
explorerUrl: chain.blockExplorerUriTemplate.address.replace(
'{{address}}',
address,
}),
),
};
}),
threshold: threshold.toString(),
Expand Down Expand Up @@ -904,25 +880,20 @@ describe('Alerts (Unit)', () => {
.expect(202)
.expect({});

const expectedWebAppUrl = urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
});
const expectedOwners = [...safe.owners, owner].map((address) => {
return {
address,
explorerUrl: urlGenerator.addressToExplorerUrl({
chain,
explorerUrl: chain.blockExplorerUriTemplate.address.replace(
'{{address}}',
address,
}),
),
};
});

expect(emailApi.createMessage).toHaveBeenCalledTimes(2);
expect(emailApi.createMessage).toHaveBeenCalledWith({
subject: 'Recovery attempt',
substitutions: {
webAppUrl: expectedWebAppUrl,
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: expectedOwners,
threshold: threshold.toString(),
unsubscriptionUrl: `${webAppBaseUri}/unsubscribe?token=${verifiedAccounts[0].unsubscriptionToken}`,
Expand All @@ -935,7 +906,7 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenCalledWith({
subject: 'Recovery attempt',
substitutions: {
webAppUrl: expectedWebAppUrl,
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: expectedOwners,
threshold: threshold.toString(),
unsubscriptionUrl: `${webAppBaseUri}/unsubscribe?token=${verifiedAccounts[1].unsubscriptionToken}`,
Expand Down Expand Up @@ -1024,10 +995,7 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(1, {
subject: 'Malicious transaction',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
unsubscriptionUrl: `${webAppBaseUri}/unsubscribe?token=${verifiedAccounts[0].unsubscriptionToken}`,
},
template: configurationService.getOrThrow(
Expand Down Expand Up @@ -1111,10 +1079,7 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(1, {
subject: 'Malicious transaction',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
unsubscriptionUrl: `${webAppBaseUri}/unsubscribe?token=${verifiedAccounts[0].unsubscriptionToken}`,
},
template: configurationService.getOrThrow(
Expand All @@ -1125,10 +1090,7 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(2, {
subject: 'Malicious transaction',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
unsubscriptionUrl: `${webAppBaseUri}/unsubscribe?token=${verifiedAccounts[0].unsubscriptionToken}`,
},
template: configurationService.getOrThrow(
Expand Down Expand Up @@ -1244,10 +1206,7 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(1, {
subject: 'Malicious transaction',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
unsubscriptionUrl: `${webAppBaseUri}/unsubscribe?token=${verifiedAccounts[0].unsubscriptionToken}`,
},
template: configurationService.getOrThrow(
Expand Down Expand Up @@ -1361,10 +1320,7 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(1, {
subject: 'Malicious transaction',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
unsubscriptionUrl: `${webAppBaseUri}/unsubscribe?token=${verifiedAccounts[0].unsubscriptionToken}`,
},
template: configurationService.getOrThrow(
Expand All @@ -1375,10 +1331,7 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenNthCalledWith(2, {
subject: 'Malicious transaction',
substitutions: {
webAppUrl: urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
}),
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
unsubscriptionUrl: `${webAppBaseUri}/unsubscribe?token=${verifiedAccounts[0].unsubscriptionToken}`,
},
template: configurationService.getOrThrow(
Expand Down Expand Up @@ -1463,24 +1416,20 @@ describe('Alerts (Unit)', () => {
.expect(202)
.expect({});

const expectedWebAppUrl = urlGenerator.addressToSafeWebAppUrl({
chain,
safeAddress: safe.address,
});
const expectedOwners = [...safe.owners, owner].map((address) => {
return {
address,
explorerUrl: urlGenerator.addressToExplorerUrl({
chain,
explorerUrl: chain.blockExplorerUriTemplate.address.replace(
'{{address}}',
address,
}),
),
};
});
expect(emailApi.createMessage).toHaveBeenCalledTimes(2);
expect(emailApi.createMessage).toHaveBeenCalledWith({
subject: 'Recovery attempt',
substitutions: {
webAppUrl: expectedWebAppUrl,
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: expectedOwners,
threshold: threshold.toString(),
unsubscriptionUrl: `${webAppBaseUri}/unsubscribe?token=${verifiedAccounts[0].unsubscriptionToken}`,
Expand All @@ -1491,7 +1440,7 @@ describe('Alerts (Unit)', () => {
expect(emailApi.createMessage).toHaveBeenCalledWith({
subject: 'Recovery attempt',
substitutions: {
webAppUrl: expectedWebAppUrl,
webAppUrl: `${webAppBaseUri}/home?safe=${chain.shortName}:${safe.address}`,
owners: expectedOwners,
threshold: threshold.toString(),
unsubscriptionUrl: `${webAppBaseUri}/unsubscribe?token=${verifiedAccounts[1].unsubscriptionToken}`,
Expand Down

0 comments on commit 8b91327

Please sign in to comment.