Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added fees to account transfer dialog #3424

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Added fees to account transfer dialog #3424

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions name.abuchen.portfolio.tests/src/name/abuchen/portfolio/model/CrossEntryTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import static org.hamcrest.CoreMatchers.nullValue;
import static org.hamcrest.MatcherAssert.assertThat;

import java.math.BigDecimal;
import java.time.LocalDateTime;
import java.time.Month;

Expand Down Expand Up @@ -113,6 +114,70 @@ public void testAccountTransferEntry()
assertThat(pA.getDateTime(), is(date));
assertThat(pB.getDateTime(), is(date));

assertThat(accountA.getCurrentAmount(LocalDateTime.now().plusMinutes(1)), is(-1000L * Values.Amount.factor()));
assertThat(accountB.getCurrentAmount(LocalDateTime.now().plusMinutes(1)), is(1000L * Values.Amount.factor()));

// check cross entity identification
assertThat(entry.getCrossOwner(pA), is((Object) accountB));
assertThat(entry.getCrossTransaction(pA), is((Transaction) pB));

assertThat(entry.getCrossOwner(pB), is((Object) accountA));
assertThat(entry.getCrossTransaction(pB), is((Transaction) pA));

// check cross editing
pA.setNote("Test"); //$NON-NLS-1$
entry.updateFrom(pA);
assertThat(pB.getNote(), is(pA.getNote()));

pB.setDateTime(LocalDateTime.of(2013, Month.MARCH, 16, 0, 0));
entry.updateFrom(pB);
assertThat(pA.getDateTime(), is(pB.getDateTime()));

// check deletion
accountA.deleteTransaction(pA, client);
assertThat(accountA.getTransactions().size(), is(0));
assertThat(accountB.getTransactions().size(), is(0));
}

@Test
public void testAccountTransferEntryWithExchange()
{
Account accountA = client.getAccounts().get(0);
Account accountB = client.getAccounts().get(1);
accountB.setCurrencyCode("CHF");

AccountTransferEntry entry = new AccountTransferEntry(accountA, accountB);
LocalDateTime date = LocalDateTime.now();
entry.setDate(date);
entry.getSourceTransaction().setCurrencyCode("EUR");
entry.getTargetTransaction().setCurrencyCode("CHF");
entry.getSourceTransaction().setAmount(1000 * Values.Amount.factor());
Transaction.Unit forex = new Transaction.Unit(Transaction.Unit.Type.GROSS_VALUE,
Money.of("EUR", 1000 * Values.Amount.factor()), Money.of("CHF", 750 * Values.Amount.factor()),
new BigDecimal("1.33333333"));
entry.getSourceTransaction().addUnit(forex);
entry.getTargetTransaction().setAmount(750 * Values.Amount.factor());
entry.insert();

assertThat(accountA.getTransactions().size(), is(1));
assertThat(accountB.getTransactions().size(), is(1));

AccountTransaction pA = accountA.getTransactions().get(0);
AccountTransaction pB = accountB.getTransactions().get(0);

assertThat(pA.getType(), is(AccountTransaction.Type.TRANSFER_OUT));
assertThat(pB.getType(), is(AccountTransaction.Type.TRANSFER_IN));

assertThat(pA.getSecurity(), nullValue());
assertThat(pB.getSecurity(), nullValue());
assertThat(pA.getAmount(), is(1000L * Values.Amount.factor()));
assertThat(pB.getAmount(), is(750L * Values.Amount.factor()));
assertThat(pA.getDateTime(), is(date));
assertThat(pB.getDateTime(), is(date));

assertThat(accountA.getCurrentAmount(date.plusMinutes(1)), is(-1000L * Values.Amount.factor()));
assertThat(accountB.getCurrentAmount(date.plusMinutes(1)), is(750L * Values.Amount.factor()));

// check cross entity identification
assertThat(entry.getCrossOwner(pA), is((Object) accountB));
assertThat(entry.getCrossTransaction(pA), is((Transaction) pB));
Expand All @@ -135,6 +200,49 @@ public void testAccountTransferEntry()
assertThat(accountB.getTransactions().size(), is(0));
}

@Test
public void testAccountTransferEntryWithFees()
{
Account accountA = client.getAccounts().get(0);
Account accountB = client.getAccounts().get(1);

AccountTransferEntry entry = new AccountTransferEntry(accountA, accountB);
LocalDateTime date = LocalDateTime.now();
entry.setDate(date);
entry.setCurrencyCode(CurrencyUnit.EUR);
entry.getSourceTransaction().setAmount(1000 * Values.Amount.factor());
entry.getSourceTransaction().addUnit(new Transaction.Unit(Transaction.Unit.Type.FEE,
Money.of("EUR", 100L * Values.Amount.factor())));
entry.getTargetTransaction().setAmount(850 * Values.Amount.factor());
entry.getTargetTransaction().addUnit(
new Transaction.Unit(Transaction.Unit.Type.FEE, Money.of("EUR", 50L * Values.Amount.factor())));
entry.insert();

assertThat(accountA.getTransactions().size(), is(1));
assertThat(accountB.getTransactions().size(), is(1));

AccountTransaction pA = accountA.getTransactions().get(0);
AccountTransaction pB = accountB.getTransactions().get(0);

assertThat(pA.getType(), is(AccountTransaction.Type.TRANSFER_OUT));
assertThat(pB.getType(), is(AccountTransaction.Type.TRANSFER_IN));

assertThat(pA.getSecurity(), nullValue());
assertThat(pB.getSecurity(), nullValue());
assertThat(pA.getDateTime(), is(date));
assertThat(pB.getDateTime(), is(date));

assertThat(accountA.getCurrentAmount(date.plusMinutes(1)), is(-1000L * Values.Amount.factor()));
assertThat(accountB.getCurrentAmount(date.plusMinutes(1)), is(850L * Values.Amount.factor()));

// check cross entity identification
assertThat(entry.getCrossOwner(pA), is((Object) accountB));
assertThat(entry.getCrossTransaction(pA), is((Transaction) pB));

assertThat(entry.getCrossOwner(pB), is((Object) accountA));
assertThat(entry.getCrossTransaction(pB), is((Transaction) pA));
}

@Test
public void testPortoflioTransferEntry()
{
Expand Down
2 changes: 2 additions & 0 deletions name.abuchen.portfolio.ui/src/name/abuchen/portfolio/ui/Messages.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ public class Messages extends NLS
public static String ColumnChangeOnPrevious_MenuLabel;
public static String ColumnChangeOnPrevious_MenuLabelAmount;
public static String ColumnColor;
public static String ColumnCredit;
public static String ColumnCreditNote;
public static String ColumnColumnLabel;
public static String ColumnConvertedAmount;
Expand All @@ -148,6 +149,7 @@ public class Messages extends NLS
public static String ColumnDaysBetweenPostfix;
public static String ColumnDaysHigh;
public static String ColumnDaysLow;
public static String ColumnDebit;
public static String ColumnDebitNote;
public static String ColumnAbsolutePerformance_MenuLabel;
public static String ColumnAbsolutePerformance_Description;
Expand Down
37 changes: 35 additions & 2 deletions ...ortfolio.ui/src/name/abuchen/portfolio/ui/dialogs/transactions/AccountTransferDialog.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,14 @@ protected void createFormElements(Composite editArea)

// other input fields

Input removedFxAmount = new Input(editArea, Messages.ColumnDebit);
removedFxAmount.bindValue(Properties.removedFxAmount.name(), Messages.ColumnDebit, Values.Amount, true);
removedFxAmount.bindCurrency(Properties.sourceAccountCurrency.name());

Input sourceFee = new Input(editArea, "- " + Messages.ColumnFees); //$NON-NLS-1$
sourceFee.bindValue(Properties.sourceFee.name(), Messages.ColumnFees, Values.Amount, false);
sourceFee.bindCurrency(Properties.sourceAccountCurrency.name());

Input fxAmount = new Input(editArea, Messages.ColumnAmount);
fxAmount.bindValue(Properties.fxAmount.name(), Messages.ColumnAmount, Values.Amount, true);
fxAmount.bindCurrency(Properties.sourceAccountCurrency.name());
Expand All @@ -145,6 +153,14 @@ protected void createFormElements(Composite editArea)
amount.bindValue(Properties.amount.name(), Messages.ColumnAmount, Values.Amount, true);
amount.bindCurrency(Properties.targetAccountCurrency.name());

Input targetFee = new Input(editArea, "- " + Messages.ColumnFees); //$NON-NLS-1$ $
targetFee.bindValue(Properties.targetFee.name(), Messages.ColumnFees, Values.Amount, false);
targetFee.bindCurrency(Properties.targetAccountCurrency.name());

Input creditedAmount = new Input(editArea, Messages.ColumnCredit);
creditedAmount.bindValue(Properties.creditedAmount.name(), Messages.ColumnCredit, Values.Amount, true);
creditedAmount.bindCurrency(Properties.targetAccountCurrency.name());

// note

Label lblNote = new Label(editArea, SWT.LEFT);
Expand All @@ -166,17 +182,34 @@ protected void createFormElements(Composite editArea)
.thenBelow(dateTime.date.getControl()).label(dateTime.label).thenRight(dateTime.time)
.thenRight(dateTime.button, 0);

// removedFxAmount
// sourceFee
// fxAmount - exchange rate - amount
forms.thenBelow(fxAmount.value).width(amountWidth).label(fxAmount.label) //
// targetFee
// creditedAmount
forms.thenBelow(removedFxAmount.value).width(amountWidth).label(removedFxAmount.label) //
.suffix(removedFxAmount.currency, currencyWidth) //
// sourceFee
.thenBelow(sourceFee.value).left(fxAmount.value).width(amountWidth).label(sourceFee.label) //
.suffix(sourceFee.currency, currencyWidth) //
// fxAmount - exchange rate - amount
.thenBelow(fxAmount.value).left(removedFxAmount.value).width(amountWidth).label(fxAmount.label) //
.thenRight(fxAmount.currency).width(currencyWidth) //
.thenRight(exchangeRate.label) //
.thenRight(exchangeRate.value).width(amountWidth) //
.thenRight(exchangeRate.buttonInvertExchangeRate, 0) //
.thenRight(exchangeRate.currency).width(amountWidth) //
.thenRight(amount.label) //
.thenRight(amount.value).width(amountWidth) //
// note
.suffix(amount.currency, currencyWidth) //
// targetFee
.thenBelow(targetFee.value).left(fxAmount.value).width(amountWidth).label(targetFee.label) //
.suffix(targetFee.currency, currencyWidth) //
// creditedAmount
.thenBelow(creditedAmount.value).left(fxAmount.value).width(amountWidth)
.label(creditedAmount.label) //
.suffix(creditedAmount.currency, currencyWidth) //
// note
.thenBelow(valueNote).height(SWTHelper.lineHeight(valueNote) * 3)
.left(target.value.getControl()).right(amount.value).label(lblNote);

Expand Down
91 changes: 78 additions & 13 deletions ...portfolio.ui/src/name/abuchen/portfolio/ui/dialogs/transactions/AccountTransferModel.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,10 @@ public class AccountTransferModel extends AbstractModel
{
public enum Properties
{
sourceAccount, targetAccount, date, time, fxAmount, exchangeRate, inverseExchangeRate, amount, //
sourceAccount, targetAccount, date, time, removedFxAmount, fxAmount, //
exchangeRate, inverseExchangeRate, amount, creditedAmount, //
note, sourceAccountCurrency, targetAccountCurrency, exchangeRateCurrencies, //
inverseExchangeRateCurrencies, calculationStatus;
inverseExchangeRateCurrencies, calculationStatus, sourceFee, targetFee;
}

private final Client client;
Expand All @@ -40,9 +41,13 @@ public enum Properties
private LocalDate date = LocalDate.now();
private LocalTime time = PresetValues.getTime();

private long removedFxAmount;
private long fxAmount;
private BigDecimal exchangeRate = BigDecimal.ONE;
private long amount;
private long creditedAmount;
private long sourceFee;
private long targetFee;
private String note;

private IStatus calculationStatus = ValidationStatus.ok();
Expand Down Expand Up @@ -101,18 +106,12 @@ public void applyChanges()

sourceTransaction.clearUnits();

if (sourceAccount.getCurrencyCode().equals(targetAccount.getCurrencyCode()))
{
sourceTransaction.setAmount(amount);
t.getTargetTransaction().setAmount(amount);
}
else
sourceTransaction.setAmount(removedFxAmount);
if (!(sourceAccount.getCurrencyCode().equals(targetAccount.getCurrencyCode())))
{
// TODO improve naming of fields: the source amount is called
// 'fxAmount' while the target amount is just called 'amount' but
// then the source account holds the 'forex' which is switched
sourceTransaction.setAmount(fxAmount);
t.getTargetTransaction().setAmount(amount);

Transaction.Unit forex = new Transaction.Unit(Transaction.Unit.Type.GROSS_VALUE, //
Money.of(sourceAccount.getCurrencyCode(), fxAmount), //
Expand All @@ -121,13 +120,23 @@ public void applyChanges()

sourceTransaction.addUnit(forex);
}
t.getTargetTransaction().setAmount(amount - targetFee);

if (sourceFee != 0)
sourceTransaction.addUnit(new Transaction.Unit(Transaction.Unit.Type.FEE,
Money.of(sourceAccount.getCurrencyCode(), sourceFee)));
if (targetFee != 0)
t.getTargetTransaction().addUnit(new Transaction.Unit(Transaction.Unit.Type.FEE,
Money.of(targetAccount.getCurrencyCode(), targetFee)));
}

@Override
public void resetToNewTransaction()
{
this.source = null;

setSourceFee(0);
setTargetFee(0);
setFxAmount(0);
setAmount(0);
setNote(null);
Expand All @@ -145,8 +154,17 @@ public void setSource(AccountTransferEntry entry)
this.time = transactionDate.toLocalTime();
this.note = entry.getSourceTransaction().getNote();

this.fxAmount = entry.getSourceTransaction().getAmount();
this.amount = entry.getTargetTransaction().getAmount();
// In AccountTransferEntry getAmount() always includes the fees. Here
// amount includes the fees of the target account (but fxAmount is
// without any fees). This way the monetary value of amount equals the
// monetary value of fxAmount with respect to their currencies.
this.removedFxAmount = entry.getSourceTransaction().getAmount();
this.creditedAmount = entry.getTargetTransaction().getAmount();

this.sourceFee = entry.getSourceTransaction().getUnitSum(Transaction.Unit.Type.FEE).getAmount();
this.targetFee = entry.getTargetTransaction().getUnitSum(Transaction.Unit.Type.FEE).getAmount();
this.fxAmount = removedFxAmount - sourceFee;
this.amount = creditedAmount + targetFee;

Optional<Transaction.Unit> forex = entry.getSourceTransaction().getUnit(Transaction.Unit.Type.GROSS_VALUE);

Expand Down Expand Up @@ -272,6 +290,16 @@ public void setTime(LocalTime time)
firePropertyChange(Properties.time.name(), this.time, this.time = time);
}

public long getRemovedFxAmount()
{
return removedFxAmount;
}

public void setRemovedFxAmount(long foreignCurrencyAmount)
{
setFxAmount(foreignCurrencyAmount - sourceFee);
}

public long getFxAmount()
{
return fxAmount;
Expand All @@ -280,6 +308,8 @@ public long getFxAmount()
public void setFxAmount(long foreignCurrencyAmount)
{
firePropertyChange(Properties.fxAmount.name(), this.fxAmount, this.fxAmount = foreignCurrencyAmount);
firePropertyChange(Properties.removedFxAmount.name(), this.removedFxAmount,
this.removedFxAmount = foreignCurrencyAmount + sourceFee);

triggerAmount(Math.round(exchangeRate.doubleValue() * foreignCurrencyAmount));

Expand All @@ -300,7 +330,7 @@ public void setExchangeRate(BigDecimal exchangeRate)
firePropertyChange(Properties.exchangeRate.name(), this.exchangeRate, this.exchangeRate = newRate);
firePropertyChange(Properties.inverseExchangeRate.name(), oldInverseRate, getInverseExchangeRate());

triggerAmount(Math.round(newRate.doubleValue() * fxAmount));
triggerAmount(Math.round(newRate.doubleValue() * (fxAmount - sourceFee)));

firePropertyChange(Properties.calculationStatus.name(), this.calculationStatus,
this.calculationStatus = calculateStatus());
Expand Down Expand Up @@ -344,9 +374,44 @@ public void setAmount(long amount)
this.calculationStatus = calculateStatus());
}

public long getCreditedAmount()
{
return creditedAmount;
}

public void setCreditedAmount(long amount)
{
setAmount(amount + targetFee);
}

public long getSourceFee()
{
return sourceFee;
}

public void setSourceFee(long sourceFeeAmount)
{
firePropertyChange(Properties.sourceFee.name(), this.sourceFee, this.sourceFee = sourceFeeAmount);
setFxAmount(removedFxAmount - sourceFeeAmount);
}

public long getTargetFee()
{
return targetFee;
}

public void setTargetFee(long targetFeeAmount)
{
firePropertyChange(Properties.targetFee.name(), this.targetFee, this.targetFee = targetFeeAmount);
firePropertyChange(Properties.creditedAmount.name(), this.creditedAmount,
this.creditedAmount = this.amount - targetFeeAmount);
}

public void triggerAmount(long amount)
{
firePropertyChange(Properties.amount.name(), this.amount, this.amount = amount);
firePropertyChange(Properties.creditedAmount.name(), this.creditedAmount,
this.creditedAmount = amount - targetFee);
}

public String getNote()
Expand Down
Loading