From de54b6843d102daf11c003b26d60db5456dd8c17 Mon Sep 17 00:00:00 2001 From: hackerESQ Date: Fri, 2 May 2025 18:14:06 -0500 Subject: [PATCH] Fix multi-currency imports (#94) --- app/Console/Commands/RefreshMarketData.php | 3 +- app/Exports/Sheets/ConfigSheet.php | 7 ++- app/Imports/Sheets/ConfigSheet.php | 16 ++++-- app/Imports/Sheets/TransactionsSheet.php | 2 +- app/Models/CurrencyRate.php | 63 ++++++++++++--------- app/Models/Dividend.php | 26 +++++---- app/Models/Portfolio.php | 10 +++- tests/0000_00_00_import_configs_test.xlsx | Bin 11109 -> 11141 bytes tests/ImportExportTest.php | 1 - tests/MultiCurrencyTest.php | 14 ++--- 10 files changed, 80 insertions(+), 62 deletions(-) diff --git a/app/Console/Commands/RefreshMarketData.php b/app/Console/Commands/RefreshMarketData.php index 5518581..0c9f9db 100644 --- a/app/Console/Commands/RefreshMarketData.php +++ b/app/Console/Commands/RefreshMarketData.php @@ -7,7 +7,6 @@ namespace App\Console\Commands; use App\Models\Holding; use App\Models\MarketData; use Illuminate\Console\Command; -use Illuminate\Support\Facades\Log; class RefreshMarketData extends Command { @@ -61,7 +60,7 @@ class RefreshMarketData extends Command try { MarketData::getMarketData($holding->symbol, $force); } catch (\Throwable $e) { - Log::error('Could not refresh '.$holding->symbol.' ('.$e->getMessage().')'); + $this->line('Could not refresh '.$holding->symbol.' ('.$e->getMessage().')'); } } } diff --git a/app/Exports/Sheets/ConfigSheet.php b/app/Exports/Sheets/ConfigSheet.php index f01ac32..6ad87c4 100644 --- a/app/Exports/Sheets/ConfigSheet.php +++ b/app/Exports/Sheets/ConfigSheet.php @@ -47,12 +47,13 @@ class ConfigSheet implements FromCollection, WithHeadings, WithTitle ]); // reinvested holdings - Holding::myHoldings()->where('reinvest_dividends', true)->get()->each(function ($holding) use (&$configs) { + $reinvested_holdings = Holding::myHoldings()->where('reinvest_dividends', true)->get(['portfolio_id', 'symbol']); + if ($reinvested_holdings->isNotEmpty()) { $configs->push([ 'key' => 'reinvested_dividends', - 'value' => $holding->id, + 'value' => $reinvested_holdings->toJson(), ]); - }); + } return $configs; } diff --git a/app/Imports/Sheets/ConfigSheet.php b/app/Imports/Sheets/ConfigSheet.php index 4c07b38..bb6d8cc 100644 --- a/app/Imports/Sheets/ConfigSheet.php +++ b/app/Imports/Sheets/ConfigSheet.php @@ -54,11 +54,17 @@ class ConfigSheet implements SkipsEmptyRows, ToCollection, WithEvents, WithHeadi $this->backupImport->user->save(); break; - case 'reinvest_dividends': - - Holding::myHoldings()->where('id', $config['value'])->update([ - 'reinvest_dividends' => true, - ]); + case 'reinvested_dividends': + if (json_validate($config['value'])) { + foreach (json_decode($config['value'], true) as $reinvest) { + Holding::myHoldings($this->backupImport->user->id) + ->where('portfolio_id', $reinvest['portfolio_id']) + ->where('symbol', $reinvest['symbol']) + ->update([ + 'reinvest_dividends' => true, + ]); + } + } break; default: diff --git a/app/Imports/Sheets/TransactionsSheet.php b/app/Imports/Sheets/TransactionsSheet.php index 700a739..30b7029 100644 --- a/app/Imports/Sheets/TransactionsSheet.php +++ b/app/Imports/Sheets/TransactionsSheet.php @@ -48,7 +48,7 @@ class TransactionsSheet implements SkipsEmptyRows, ToCollection, WithEvents, Wit // if has any transactions not in base currency, need to sync timeseries conversion rates if ($transactions->where('currency', '!=', config('investbrain.base_currency'))->isNotEmpty()) { - CurrencyRate::timeSeriesRates('', $transactions->min('date')); + CurrencyRate::timeSeriesRates('', $transactions->min('date'), $transactions->max('date')); } $totalBatches = count($transactions) / $this->batchSize(); diff --git a/app/Models/CurrencyRate.php b/app/Models/CurrencyRate.php index a777aae..a324d16 100644 --- a/app/Models/CurrencyRate.php +++ b/app/Models/CurrencyRate.php @@ -111,7 +111,7 @@ class CurrencyRate extends Model * * @return array */ - public static function timeSeriesRates(string $currency, mixed $start = null, mixed $end = null): array + public static function timeSeriesRates(string|array $currency, mixed $start = null, mixed $end = null): array { if (empty($start)) { return []; @@ -134,22 +134,27 @@ class CurrencyRate extends Model [$currency, $adjustment] = self::getCurrencyAliasAdjustments($currency); - $currencies = Currency::all()->pluck('currency')->toArray(); + if (! empty($currency)) { - // call api in chunks - $rates = []; - foreach (collect($period)->chunk(500) as $chunk) { + $currencies = Arr::wrap($currency); - $chunkRates = Frankfurter::setSymbols($currencies)->timeSeries($chunk->min(), $chunk->max()); + } else { - $rates = array_merge($rates, Arr::get($chunkRates, 'rates', [])); + $currencies = Currency::all()->pluck('currency')->toArray(); } + // get rates + $rates = Frankfurter::setSymbols($currencies)->timeSeries($period->first(), $period->last()); + + $rates = collect(Arr::get($rates, 'rates', []))->sortKeys()->toArray(); + + $datesOnly = array_keys($rates); + // loop through each date $updates = []; foreach ($period as $date) { - $lookupDate = self::getNearestPastDate($date, $rates); + $lookupDate = self::getNearestPastDate($date, $datesOnly, $rates); if (is_null($lookupDate)) { continue; @@ -181,33 +186,39 @@ class CurrencyRate extends Model ->toArray(); } - private static function getNearestPastDate(CarbonInterface $date, array $rates): ?CarbonInterface + private static function getNearestPastDate(CarbonInterface $date, array $datesOnly, array $rates): ?CarbonInterface { - $datesWithRates = array_keys($rates); - sort($datesWithRates); + + // if no dates, nothing to do... + if (empty($datesOnly)) { + + return null; + } + + $mutableDate = $date->copy(); + $weekAgo = $date->copy()->subWeek(); + $firstDate = Carbon::parse($datesOnly[0]); // get rates or find closest valid rate (handles missing weekend rates) - while (! isset($rates[$date->toDateString()])) { + while (! isset($rates[$mutableDate->toDateString()])) { + + // prevent runaway infinite loops + if ($mutableDate->lessThan($weekAgo)) { + + return null; + } // is this the start of a range that falls on a weekend? - if ($date->lessThan($first_date = Carbon::parse($datesWithRates[0]))) { + if ($mutableDate->lessThan($firstDate)) { - $date = $first_date; - break; + return $firstDate; } // try the day before then - $date = Carbon::parse($date)->subDay(); - - // prevent runaway infinite loops - if ($date->lessThan($date->copy()->subWeek())) { - - $date = null; - break; - } + $mutableDate = $mutableDate->subDay(); } - return $date; + return $mutableDate; } public static function refreshCurrencyData($force = false): void @@ -248,9 +259,7 @@ class CurrencyRate extends Model public static function chunkInsert(array $updates): void { - $chunks = array_chunk($updates, 500); - - foreach ($chunks as $chunk) { + foreach (array_chunk($updates, 500) as $chunk) { QueuedCurrencyRateInsertJob::dispatch($chunk); } diff --git a/app/Models/Dividend.php b/app/Models/Dividend.php index f340566..60bf94e 100644 --- a/app/Models/Dividend.php +++ b/app/Models/Dividend.php @@ -105,20 +105,25 @@ class Dividend extends Model $market_data = MarketData::getMarketData($symbol); - // get historic conversion rates - $rate_to_base = CurrencyRate::timeSeriesRates($market_data->currency, $start_date, $end_date); + $dividend_data + ->chunk(10) + ->each(function ($chunk) use ($market_data) { - // create mass insert - foreach ($dividend_data as $index => $dividend) { - $rate_to_base_date = 1 / Arr::get($rate_to_base, Carbon::parse(Arr::get($dividend, 'date'))->toDateString(), 1); + // get historic conversion rates + $rate_to_base = CurrencyRate::timeSeriesRates($market_data->currency, $chunk->min('date'), $chunk->max('date')); - $dividend['dividend_amount_base'] = $dividend['dividend_amount'] * $rate_to_base_date; + // create mass insert + foreach ($chunk as $index => $dividend) { + $rate_to_base_date = 1 / Arr::get($rate_to_base, Carbon::parse(Arr::get($dividend, 'date'))->toDateString(), 1); - $dividend_data[$index] = [...$dividend, ...['id' => Str::uuid()->toString(), 'updated_at' => now(), 'created_at' => now()]]; - } + $dividend['dividend_amount_base'] = $dividend['dividend_amount'] * $rate_to_base_date; - // insert records - (new self)->insertOrIgnore($dividend_data->toArray()); + $chunk[$index] = [...$dividend, ...['id' => Str::uuid()->toString(), 'updated_at' => now(), 'created_at' => now()]]; + } + + // insert records + (new self)->insertOrIgnore($chunk->toArray()); + }); // sync to holdings self::syncHoldings($symbol); @@ -186,6 +191,7 @@ class Dividend extends Model 'date' => $dividend['date'], 'portfolio_id' => $holding->portfolio_id, 'symbol' => $holding->symbol, + 'currency' => $holding->market_data->currency, 'transaction_type' => 'BUY', 'reinvested_dividend' => true, 'cost_basis' => 0, diff --git a/app/Models/Portfolio.php b/app/Models/Portfolio.php index 4f42002..6a6dc9d 100644 --- a/app/Models/Portfolio.php +++ b/app/Models/Portfolio.php @@ -152,7 +152,12 @@ class Portfolio extends Model $total_performance = []; - $holdings->each(function ($holding) use (&$total_performance) { + // get unique currencies for holdings + foreach ($holdings->groupBy('market_data.currency')->keys() as $currency) { + $currency_rates[$currency] = CurrencyRate::timeSeriesRates($currency, $holdings->min('first_transaction_date'), now()); + } + + $holdings->each(function ($holding) use (&$total_performance, $currency_rates) { $period = CarbonPeriod::create( $holding->first_transaction_date, @@ -163,7 +168,6 @@ class Portfolio extends Model $daily_performance = $holding->dailyPerformance($holding->first_transaction_date, now()); $all_history = app(MarketDataInterface::class)->history($holding->symbol, $holding->first_transaction_date, now()); - $currency_rates = CurrencyRate::timeSeriesRates($holding->market_data->currency, $holding->first_transaction_date, now()); $holding_performance = []; @@ -179,7 +183,7 @@ class Portfolio extends Model $holding_performance[$date] = [ 'date' => $date, 'portfolio_id' => $this->id, - 'total_market_value' => $total_market_value * (1 / Arr::get($currency_rates, $date, 1)), + 'total_market_value' => $total_market_value * (1 / Arr::get($currency_rates[$holding->market_data->currency], $date, 1)), ]; } } diff --git a/tests/0000_00_00_import_configs_test.xlsx b/tests/0000_00_00_import_configs_test.xlsx index 325393660526ac2c02795be735713e11b3505262..49da7ce8efd6861ab549d090214f70a778ae540e 100644 GIT binary patch delta 2404 zcmV-q37huiR)trvBnbr3lVD7fCGxo$<&sl2R@w$GG3SlEvN5+%ZYv}ixyRR0T2r=xZPEt z809pYuB6grAwc#v=%THLUl0C)4^6vS2|{N}<(z6|)Y4u6v{#cp(;xU=5WGsk_B;pg zLkd03*4*vkftBukkX7%1)jNWJ1^%be0dD#2h4FXS$ zgk@S1%Mv9mSa2`tvv#YnlT3lR(J%I=;9d_b3 zS;M}-?x*{*ow~$#9wpUL@c2CT!|>*I&~6RGBx&{iaMT)2hC%DL-%oD(qj55rbY4J= z?nP9Q(q{WDchE5ogMWm7evoYw3H+$!>5;~>zrR}U{-$YSFHW#K-0nHsn4>w|BicT2 znW{~tXxH@nJx6U_f^Wl_>o&jPt(d*^QSZBnn>e#!8v*| z2Tr-eoVLr#>6|g)VuzB9$Q67QjthaN!=%pJNl-USzB(E0>an2}R!(aaQ6i|TN4Qn| zhfu9UjNqF>*`yRAfdDwEn$vuauKog(Q56&k;doB7RRRD2A(KH2A%Dqk+b|G@?*;k} z0`C?j*^zC*a^Nj!4h4#?XPO#Wgh^2)=_M%o?j1_8k|sGA0W5>d==*1eKQ+5OD;XTY zSfT5j#9>4NsJSjgz01ju?{|X<39Msv!KAJsCl6rB?fm11*-4wdtpHqrgxcn$a<0iJ zwY&nwY^WQkai`QqF@J|sv!k{#z>4-IC20~xDOF6=q$6bJ8ip=Q!C|fWK|$?0hylqQ z=5H&}*b7bZYjBF0{h=9ftr|pZg%s|gg_1z=?0#2k!?qIZKF1@*gR{Y3g3s_G)2_c# zQi2<8b?HJR(=O-NUpvl_0G_-2a`>_84aiLX1ZDZCG%M!@PvI;fq^n7i#S^* zgwAKJe|`!$*=OUxvF$gIz#X7QOagDut=4-Vc#lgF5^WpciHtjOgcV4cljRs4{=3z{ zAqvo5;raNYc7N9z`ilv+%)&~`U!rgoO5(*SpkxQ>zUtE_5WC9Z>J1j|os$(0Yp@(G zi>ZWd9b8Lf;(wrozk296cJ6Y2L{7pq9Ytw^5Pq;utBY2fW%quY>zX+>pBa4$&_f8a z4f8iL&io<1C6Bs3bb+y$kj35{=Sec7N357XZZ7}Z98vb06m@zr6u4T<$F}J`N#4AW zd?RG^LbyAfuHuI6;4?EjQQJU5+4dGs({#;`LL58Y_>PaUhgL?N29 z)Y@Gp-U0o`ZT=6FQ56)k!44n^33D|yA)f&N0F;yJ7aV_6U2oGc6n#hHKUjVyOV^aO zosvwW9SEVJv1Z!?LX#8Utyeg9@V%)hi2sheF*L>9qmO-$&c}6r^0PH!1D?>?LLMHB zqyXDEjrP8fSJnGuCIzvwS{Y}dkXs;mQoeqb6R`-hmI}FM?vB%xnl-eF4qOK|+}t_e zDh@|~pHhG4L1|h8aBI@>Xmpge3au24>n#^DJCvfg_@jr#BQQRaWlmV;TwXZOcg~;_ zAD4N`WuETv$GZyIz!3cG7-$IIAgv7&5Aj71#-NArRj2pCl~{sxu-@+^+P$~=G1IzXWW3N`{WHxuavspO1wFTPCxB^k8Pfx*CvRl)vv5quZ^S&)<) zXQy#+ai&YVw`?URady83JsbPJU-eaLBlcggZ@bS=Y1B-caaR&(y-|{*#un;>Nq>^2mPccF`SBAZ)+s%I2T_o*0=MyJMkBZ=x#;8T|9WBL46$RotEAP+aUSUG+F{V1n>#_moYY;7CJQvio_N1X`xr; zxqA4tf24Jpm+P#^v$EP3cYJ-v^XmIJ!=ip%*_7xm-Ta^UF3aOr?#qf97bVZvH^eKT zPOnrT9IYRw-N|q8>Q6}FrEPb01g8H01*HH00000 W000000000;lRGOU1|28>0002y$%Bdj delta 2380 zcmV-S3A6TvSLIf)Bnbre5#)7~CCOC7hh=ahgjOWx|=z=SVEVRj~gGaLrY+*P}ZN|BO%ViLS zVJFC`PMX)U3?8ayt;iC3QgGW}>h)TF%}Bms9gp77eEJ0wCr>3b(C5 zG0IsySt@PlT!QQkX`_vSUkCmqgeIz2g3#GgxnM?Hy>J%*9qdS-=_g@8$g31=&U5fS zq*h?~O87lIuri$wvg#bLI!7>n5`G$ufVC>G#E`rXqO}j)BHX`P2+8kv)qe@(2U;9k;a?N(tAG7aWNzxbb07!EZB)&iN(QWn#>ol=*f(#QmV2X_{92TU8j zT~R2pK&TaHoEs>pz(LOjC;p`8#qb{vrOay&K>Uv2uKzX>z|+LF3#ZN?=ud{S_e!4H)iBEjA;J(M`KYqUMMUIxir` zbYiAy>9YNnd*}qW$Uj1VKj@~8kT5PqdZh6j?yrWwziH~&ix=z#x1lI8qUM&eU)-Be(cvVleF?x{OOFDp~`w*aE_kL zfm7}1000$}K@1^(+is&U5Qgtp+IO&g z9}FZV2?DC-P;D<(TJ7=LF+;Eln6;flwd%X?7#gx|_9ChjA|HnDpZPr|xjohu*@HGx zH7N}|pAu+<%4M@n>5uRCqZuW}@+Rk%Y9OUgVCZf3@k4S@`q7jCmLQ=qDJ`vSW5$dq zq2|U@Ei|}OC|z@Zi&MR2rq#gn?xnh7q3=hm=CYwZA=cM0R8dF)D^cjv9X}$STw+#T{_;$ zJf(k@<5jr052Mj+I$e(z(d2Hl@W<29Y#pp8<9V>2ub0m>OLB=P?5ZLfij>|3@gk%w zOS=B~DdAxL857Gl-=G3v0W|_jTzhVmdUSygxa1?zbODaYg_V0)LRF;{sl@#5GscyxN>j44Vvf1~KQ|wPF~17Q1;P_G&YD zBi5fnUuDa;@R{qaYz(QO==_{gqI<)A564!u&gCikwow*+JIzY``~cSNd)SbLQuZ`) zehweB>@Sm_0~NEw4j>5$qrkhAf&l;kYLo029DkGDPunmQ#os6Gf3W->l76VPC<)ry zfzY%GXn@D*Hol?O96R{lR4ae|Ts|Zemq#DlN56BfouB_}jo6_l?Q9`OCz%vzYp1k* zEaY8vk<6ta2CKk0i-p`H$@B8-mz;=2xNE79TjuU8O{v~u3v}W-v|*=lz6B0T|Cmze z5q}hI5xF(#ILl^f3))IiyWVmkCnG6(tDkyYz6#@+EOXLj&gHf9+&H71xLV~Ymw9^7 zU-v4_28ZHr$4Eo*LDSY~;-S6_V2o-g-*xJkTzRkrHHWf}(5(0E#+ium!<}m1MYuOT zSk}Cc$XsanJCKgDtihnT_PUO|)}7H&#D81Vc83&Fik04Jg*J-23SlE|$6&-S@PCoT zJ(&KJ6U$FIs~3mMFzDmn?=&QSfVc4wZV`-r4%i2asD*FV2LHn-hVF~HnZs;0PEIG& z(`346Hpyn8Fv;qAG@WEHSED9syZ1gk{Adb4F_MmU;onOBquwh1z?+7@QvL@30JEYQ z#03g<_lBa!0ssId2$OLY8MBEW`T-3fFv)vkrbT`-$eB5FvydUi0|jR#iWr@-_+{q8aUOtOH&DA8C08`47{ch$!aYw5VO9=?0) zz@vtcMUn|zx(zuZI!oscJS09rEkiMD`32^4ey=58XA zGg8S3>0W%307_D5pap}07K?(uc@caY{#lTeD`%^*cX6h3yVqs5$^Sricpg7n&9yzj`LC8E0#0k`qswFY(HS6eN; z4YtPYTU~1j**<_z*muU*cv|S#B&aN|h>r`sC_k1DUv`hADp|S83znD5U2)4-x124X zhZ$z|!^*})d+z4{#G9N&Xt86A7_V-4c}=_ks`N_*!om7}+>M+^vmadj1(O#j*aY?w z=Dk}ta_lBaAz$z93IUkeHDHW6L yDlY*xlN~D~8)qeo7@YwC01g8H01*HH00000000000000flUpk!2K^@h0001jjBPjo diff --git a/tests/ImportExportTest.php b/tests/ImportExportTest.php index 9c87469..e2ee125 100644 --- a/tests/ImportExportTest.php +++ b/tests/ImportExportTest.php @@ -84,7 +84,6 @@ class ImportExportTest extends TestCase ]); $holding = Holding::create([ - 'id' => '9cf8a662-7347-49fb-b9de-0cc1430a8d1f', 'portfolio_id' => '9e792bb8-94e7-4ed3-b8cc-43b50d34c337', 'symbol' => 'ACME', 'quantity' => 0, diff --git a/tests/MultiCurrencyTest.php b/tests/MultiCurrencyTest.php index 99b9d98..bbae3e7 100644 --- a/tests/MultiCurrencyTest.php +++ b/tests/MultiCurrencyTest.php @@ -133,11 +133,8 @@ class MultiCurrencyTest extends TestCase $portfolio = Portfolio::factory()->create(); $transaction = Transaction::factory()->buy()->lastYear()->portfolio($portfolio->id)->symbol('ACME')->create(); - $expected_num_calls = count(collect(CarbonPeriod::create($transaction->date, now()))->chunk(500)); - Frankfurter::expects('setSymbols') - ->andReturnSelf() - ->times($expected_num_calls); + ->andReturnSelf(); Frankfurter::expects('timeSeries') ->andReturn(['rates' => [ now()->subDays(3)->toDateString() => [ @@ -152,8 +149,7 @@ class MultiCurrencyTest extends TestCase now()->toDateString() => [ 'ZZZ' => .01, ], - ]]) - ->times($expected_num_calls); + ]]); CurrencyRate::timeSeriesRates( '', // use fake currency to force @@ -252,11 +248,9 @@ class MultiCurrencyTest extends TestCase }); Frankfurter::expects('setSymbols') - ->andReturnSelf() - ->times(4); + ->andReturnSelf(); Frankfurter::expects('timeSeries') - ->andReturn(['rates' => $results]) - ->times(4); + ->andReturn(['rates' => $results]); CurrencyRate::timeSeriesRates('ZZZ', $start, $end); }