Browse Source

Fix creation of asset profiles with symbol in wrong letter case by using original symbol

pull/7727/head
Thomas Kaul 3 days ago
parent
commit
125292a5f2
  1. 2
      apps/api/src/app/activities/activities.controller.ts
  2. 5
      apps/api/src/app/import/import.service.ts
  3. 97
      apps/api/src/services/data-provider/data-provider.service.spec.ts
  4. 8
      apps/api/src/services/data-provider/data-provider.service.ts
  5. 59
      apps/api/src/services/queues/data-gathering/data-gathering.service.spec.ts
  6. 4
      apps/api/src/services/queues/data-gathering/data-gathering.service.ts
  7. 9
      apps/client/src/app/components/admin-market-data/admin-market-data.component.ts
  8. 33
      libs/common/src/lib/helper.spec.ts
  9. 17
      libs/common/src/lib/helper.ts
  10. 9
      libs/ui/src/lib/services/admin.service.ts

2
apps/api/src/app/activities/activities.controller.ts

@ -227,7 +227,7 @@ export class ActivitiesController {
let assetProfiles: { let assetProfiles: {
[assetProfileIdentifier: string]: Partial<SymbolProfile>; [assetProfileIdentifier: string]: Partial<SymbolProfile>;
}; } = {};
try { try {
assetProfiles = await this.dataProviderService.validateActivities({ assetProfiles = await this.dataProviderService.validateActivities({

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

@ -847,14 +847,17 @@ export class ImportService {
name, name,
scraperConfiguration, scraperConfiguration,
sectors, sectors,
symbol,
symbolMapping, symbolMapping,
url, url,
updatedAt updatedAt
} = assetProfile; } = assetProfile;
const symbol = activity.assetProfile.symbol;
const validatedAccount = accounts.find(({ id }) => { const validatedAccount = accounts.find(({ id }) => {
return id === accountId; return id === accountId;
}); });
const validatedTags = tags.filter(({ id: tagId }) => { const validatedTags = tags.filter(({ id: tagId }) => {
return tagIds.some((activityTagId) => { return tagIds.some((activityTagId) => {
return activityTagId === tagId; return activityTagId === tagId;

97
apps/api/src/services/data-provider/data-provider.service.spec.ts

@ -0,0 +1,97 @@
import { getAssetProfileIdentifier } from '@ghostfolio/common/helper';
import { DataSource } from '@prisma/client';
import { DataProviderService } from './data-provider.service';
describe('DataProviderService', () => {
let dataProviderService: DataProviderService;
let getAssetProfile: jest.Mock;
beforeEach(() => {
getAssetProfile = jest.fn();
const dataProviderInterface = {
getAssetProfile,
getName: () => {
return DataSource.YAHOO;
}
};
dataProviderService = new DataProviderService(
null,
[dataProviderInterface] as any,
null,
null,
null,
null
);
});
describe('getAssetProfiles', () => {
it('Corrects the letter case of the symbol', async () => {
getAssetProfile.mockResolvedValue({
currency: 'USD',
dataSource: DataSource.YAHOO,
name: 'Apple Inc.',
symbol: 'AAPL'
});
const assetProfiles = await dataProviderService.getAssetProfiles([
{ dataSource: DataSource.YAHOO, symbol: 'aapl' }
]);
expect(
assetProfiles[
getAssetProfileIdentifier({
dataSource: DataSource.YAHOO,
symbol: 'aapl'
})
].symbol
).toEqual('AAPL');
});
it('Keeps the requested symbol if the data provider resolves it to a different symbol', async () => {
getAssetProfile.mockResolvedValue({
currency: 'USD',
dataSource: DataSource.YAHOO,
name: 'Meta Platforms, Inc.',
symbol: 'META'
});
const assetProfiles = await dataProviderService.getAssetProfiles([
{ dataSource: DataSource.YAHOO, symbol: 'FB' }
]);
expect(
assetProfiles[
getAssetProfileIdentifier({
dataSource: DataSource.YAHOO,
symbol: 'FB'
})
].symbol
).toEqual('FB');
});
it('Keeps the requested symbol if the data provider reports no symbol', async () => {
getAssetProfile.mockResolvedValue({
currency: 'USD',
dataSource: DataSource.YAHOO,
name: 'Apple Inc.'
});
const assetProfiles = await dataProviderService.getAssetProfiles([
{ dataSource: DataSource.YAHOO, symbol: 'aapl' }
]);
expect(
assetProfiles[
getAssetProfileIdentifier({
dataSource: DataSource.YAHOO,
symbol: 'aapl'
})
].symbol
).toEqual('aapl');
});
});
});

8
apps/api/src/services/data-provider/data-provider.service.ts

@ -23,6 +23,7 @@ import {
getStartOfUtcDate, getStartOfUtcDate,
isCurrency, isCurrency,
isDerivedCurrency, isDerivedCurrency,
isSameSymbol,
isValidCustomAssetProfileSymbol, isValidCustomAssetProfileSymbol,
isValidSearchQuery isValidSearchQuery
} from '@ghostfolio/common/helper'; } from '@ghostfolio/common/helper';
@ -131,7 +132,12 @@ export class DataProviderService implements OnModuleInit {
] = { ] = {
...assetProfile, ...assetProfile,
name: formatAssetProfileName(assetProfile), name: formatAssetProfileName(assetProfile),
symbol: assetProfile.symbol ?? symbol symbol: isSameSymbol({
symbol1: symbol,
symbol2: assetProfile.symbol
})
? assetProfile.symbol
: symbol
}; };
} }
}) })

59
apps/api/src/services/queues/data-gathering/data-gathering.service.spec.ts

@ -4,36 +4,55 @@ import {
} from '@ghostfolio/common/config'; } from '@ghostfolio/common/config';
import { parseDate } from '@ghostfolio/common/helper'; import { parseDate } from '@ghostfolio/common/helper';
import { DataSource } from '@prisma/client';
import { DataGatheringService } from './data-gathering.service'; import { DataGatheringService } from './data-gathering.service';
describe('DataGatheringService', () => { describe('DataGatheringService', () => {
let dataGatheringQueue: { addBulk: jest.Mock; clean: jest.Mock }; let dataGatheringQueue: { addBulk: jest.Mock; clean: jest.Mock };
let dataGatheringService: DataGatheringService; let dataGatheringService: DataGatheringService;
let dataProviderService: { getHistoricalRaw: jest.Mock };
let prismaService: { marketData: { groupBy: jest.Mock; upsert: jest.Mock } }; let dataProviderService: {
getAssetProfiles: jest.Mock;
getHistoricalRaw: jest.Mock;
};
let prismaService: {
marketData: { groupBy: jest.Mock; upsert: jest.Mock };
symbolProfile: { upsert: jest.Mock };
};
let symbolProfileService: { getSymbolProfiles: jest.Mock };
beforeEach(() => { beforeEach(() => {
dataGatheringQueue = { dataGatheringQueue = {
addBulk: jest.fn().mockResolvedValue([]), addBulk: jest.fn().mockResolvedValue([]),
clean: jest.fn().mockResolvedValue([]) clean: jest.fn().mockResolvedValue([])
}; };
dataProviderService = { getHistoricalRaw: jest.fn() }; dataProviderService = {
getAssetProfiles: jest.fn().mockResolvedValue({}),
getHistoricalRaw: jest.fn()
};
prismaService = { prismaService = {
marketData: { marketData: {
groupBy: jest.fn().mockResolvedValue([]), groupBy: jest.fn().mockResolvedValue([]),
upsert: jest.fn().mockResolvedValue({}) upsert: jest.fn().mockResolvedValue({})
} },
symbolProfile: { upsert: jest.fn().mockResolvedValue({}) }
};
symbolProfileService = {
getSymbolProfiles: jest.fn().mockResolvedValue([])
}; };
dataGatheringService = new DataGatheringService( dataGatheringService = new DataGatheringService(
null, [],
dataGatheringQueue as any, dataGatheringQueue as any,
dataProviderService as any, dataProviderService as any,
null, null,
null, null,
prismaService as any, prismaService as any,
null, null,
null symbolProfileService as any
); );
}); });
@ -110,6 +129,34 @@ describe('DataGatheringService', () => {
}); });
}); });
describe('gatherAssetProfiles', () => {
it('Keeps the requested symbol, so that no duplicate asset profile is created', async () => {
dataProviderService.getAssetProfiles.mockResolvedValue({
'YAHOO-aapl': {
currency: 'USD',
dataSource: DataSource.YAHOO,
name: 'Apple Inc.',
symbol: 'AAPL'
}
});
await dataGatheringService.gatherAssetProfiles([
{ dataSource: DataSource.YAHOO, symbol: 'aapl' }
]);
expect(prismaService.symbolProfile.upsert).toHaveBeenCalledWith(
expect.objectContaining({
where: {
dataSource_symbol: {
dataSource: DataSource.YAHOO,
symbol: 'aapl'
}
}
})
);
});
});
describe('gatherRecentMarketData', () => { describe('gatherRecentMarketData', () => {
it('queries the asset profiles with recent market data once and reuses them', async () => { it('queries the asset profiles with recent market data once and reuses them', async () => {
const assetProfileIdentifiersWithRecentMarketData = [ const assetProfileIdentifiersWithRecentMarketData = [

4
apps/api/src/services/queues/data-gathering/data-gathering.service.ts

@ -128,9 +128,7 @@ export class DataGatheringService {
}); });
} catch (error) { } catch (error) {
this.logger.error( this.logger.error(
`Failed to enhance data for ${symbol} (${ `Failed to enhance data for ${symbol} (${dataSource}) by ${dataEnhancer.getName()}`,
assetProfile.dataSource
}) by ${dataEnhancer.getName()}`,
error error
); );
} }

9
apps/client/src/app/components/admin-market-data/admin-market-data.component.ts

@ -496,14 +496,19 @@ export class GfAdminMarketDataComponent implements AfterViewInit, OnInit {
this.adminService this.adminService
.addAssetProfile({ dataSource, symbol }) .addAssetProfile({ dataSource, symbol })
.pipe(takeUntilDestroyed(this.destroyRef)) .pipe(takeUntilDestroyed(this.destroyRef))
.subscribe(() => { .subscribe((assetProfile) => {
this.loadData(); this.loadData();
this.onOpenAssetProfileDialog({
dataSource,
symbol: assetProfile?.symbol ?? symbol
});
}); });
} else { } else {
this.loadData(); this.loadData();
}
this.onOpenAssetProfileDialog({ dataSource, symbol }); this.onOpenAssetProfileDialog({ dataSource, symbol });
}
}); });
}); });
} }

33
libs/common/src/lib/helper.spec.ts

@ -11,6 +11,7 @@ import {
isAccountExcluded, isAccountExcluded,
isCurrency, isCurrency,
isCurrencySymbol, isCurrencySymbol,
isSameSymbol,
isSplitRatio, isSplitRatio,
isValidCustomAssetProfileSymbol, isValidCustomAssetProfileSymbol,
isValidGranteeOfAccess, isValidGranteeOfAccess,
@ -311,6 +312,38 @@ describe('Helper', () => {
}); });
}); });
describe('Is same symbol', () => {
it('Same symbol', () => {
expect(isSameSymbol({ symbol1: 'AAPL', symbol2: 'AAPL' })).toEqual(true);
});
it('Same symbol in a different letter case', () => {
expect(isSameSymbol({ symbol1: 'aapl', symbol2: 'AAPL' })).toEqual(true);
expect(isSameSymbol({ symbol1: 'AaPl', symbol2: 'AAPL' })).toEqual(true);
expect(
isSameSymbol({ symbol1: 'usd-coin', symbol2: 'USD-Coin' })
).toEqual(true);
});
it('Different symbol', () => {
expect(isSameSymbol({ symbol1: 'FB', symbol2: 'META' })).toEqual(false);
expect(
isSameSymbol({ symbol1: 'US0378331005', symbol2: 'AAPL' })
).toEqual(false);
expect(isSameSymbol({ symbol1: 'BRK.B', symbol2: 'BRK-B' })).toEqual(
false
);
});
it('Missing symbol', () => {
expect(isSameSymbol({ symbol1: undefined, symbol2: 'AAPL' })).toEqual(
false
);
expect(isSameSymbol({ symbol1: 'AAPL', symbol2: null })).toEqual(false);
expect(isSameSymbol({ symbol1: '', symbol2: '' })).toEqual(false);
});
});
describe('Is split ratio', () => { describe('Is split ratio', () => {
it('Forward split', () => { it('Forward split', () => {
expect(isSplitRatio({ denominator: 1, numerator: 2 })).toEqual(true); expect(isSplitRatio({ denominator: 1, numerator: 2 })).toEqual(true);

17
libs/common/src/lib/helper.ts

@ -606,6 +606,23 @@ export function isRootCurrency(aCurrency: string) {
}); });
} }
/**
* Checks whether two symbols are the same, ignoring the letter case. Data
* providers can report a symbol in a different letter case than requested, for
* example "AAPL" for "aapl".
*/
export function isSameSymbol({
symbol1,
symbol2
}: {
symbol1: string;
symbol2: string;
}) {
return (
!!symbol1 && !!symbol2 && symbol1.toLowerCase() === symbol2.toLowerCase()
);
}
/** /**
* Validates the ratio of a stock split, expressed as the number of shares held * Validates the ratio of a stock split, expressed as the number of shares held
* after the split (numerator) per number of shares held before (denominator), * after the split (numerator) per number of shares held before (denominator),

9
libs/ui/src/lib/services/admin.service.ts

@ -25,7 +25,12 @@ import { GF_ENVIRONMENT } from '@ghostfolio/ui/environment';
import { HttpClient, HttpHeaders, HttpParams } from '@angular/common/http'; import { HttpClient, HttpHeaders, HttpParams } from '@angular/common/http';
import { inject, Service } from '@angular/core'; import { inject, Service } from '@angular/core';
import { AssetProfileSplit, MarketData, Platform } from '@prisma/client'; import {
AssetProfileSplit,
MarketData,
Platform,
SymbolProfile
} from '@prisma/client';
import { JobStatus } from 'bull'; import { JobStatus } from 'bull';
import { isNumber } from 'lodash'; import { isNumber } from 'lodash';
@ -35,7 +40,7 @@ export class AdminService {
private readonly http = inject(HttpClient); private readonly http = inject(HttpClient);
public addAssetProfile({ dataSource, symbol }: AssetProfileIdentifier) { public addAssetProfile({ dataSource, symbol }: AssetProfileIdentifier) {
return this.http.post<void>( return this.http.post<SymbolProfile>(
`/api/v1/admin/profile-data/${dataSource}/${encodeURIComponent(symbol)}`, `/api/v1/admin/profile-data/${dataSource}/${encodeURIComponent(symbol)}`,
null null
); );

Loading…
Cancel
Save