Skip to content

Commit 462b05b

Browse files
frisonislandkeshavbhatt
frisonisland
and
keshavbhatt
authored
fix: [CO-1914] original sender missing from cc (#840)
* test(editor-slice-utils): add unit tests for retrieveCC function * test(editor-slice-utils): enhance unit tests for retrieveCC function with additional scenarios * fix: do not remove from address when replying to email using a different identity - added some tests - need to do other tests to check all cases and document the behavior * refactor: reorganize address retrieval logic and enhance unit tests for retrieveALL and retrieveReplyTo functions * test(editor-slice-utils): enhance unit tests for retrieveALL and retrieveReplyTo functions with additional scenarios * test: add higher-level tests on replyAll * test: add some other tests describing CC behavior * test: change description of tests * test: change description of tests pt.2 * test(editor-generator): update participant role constants and enhance reply all message tests * refactor: rename get_available_addresses to get-available-addresses and update imports * feat: add unit tests for getAvailableAddresses function and improve documentation --------- Co-authored-by: Keshav Bhatt <[email protected]> Co-authored-by: Keshav Bhatt <[email protected]>
1 parent 793dbf5 commit 462b05b

8 files changed

+950
-75
lines changed

‎src/helpers/get-available-addresses.ts

+72
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
/*
2+
* SPDX-FileCopyrightText: 2025 Zextras <https://www.zextras.com>
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only
5+
*/
6+
import { getUserAccount, getUserSettings } from '@zextras/carbonio-shell-ui';
7+
import { isArray } from 'lodash';
8+
9+
import { AvailableAddress } from '../carbonio-ui-commons/types/identities';
10+
import { NO_ACCOUNT_NAME } from '../constants';
11+
12+
/**
13+
* Retrieves the available email addresses for the user, including:
14+
* - Primary account email
15+
* - Email aliases
16+
* - Delegated email addresses (with `sendAs` or `sendOnBehalfOf` rights)
17+
*
18+
* @returns {Array<AvailableAddress>} An array of available email addresses with their types and owner accounts.
19+
*/
20+
export const getAvailableAddresses = (): Array<AvailableAddress> => {
21+
const account = getUserAccount();
22+
const settings = getUserSettings();
23+
const result: Array<AvailableAddress> = [];
24+
25+
// Adds the email address of the primary account
26+
result.push({
27+
address: account?.name ?? NO_ACCOUNT_NAME,
28+
type: 'primary',
29+
ownerAccount: account?.name ?? NO_ACCOUNT_NAME
30+
});
31+
32+
// Adds all the aliases
33+
if (settings.attrs.zimbraMailAlias) {
34+
if (isArray(settings.attrs.zimbraMailAlias)) {
35+
result.push(
36+
...settings.attrs.zimbraMailAlias.map<AvailableAddress>((alias: string) => ({
37+
address: alias,
38+
type: 'alias',
39+
ownerAccount: account?.name ?? NO_ACCOUNT_NAME
40+
}))
41+
);
42+
} else {
43+
result.push({
44+
address: settings.attrs.zimbraMailAlias,
45+
type: 'alias',
46+
ownerAccount: account?.name ?? NO_ACCOUNT_NAME
47+
});
48+
}
49+
}
50+
51+
// Adds the email addresses of all the delegation accounts
52+
if (account?.rights?.targets) {
53+
account.rights.targets.forEach((target) => {
54+
if (target.target && (target.right === 'sendAs' || target.right === 'sendOnBehalfOf')) {
55+
target.target.forEach((user) => {
56+
if (user.type === 'account' && user.email) {
57+
user.email.forEach((email) => {
58+
result.push({
59+
address: email.addr,
60+
type: 'delegation',
61+
right: target.right,
62+
ownerAccount: email.addr
63+
});
64+
});
65+
}
66+
});
67+
}
68+
});
69+
}
70+
71+
return result;
72+
};

‎src/helpers/identities.ts

+6-62
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,17 @@
33
*
44
* SPDX-License-Identifier: AGPL-3.0-only
55
*/
6-
import { Account, getUserAccount, getUserSettings, t } from '@zextras/carbonio-shell-ui';
6+
import { Account, getUserAccount, t } from '@zextras/carbonio-shell-ui';
77
import { TFunction } from 'i18next';
8-
import { filter, findIndex, flatten, isArray, map, remove } from 'lodash';
8+
import { filter, findIndex, flatten, map, remove } from 'lodash';
99

10-
import { getFolderIdParts, getMessageOwnerAccountName } from './folders';
1110
import { ParticipantRole } from '../carbonio-ui-commons/constants/participants';
1211
import { getRootsMap } from '../carbonio-ui-commons/store/zustand/folder/hooks';
1312
import type { Folders } from '../carbonio-ui-commons/types/folder';
1413
import { NO_ACCOUNT_NAME } from '../constants';
1514
import type { MailMessage, Participant } from '../types';
15+
import { getFolderIdParts, getMessageOwnerAccountName } from './folders';
16+
import { getAvailableAddresses } from './get-available-addresses';
1617

1718
/**
1819
* The name of the primary identity
@@ -127,63 +128,6 @@ const IdentityTypeWeights = {
127128
const getNoIdentityPlaceholder = (): string =>
128129
t('label.no_identity_selected', '<No identity selected>');
129130

130-
/**
131-
* Returns the list of all the available addresses for the account and their type
132-
*/
133-
const getAvailableAddresses = (): Array<AvailableAddress> => {
134-
const account = getUserAccount();
135-
const settings = getUserSettings();
136-
const result: Array<AvailableAddress> = [];
137-
138-
// Adds the email address of the primary account
139-
result.push({
140-
address: account?.name ?? NO_ACCOUNT_NAME,
141-
type: 'primary',
142-
ownerAccount: account?.name ?? NO_ACCOUNT_NAME
143-
});
144-
145-
// Adds all the aliases
146-
if (settings.attrs.zimbraMailAlias) {
147-
if (isArray(settings.attrs.zimbraMailAlias)) {
148-
result.push(
149-
...(settings.attrs.zimbraMailAlias as string[]).map<AvailableAddress>((alias: string) => ({
150-
address: alias,
151-
type: 'alias',
152-
ownerAccount: account?.name ?? NO_ACCOUNT_NAME
153-
}))
154-
);
155-
} else {
156-
result.push({
157-
address: settings.attrs.zimbraMailAlias as string,
158-
type: 'alias',
159-
ownerAccount: account?.name ?? NO_ACCOUNT_NAME
160-
});
161-
}
162-
}
163-
164-
// Adds the email addresses of all the delegation accounts
165-
if (account?.rights?.targets) {
166-
account.rights.targets.forEach((target) => {
167-
if (target.target && (target.right === 'sendAs' || target.right === 'sendOnBehalfOf')) {
168-
target.target.forEach((user) => {
169-
if (user.type === 'account' && user.email) {
170-
user.email.forEach((email) => {
171-
result.push({
172-
address: email.addr,
173-
type: 'delegation',
174-
right: target.right,
175-
ownerAccount: email.addr
176-
});
177-
});
178-
}
179-
});
180-
}
181-
});
182-
}
183-
184-
return result;
185-
};
186-
187131
/**
188132
* Returns the name of the account that owns the given address
189133
*
@@ -193,7 +137,8 @@ const getAddressOwnerAccount = (address: string): string | null => {
193137
if (!address) {
194138
return null;
195139
}
196-
const addressInfo = getAvailableAddresses().filter((info) => info.address === address);
140+
const availableAddresses = getAvailableAddresses();
141+
const addressInfo = availableAddresses.filter((info) => info.address === address);
197142
if (addressInfo.length === 0) {
198143
return null;
199144
}
@@ -574,7 +519,6 @@ export {
574519
computeIdentityWeight,
575520
filterMatchingRecipients,
576521
getAddressOwnerAccount,
577-
getAvailableAddresses,
578522
getIdentitiesDescriptors,
579523
getIdentityFromParticipant,
580524
getMessageSenderAccount,

‎src/helpers/tests/get-available-addresses.test.ts

+175
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,175 @@
1+
/*
2+
* SPDX-FileCopyrightText: 2025 Zextras <https://www.zextras.com>
3+
*
4+
* SPDX-License-Identifier: AGPL-3.0-only
5+
*/
6+
7+
import { getUserAccount, getUserSettings } from '@zextras/carbonio-shell-ui';
8+
9+
import { NO_ACCOUNT_NAME } from '../../constants';
10+
import { getAvailableAddresses } from '../get-available-addresses';
11+
12+
jest.mock('@zextras/carbonio-shell-ui', () => ({
13+
getUserAccount: jest.fn(),
14+
getUserSettings: jest.fn()
15+
}));
16+
17+
describe('getAvailableAddresses', () => {
18+
const primaryAccountAddress = '[email protected]';
19+
it('should return primary account address when defined', () => {
20+
(getUserAccount as jest.Mock).mockReturnValue({ name: primaryAccountAddress });
21+
(getUserSettings as jest.Mock).mockReturnValue({ attrs: {} });
22+
23+
const result = getAvailableAddresses();
24+
25+
expect(result).toEqual([
26+
{ address: primaryAccountAddress, type: 'primary', ownerAccount: primaryAccountAddress }
27+
]);
28+
});
29+
30+
it('should return primary account address with no account name(NO_ACCOUNT_NAME) when account is null', () => {
31+
(getUserAccount as jest.Mock).mockReturnValue(null);
32+
(getUserSettings as jest.Mock).mockReturnValue({ attrs: {} });
33+
34+
const result = getAvailableAddresses();
35+
36+
expect(result).toEqual([
37+
{ address: NO_ACCOUNT_NAME, type: 'primary', ownerAccount: NO_ACCOUNT_NAME }
38+
]);
39+
});
40+
41+
it('should return primary account address and aliases when they are defined in zimbraMailAlias', () => {
42+
(getUserAccount as jest.Mock).mockReturnValue({ name: primaryAccountAddress });
43+
(getUserSettings as jest.Mock).mockReturnValue({
44+
attrs: { zimbraMailAlias: ['[email protected]', '[email protected]'] }
45+
});
46+
47+
const result = getAvailableAddresses();
48+
49+
expect(result).toEqual([
50+
{ address: primaryAccountAddress, type: 'primary', ownerAccount: primaryAccountAddress },
51+
{ address: '[email protected]', type: 'alias', ownerAccount: primaryAccountAddress },
52+
{ address: '[email protected]', type: 'alias', ownerAccount: primaryAccountAddress }
53+
]);
54+
});
55+
56+
it('should return primary account address and single alias when only one is defined in zimbraMailAlias', () => {
57+
(getUserAccount as jest.Mock).mockReturnValue({ name: primaryAccountAddress });
58+
(getUserSettings as jest.Mock).mockReturnValue({
59+
attrs: { zimbraMailAlias: '[email protected]' }
60+
});
61+
62+
const result = getAvailableAddresses();
63+
64+
expect(result).toEqual([
65+
{ address: primaryAccountAddress, type: 'primary', ownerAccount: primaryAccountAddress },
66+
{ address: '[email protected]', type: 'alias', ownerAccount: primaryAccountAddress }
67+
]);
68+
});
69+
70+
it('should return primary account address and delegation addresses when the delegation rights are defined', () => {
71+
(getUserAccount as jest.Mock).mockReturnValue({
72+
name: primaryAccountAddress,
73+
rights: {
74+
targets: [
75+
{
76+
right: 'sendAs',
77+
target: [{ type: 'account', email: [{ addr: '[email protected]' }] }]
78+
},
79+
{
80+
right: 'sendOnBehalfOf',
81+
target: [{ type: 'account', email: [{ addr: '[email protected]' }] }]
82+
}
83+
]
84+
}
85+
});
86+
(getUserSettings as jest.Mock).mockReturnValue({ attrs: {} });
87+
88+
const result = getAvailableAddresses();
89+
90+
expect(result).toEqual([
91+
{ address: primaryAccountAddress, type: 'primary', ownerAccount: primaryAccountAddress },
92+
{
93+
address: '[email protected]',
94+
type: 'delegation',
95+
right: 'sendAs',
96+
ownerAccount: '[email protected]'
97+
},
98+
{
99+
address: '[email protected]',
100+
type: 'delegation',
101+
right: 'sendOnBehalfOf',
102+
ownerAccount: '[email protected]'
103+
}
104+
]);
105+
});
106+
107+
it('should return primary account address, aliases, and delegation addresses', () => {
108+
(getUserAccount as jest.Mock).mockReturnValue({
109+
name: primaryAccountAddress,
110+
rights: {
111+
targets: [
112+
{
113+
right: 'sendAs',
114+
target: [
115+
{ type: 'account', email: [{ addr: '[email protected]' }] },
116+
{ type: 'account', email: [{ addr: '[email protected]' }] }
117+
]
118+
}
119+
]
120+
}
121+
});
122+
(getUserSettings as jest.Mock).mockReturnValue({
123+
attrs: { zimbraMailAlias: ['[email protected]', '[email protected]'] }
124+
});
125+
126+
const result = getAvailableAddresses();
127+
128+
expect(result).toEqual([
129+
{ address: primaryAccountAddress, type: 'primary', ownerAccount: primaryAccountAddress },
130+
{ address: '[email protected]', type: 'alias', ownerAccount: primaryAccountAddress },
131+
{ address: '[email protected]', type: 'alias', ownerAccount: primaryAccountAddress },
132+
{
133+
address: '[email protected]',
134+
type: 'delegation',
135+
right: 'sendAs',
136+
ownerAccount: '[email protected]'
137+
},
138+
{
139+
address: '[email protected]',
140+
type: 'delegation',
141+
right: 'sendAs',
142+
ownerAccount: '[email protected]'
143+
}
144+
]);
145+
});
146+
147+
it('should return primary account address and no delegation addresses when the delegation rights are different then sendAs and sendOnBehalfOf', () => {
148+
(getUserAccount as jest.Mock).mockReturnValue({
149+
name: primaryAccountAddress,
150+
rights: {
151+
targets: [
152+
{
153+
right: 'sendAsDistList',
154+
target: [{ type: 'account', email: [{ addr: '[email protected]' }] }]
155+
},
156+
{
157+
right: 'viewFreeBusy',
158+
target: [{ type: 'account', email: [{ addr: '[email protected]' }] }]
159+
},
160+
{
161+
right: 'sendOnBehalfOfDistList',
162+
target: [{ type: 'account', email: [{ addr: '[email protected]' }] }]
163+
}
164+
]
165+
}
166+
});
167+
(getUserSettings as jest.Mock).mockReturnValue({ attrs: {} });
168+
169+
const result = getAvailableAddresses();
170+
171+
expect(result).toEqual([
172+
{ address: primaryAccountAddress, type: 'primary', ownerAccount: primaryAccountAddress }
173+
]);
174+
});
175+
});

‎src/helpers/tests/identities.test.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import { faker } from '@faker-js/faker';
77

88
import { FOLDERS } from '../../carbonio-ui-commons/constants/folders';
99
import { ParticipantRole } from '../../carbonio-ui-commons/constants/participants';
10-
import { getRootsMap } from '../../carbonio-ui-commons/store/zustand/folder/hooks';
10+
import { getRootsMap } from '../../carbonio-ui-commons/store/zustand/folder';
1111
import { populateFoldersStore } from '../../carbonio-ui-commons/test/mocks/store/folders';
1212
import { getMocksContext } from '../../carbonio-ui-commons/test/mocks/utils/mocks-context';
1313
import { generateMessage } from '../../tests/generators/generateMessage';

‎src/store/editor-slice-utils.ts

+11-7
Original file line numberDiff line numberDiff line change
@@ -94,12 +94,12 @@ export function retrieveALL(
9494
original.participants,
9595
(c: Participant): boolean => c.type === ParticipantRole.TO
9696
);
97+
9798
const fromEmails = filter(
9899
original.participants,
99-
(c: Participant): boolean =>
100-
c.type === ParticipantRole.FROM &&
101-
replySenderAccountName !== getAddressOwnerAccount(c.address)
100+
(c: Participant): boolean => c.type === ParticipantRole.FROM
102101
);
102+
103103
const replyToParticipants = filter(
104104
original.participants,
105105
(c: Participant): boolean => c.type === ParticipantRole.REPLY_TO
@@ -109,12 +109,16 @@ export function retrieveALL(
109109
filter(original.participants, (c: Participant): boolean => c.type === ParticipantRole.FROM),
110110
(c: Participant): boolean => replySenderAccountName === getAddressOwnerAccount(c.address)
111111
);
112+
112113
if (replyToParticipants.length === 0) {
113114
if (original.parent === FOLDERS.SENT || original.isSentByMe || isSentByMe) {
114-
return filter(toEmails, (c) => replySenderAccountName !== getAddressOwnerAccount(c.address))
115-
.length === 0
116-
? toEmails
117-
: filter(toEmails, (c) => replySenderAccountName !== getAddressOwnerAccount(c.address));
115+
const replyToFrom = changeTypeOfParticipants(fromEmails, ParticipantRole.TO);
116+
const replyingTo = [...replyToFrom, ...toEmails];
117+
const participantsWithoutSender = filter(
118+
replyingTo,
119+
(c) => replySenderAccountName !== getAddressOwnerAccount(c.address)
120+
);
121+
return participantsWithoutSender.length === 0 ? replyToFrom : participantsWithoutSender;
118122
}
119123
return changeTypeOfParticipants(fromEmails, ParticipantRole.TO);
120124
}

0 commit comments

Comments
 (0)