Task/improve check for duplicates in preview step of activities import (#7541)

* Improve check for duplicates

* Update changelog

CHANGELOG.md

@@ -5,6 +5,18 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## Unreleased
+
+### Changed
+
+- Improved the check for duplicates in the preview step of the activities import (regardless of the account)
+- Improved the check for duplicates in the preview step of the import dividends dialog (regardless of the account)
+- Extended the activities import to reuse an existing account of the user by name and currency
+
+### Fixed
+
+- Fixed the check for duplicates in the preview step of the activities import for activities without a comment
+
 ## 3.44.0 - 2026-08-07
 
 ### Added

apps/api/src/app/import/import.service.ts

@@ -15,7 +15,11 @@ import {
   NON_INVESTMENT_ACTIVITY_TYPES,
   TAG_ID_EXCLUDE_FROM_ANALYSIS
 } from '@ghostfolio/common/config';
-import { CreateAssetProfileDto, CreateOrderDto } from '@ghostfolio/common/dtos';
+import {
+  CreateAccountWithBalancesDto,
+  CreateAssetProfileDto,
+  CreateOrderDto
+} from '@ghostfolio/common/dtos';
 import {
   getAssetProfileIdentifier,
   isValidCustomAssetProfileSymbol,
@@ -34,7 +38,7 @@ import {
 } from '@ghostfolio/common/types';
 
 import { Injectable } from '@nestjs/common';
-import { DataSource, Prisma } from '@prisma/client';
+import { Account, DataSource, Prisma } from '@prisma/client';
 import { Big } from 'big.js';
 import { endOfToday, isAfter, isSameSecond, parseISO } from 'date-fns';
 import { omit, uniqBy } from 'lodash';
@@ -96,7 +100,9 @@ export class ImportService {
             filters,
             userCurrency,
             userId,
-            startDate: parseDate(dateOfFirstActivity)
+            includeDrafts: true,
+            startDate: parseDate(dateOfFirstActivity),
+            withExcludedAccountsAndActivities: true
           }),
           this.symbolProfileService.getSymbolProfiles([
             {
@@ -117,6 +123,8 @@ export class ImportService {
 
       return await Promise.all(
         Object.entries(dividends).map(([dateString, { marketPrice }]) => {
+          const date = parseDate(dateString);
+
           const quantity =
             historicalData.find((historicalDataItem) => {
               return historicalDataItem.date === dateString;
@@ -124,10 +132,8 @@ export class ImportService {
 
           const value = new Big(quantity).mul(marketPrice).toNumber();
 
-          const date = parseDate(dateString);
           const isDuplicate = activities.some((activity) => {
             return (
-              activity.accountId === account?.id &&
               activity.assetProfile.currency === assetProfile.currency &&
               activity.assetProfile.dataSource === assetProfile.dataSource &&
               isSameSecond(activity.date, date) &&
@@ -326,17 +332,29 @@ export class ImportService {
       }
     }
 
-    if (!isDryRun && accountsWithBalancesDto?.length) {
+    if (accountsWithBalancesDto?.length) {
-      const [existingAccounts, existingPlatforms] = await Promise.all([
+      const [
+        existingAccountsOfOtherUsers,
+        existingAccountsOfUser,
+        existingPlatforms
+      ] = await Promise.all([
         this.accountService.accounts({
           where: {
             id: {
-              in: accountsWithBalancesDto.map(({ id }) => {
+              in: accountsWithBalancesDto
-                return id;
+                .filter(({ id }) => {
-              })
+                  return Boolean(id);
-            }
+                })
+                .map(({ id }) => {
+                  return id;
+                })
+            },
+            userId: { not: user.id }
           }
         }),
+        this.accountService.accounts({
+          where: { userId: user.id }
+        }),
         this.platformService.getPlatforms()
       ]);
 
@@ -347,79 +365,116 @@ export class ImportService {
       );
 
       for (const accountWithBalances of accountsWithBalancesDto) {
-        // Check if there is any existing account with the same ID
+        // Skip the account if it already belongs to the user
-        const accountWithSameId = existingAccounts.find((existingAccount) => {
+        if (
-          return existingAccount.id === accountWithBalances.id;
+          existingAccountsOfUser.some(({ id }) => {
+            return id === accountWithBalances.id;
+          })
+        ) {
+          continue;
+        }
+
+        // If there is no account or if the account belongs to a different
+        // user, then reuse an existing account of the user with the same name
+        // and currency or create a new account
+        const accountToReuse = this.getAccountToReuse({
+          accountWithBalances,
+          accountsWithBalancesDto,
+          existingAccountsOfUser
         });
 
-        // If there is no account or if the account belongs to a different user then create a new account
+        if (accountToReuse) {
-        if (!accountWithSameId || accountWithSameId.userId !== user.id) {
+          // Reuse the account of the user instead of creating a duplicate. The
-          const account = omit(accountWithBalances, [
+          // balances, the platform and the tags of the import are deliberately
-            'balance',
+          // not applied to leave the existing account of the user untouched.
-            'balances',
+          if (
-            'isExcluded',
+            accountWithBalances.id &&
-            'tags'
+            accountWithBalances.id !== accountToReuse.id
-          ]);
+          ) {
+            // Store the new to old account ID mappings for updating activities
+            accountIdMapping[accountWithBalances.id] = accountToReuse.id;
+          }
 
-          let oldAccountId: string;
+          continue;
-          const platformId =
+        }
-            platformIdMapping[account.platformId] ?? account.platformId;
 
-          delete account.platformId;
+        if (isDryRun) {
+          continue;
+        }
 
-          if (accountWithSameId) {
+        // Check if there is any existing account of a different user with the
-            oldAccountId = account.id;
+        // same ID, since the ID cannot be reused in this case
-            delete account.id;
+        const accountWithSameIdOfOtherUser = existingAccountsOfOtherUsers.find(
+          ({ id }) => {
+            return id === accountWithBalances.id;
           }
+        );
 
-          const tagIds = (accountWithBalances.tags ?? [])
+        const account = omit(accountWithBalances, [
-            .map((tagId) => {
+          'balance',
-              return tagIdMapping[tagId] ?? tagId;
+          'balances',
-            })
+          'isExcluded',
-            .filter((tagId) => {
+          'tags'
-              return existingTagIds.has(tagId);
+        ]);
-            });
 
-          // Map the legacy isExcluded attribute of old export files to
+        let oldAccountId: string;
-          // the "Exclude from Analysis" tag
+        const platformId =
-          if (
+          platformIdMapping[account.platformId] ?? account.platformId;
-            accountWithBalances.isExcluded &&
-            existingTagIds.has(TAG_ID_EXCLUDE_FROM_ANALYSIS) &&
-            !tagIds.includes(TAG_ID_EXCLUDE_FROM_ANALYSIS)
-          ) {
-            tagIds.push(TAG_ID_EXCLUDE_FROM_ANALYSIS);
-          }
 
-          let accountObject: Prisma.AccountCreateInput = {
+        delete account.platformId;
-            ...account,
-            balances: {
-              create: accountWithBalances.balances ?? []
-            },
-            user: { connect: { id: user.id } }
-          };
 
-          if (
+        if (accountWithSameIdOfOtherUser) {
-            existingPlatforms.some(({ id }) => {
+          oldAccountId = account.id;
-              return id === platformId;
+          delete account.id;
-            })
+        }
-          ) {
-            accountObject = {
-              ...accountObject,
-              platform: { connect: { id: platformId } }
-            };
-          }
 
-          const newAccount = await this.accountService.createAccount({
+        const tagIds = (accountWithBalances.tags ?? [])
-            tagIds,
+          .map((tagId) => {
-            balance: accountWithBalances.balance,
+            return tagIdMapping[tagId] ?? tagId;
-            data: accountObject,
+          })
-            userId: user.id
+          .filter((tagId) => {
+            return existingTagIds.has(tagId);
           });
 
-          // Store the new to old account ID mappings for updating activities
+        // Map the legacy isExcluded attribute of old export files to
-          if (accountWithSameId && oldAccountId) {
+        // the "Exclude from Analysis" tag
-            accountIdMapping[oldAccountId] = newAccount.id;
+        if (
-          }
+          accountWithBalances.isExcluded &&
+          existingTagIds.has(TAG_ID_EXCLUDE_FROM_ANALYSIS) &&
+          !tagIds.includes(TAG_ID_EXCLUDE_FROM_ANALYSIS)
+        ) {
+          tagIds.push(TAG_ID_EXCLUDE_FROM_ANALYSIS);
+        }
+
+        let accountObject: Prisma.AccountCreateInput = {
+          ...account,
+          balances: {
+            create: accountWithBalances.balances ?? []
+          },
+          user: { connect: { id: user.id } }
+        };
+
+        if (
+          existingPlatforms.some(({ id }) => {
+            return id === platformId;
+          })
+        ) {
+          accountObject = {
+            ...accountObject,
+            platform: { connect: { id: platformId } }
+          };
+        }
+
+        const newAccount = await this.accountService.createAccount({
+          tagIds,
+          balance: accountWithBalances.balance,
+          data: accountObject,
+          userId: user.id
+        });
+
+        // Store the new to old account ID mappings for updating activities
+        if (accountWithSameIdOfOtherUser && oldAccountId) {
+          accountIdMapping[oldAccountId] = newAccount.id;
         }
       }
     }
@@ -537,12 +592,12 @@ export class ImportService {
         activity.symbol = assetProfileSymbolMapping[activity.symbol];
       }
 
-      if (!isDryRun) {
+      // If an account is created or reused, then update the accountId in all activities
-        // If a new account is created, then update the accountId in all activities
+      if (accountIdMapping[activity.accountId]) {
-        if (accountIdMapping[activity.accountId]) {
+        activity.accountId = accountIdMapping[activity.accountId];
-          activity.accountId = accountIdMapping[activity.accountId];
+      }
-        }
 
+      if (!isDryRun) {
         // If a new tag is created, then update the tag ID in all activities
         activity.tags = (activity.tags ?? []).map((tagId) => {
           return tagIdMapping[tagId] ?? tagId;
@@ -570,9 +625,20 @@ export class ImportService {
     );
 
     if (isDryRun) {
-      accountsWithBalancesDto.forEach(({ id, name }) => {
+      accountsWithBalancesDto
-        accounts.push({ id, name });
+        .filter(({ id }) => {
-      });
+          // Skip the accounts which are reused or which already belong to the
+          // user, since they are part of the accounts of the user above
+          return (
+            !accountIdMapping[id] &&
+            !accounts.some(({ id: accountId }) => {
+              return accountId === id;
+            })
+          );
+        })
+        .forEach(({ id, name }) => {
+          accounts.push({ id, name });
+        });
     }
 
     const tags = (await this.tagService.getTagsForUser(user.id)).map(
@@ -826,10 +892,10 @@ export class ImportService {
         unitPrice
       }) => {
         const date = parseISO(dateString);
+
         const isDuplicate = existingActivities.some((activity) => {
           return (
-            activity.accountId === accountId &&
+            (activity.comment || null) === (comment || null) &&
-            activity.comment === comment &&
             (activity.currency === currency ||
               activity.assetProfile.currency === currency) &&
             activity.assetProfile.dataSource === dataSource &&
@@ -877,6 +943,52 @@ export class ImportService {
     );
   }
 
+  /**
+   * Returns the account of the user to reuse for the given account of the
+   * import, based on the name and the currency. The currency is considered
+   * because the activities of the import would otherwise end up in an account
+   * of a different currency. The name is only considered if it is unambiguous,
+   * both in the accounts of the user and in the accounts of the import, since
+   * it is not unique. Otherwise, distinct accounts would be merged into a
+   * single one.
+   */
+  private getAccountToReuse({
+    accountWithBalances,
+    accountsWithBalancesDto,
+    existingAccountsOfUser
+  }: {
+    accountWithBalances: CreateAccountWithBalancesDto;
+    accountsWithBalancesDto: ImportDataDto['accounts'];
+    existingAccountsOfUser: Account[];
+  }): Account {
+    const matchingAccountsOfUser = existingAccountsOfUser.filter(
+      ({ currency, name }) => {
+        return (
+          currency === accountWithBalances.currency &&
+          name === accountWithBalances.name
+        );
+      }
+    );
+
+    const matchingAccountsToImport = accountsWithBalancesDto.filter(
+      ({ currency, name }) => {
+        return (
+          currency === accountWithBalances.currency &&
+          name === accountWithBalances.name
+        );
+      }
+    );
+
+    if (
+      matchingAccountsOfUser.length !== 1 ||
+      matchingAccountsToImport.length !== 1
+    ) {
+      return undefined;
+    }
+
+    return matchingAccountsOfUser[0];
+  }
+
   private isUniqueAccount(accounts: AccountWithValue[]) {
     const uniqueAccountIds = new Set<string>();