Fix issue where editing the transfer account of a transaction could c…

…ause inconsistency in the split value and quantity

Splits now implement Parcelable - easing passing of data between TransactionForm and SplitEditor
Update the transaction UID of all splits whenever the UID of a transaction is set
Add tests for multi-currency transaction creation and editing

- fixes #506
- closes #524

app/src/androidTest/java/org/gnucash/android/test/ui/TransactionsActivityTest.java

@@ -191,10 +191,13 @@ public void testAddTransactionShouldRequireAmount(){
 				.check(matches(isDisplayed()))
 				.perform(typeText("Lunch"));
 
-		onView(withId(R.id.menu_save)).perform(click());
+		Espresso.closeSoftKeyboard();
+
+		onView(withId(R.id.menu_save))
+				.check(matches(isDisplayed()))
+				.perform(click());
 		onView(withText(R.string.title_add_transaction)).check(matches(isDisplayed()));
 
-		Espresso.closeSoftKeyboard();
 		sleep(1000);
 
 		assertToastDisplayed(R.string.toast_transanction_amount_required);
@@ -294,7 +297,9 @@ public void testAddMultiCurrencyTransaction(){
 		Espresso.pressBack(); //close calculator keyboard
 
 		onView(withId(R.id.input_transfer_account_spinner)).perform(click());
-		onView(withText(euroAccount.getFullName())).check(matches(isDisplayed())).perform(click());
+		onView(withText(euroAccount.getFullName()))
+				.check(matches(isDisplayed()))
+				.perform(click());
 
 		onView(withId(R.id.menu_save)).perform(click());
 
@@ -309,7 +314,7 @@ public void testAddMultiCurrencyTransaction(){
 		List<Transaction> allTransactions = mTransactionsDbAdapter.getAllTransactionsForAccount(TRANSACTIONS_ACCOUNT_UID);
 		assertThat(allTransactions).hasSize(transactionCount+1);
 		Transaction multiTrans = allTransactions.get(0);
-
+		assertThat(multiTrans.getSplits()).hasSize(2);
 		assertThat(multiTrans.getSplits()).extracting("mAccountUID")
 				.contains(TRANSACTIONS_ACCOUNT_UID)
 				.contains(euroAccount.getUID());
@@ -339,7 +344,15 @@ public void testEditTransaction(){
 
 		Transaction editedTransaction = mTransactionsDbAdapter.getRecord(mTransaction.getUID());
 		assertThat(editedTransaction.getDescription()).isEqualTo(trnName);
-		assertThat(editedTransaction.getSplits()).isEqualTo(mTransaction.getSplits());
+		assertThat(editedTransaction.getSplits()).hasSize(2);
+
+		Split split = mTransaction.getSplits(TRANSACTIONS_ACCOUNT_UID).get(0);
+		Split editedSplit = editedTransaction.getSplits(TRANSACTIONS_ACCOUNT_UID).get(0);
+		assertThat(split.isEquivalentTo(editedSplit)).isTrue();
+
+		split = mTransaction.getSplits(TRANSFER_ACCOUNT_UID).get(0);
+		editedSplit = editedTransaction.getSplits(TRANSFER_ACCOUNT_UID).get(0);
+		assertThat(split.isEquivalentTo(editedSplit)).isTrue();
 	}
 
 	/**
@@ -403,8 +416,7 @@ public void testSplitEditor(){
 
 		onView(withId(R.id.split_list_layout)).check(matches(allOf(isDisplayed(), hasDescendant(withId(R.id.input_split_amount)))));
 
-		onView(withId(R.id.menu_add_split)).perform(click());
-
+		onView(allOf(withId(R.id.input_split_amount), withText("-499"))).perform(clearText());
 		onView(allOf(withId(R.id.input_split_amount), withText(""))).perform(typeText("400"));
 
 		onView(withId(R.id.menu_save)).perform(click());
@@ -798,9 +810,9 @@ public void editingTransferAccount_shouldKeepSplitAmountsConsistent() {
 		String trnDescription = "Multicurrency Test Trn";
 		Transaction multiTransaction = new Transaction(trnDescription);
 		Split split1 = new Split(expectedValue, TRANSACTIONS_ACCOUNT_UID);
-		split1.setType(TransactionType.DEBIT);
+		split1.setType(TransactionType.CREDIT);
 		Split split2 = new Split(expectedValue, expectedQty, euroAccount.getUID());
-		split2.setType(TransactionType.CREDIT);
+		split2.setType(TransactionType.DEBIT);
 		multiTransaction.addSplit(split1);
 		multiTransaction.addSplit(split2);
 		multiTransaction.setCommodity(COMMODITY);
@@ -811,6 +823,10 @@ public void editingTransferAccount_shouldKeepSplitAmountsConsistent() {
 		assertThat(savedTransaction.getSplits()).extracting("mQuantity").contains(expectedQty);
 		assertThat(savedTransaction.getSplits()).extracting("mValue").contains(expectedValue);
 
+		assertThat(savedTransaction.getSplits(TRANSACTIONS_ACCOUNT_UID).get(0)
+				.isEquivalentTo(multiTransaction.getSplits(TRANSACTIONS_ACCOUNT_UID).get(0)))
+				.isTrue();
+
 		refreshTransactionsList();
 
 		//open transaction for editing
@@ -832,10 +848,17 @@ public void editingTransferAccount_shouldKeepSplitAmountsConsistent() {
 
 		onView(withId(R.id.menu_save)).perform(click());
 
+		Transaction editedTransaction = mTransactionsDbAdapter.getRecord(multiTransaction.getUID());
+		assertThat(editedTransaction.getSplits(TRANSACTIONS_ACCOUNT_UID).get(0)
+				.isEquivalentTo(savedTransaction.getSplits(TRANSACTIONS_ACCOUNT_UID).get(0)))
+				.isTrue();
+
+		Money firstAcctBalance = mAccountsDbAdapter.getAccountBalance(TRANSACTIONS_ACCOUNT_UID);
+		assertThat(firstAcctBalance).isEqualTo(editedTransaction.getBalance(TRANSACTIONS_ACCOUNT_UID));
+
 		Money transferBalance = mAccountsDbAdapter.getAccountBalance(TRANSFER_ACCOUNT_UID);
-		assertThat(transferBalance).isEqualTo(expectedValue);
+		assertThat(transferBalance).isEqualTo(editedTransaction.getBalance(TRANSFER_ACCOUNT_UID));
 
-		Transaction editedTransaction = mTransactionsDbAdapter.getRecord(multiTransaction.getUID());
 		assertThat(editedTransaction.getBalance(TRANSFER_ACCOUNT_UID)).isEqualTo(expectedValue);
 
 		Split transferAcctSplit = editedTransaction.getSplits(TRANSFER_ACCOUNT_UID).get(0);

app/src/main/java/org/gnucash/android/db/adapter/PricesDbAdapter.java

@@ -77,10 +77,14 @@ public Price buildModelInstance(@NonNull final Cursor cursor) {
     }
 
     /**
-     * get the price for commodity / currency pair
+     * Get the price for commodity / currency pair.
+     * The price can be used to convert from one commodity to another. The 'commodity' is the origin and the 'currency' is the target for the conversion.
      *
-     * Pair is used instead of Price because we must sometimes invert the commodity/currency in DB,
-     * rendering the Price UID invalid.
+     * <p>Pair is used instead of Price object because we must sometimes invert the commodity/currency in DB,
+     * rendering the Price UID invalid.</p>
+     *
+     * @param commodityUID GUID of the commodity which is starting point for conversion
+     * @param currencyUID GUID of target commodity for the conversion
      *
      * @return The numerator/denominator pair for commodity / currency pair
      */

app/src/main/java/org/gnucash/android/model/Split.java

@@ -1,6 +1,8 @@
 package org.gnucash.android.model;
 
 
+import android.os.Parcel;
+import android.os.Parcelable;
 import android.support.annotation.NonNull;
 
 import org.gnucash.android.db.adapter.AccountsDbAdapter;
@@ -17,7 +19,7 @@
  *
  * @author Ngewi Fet <ngewif@gmail.com>
  */
-public class Split extends BaseModel{
+public class Split extends BaseModel implements Parcelable{
 
     /**
      * Flag indicating that the split has been reconciled
@@ -89,12 +91,13 @@ public Split(@NonNull Money value, @NonNull Money quantity, String accountUID){
 
     /**
      * Initialize split with a value amount and account
-     * @param amount Money value amount of this split. Value is always in the currency the owning transaction
-     * @param accountUID String UID of transfer account
+     * @param amount Money value amount of this split. Value is always in the currency the owning transaction.
+     *               This amount will be assigned as both the value and the quantity of this split
+     * @param accountUID String UID of owning account
      */
     public Split(@NonNull Money amount, String accountUID){
-        setQuantity(amount);
         setValue(amount);
+        setQuantity(new Money(amount));
         setAccountUID(accountUID);
         //NOTE: This is a rather simplististic approach to the split type.
         //It typically also depends on the account type of the account. But we do not want to access
@@ -442,6 +445,34 @@ public static Split parseSplit(String splitCsvString) {
         }
     }
 
+    /**
+     * Two splits are considered equivalent if all the fields (excluding GUID and timestamps - created, modified, reconciled) are equal.
+     * Any two splits which are equal are also equivalent, but the reverse is not true
+     * <p>The difference with to {@link #equals(Object)} is that the GUID of the split is not considered.
+     * This is useful in cases where a new split is generated for a transaction with the same properties,
+     * but a new GUID is generated e.g. when editing a transaction and modifying the splits</p>
+     *
+     * @param split Other split for which to test equivalence
+     * @return {@code true} if both splits are equivalent, {@code false} otherwise
+     */
+    public boolean isEquivalentTo(Split split){
+        if (this == split) return true;
+        if (super.equals(split)) return true;
+
+        if (mReconcileState != split.mReconcileState) return false;
+        if (!mValue.equals(split.mValue)) return false;
+        if (!mQuantity.equals(split.mQuantity)) return false;
+        if (!mTransactionUID.equals(split.mTransactionUID)) return false;
+        if (!mAccountUID.equals(split.mAccountUID)) return false;
+        if (mSplitType != split.mSplitType) return false;
+        return mMemo != null ? mMemo.equals(split.mMemo) : split.mMemo == null;
+    }
+
+    /**
+     * Two splits are considered equal if all their properties excluding timestampes (created, modified, reconciled) are equal.
+     * @param o Other split to compare for equality
+     * @return {@code true} if this split is equal to {@code o}, {@code false} otherwise
+     */
     @Override
     public boolean equals(Object o) {
         if (this == o) return true;
@@ -457,7 +488,6 @@ public boolean equals(Object o) {
         if (!mAccountUID.equals(split.mAccountUID)) return false;
         if (mSplitType != split.mSplitType) return false;
         return mMemo != null ? mMemo.equals(split.mMemo) : split.mMemo == null;
-
     }
 
     @Override
@@ -472,4 +502,74 @@ public int hashCode() {
         result = 31 * result + (int) mReconcileState;
         return result;
     }
+
+    @Override
+    public int describeContents() {
+        return 0;
+    }
+
+    @Override
+    public void writeToParcel(Parcel dest, int flags) {
+        dest.writeString(getUID());
+        dest.writeString(mAccountUID);
+        dest.writeString(mTransactionUID);
+        dest.writeString(mSplitType.name());
+
+        dest.writeLong(mValue.getNumerator());
+        dest.writeLong(mValue.getDenominator());
+        dest.writeString(mValue.getCommodity().getCurrencyCode());
+
+        dest.writeLong(mQuantity.getNumerator());
+        dest.writeLong(mQuantity.getDenominator());
+        dest.writeString(mQuantity.getCommodity().getCurrencyCode());
+
+        dest.writeString(mMemo == null ? "" : mMemo);
+        dest.writeString(String.valueOf(mReconcileState));
+        dest.writeString(mReconcileDate.toString());
+    }
+
+    /**
+     * Constructor for creating a Split object from a Parcel
+     * @param source Source parcel containing the split
+     * @see #CREATOR
+     */
+    private Split(Parcel source){
+        setUID(source.readString());
+        mAccountUID = source.readString();
+        mTransactionUID = source.readString();
+        mSplitType = TransactionType.valueOf(source.readString());
+
+        long valueNum = source.readLong();
+        long valueDenom = source.readLong();
+        String valueCurrency = source.readString();
+        mValue = new Money(valueNum, valueDenom, valueCurrency);
+
+        long qtyNum = source.readLong();
+        long qtyDenom = source.readLong();
+        String qtyCurrency = source.readString();
+        mQuantity = new Money(qtyNum, qtyDenom, qtyCurrency);
+
+        String memo = source.readString();
+        mMemo = memo.isEmpty() ? null : memo;
+        mReconcileState = source.readString().charAt(0);
+        mReconcileDate = Timestamp.valueOf(source.readString());
+    }
+
+    /**
+     * Creates new Parcels containing the information in this split during serialization
+     */
+    public static final Parcelable.Creator<Split> CREATOR
+            = new Parcelable.Creator<Split>() {
+
+        @Override
+        public Split createFromParcel(Parcel source) {
+            return new Split(source);
+        }
+
+        @Override
+        public Split[] newArray(int size) {
+            return new Split[size];
+        }
+    };
+
 }

app/src/main/java/org/gnucash/android/model/Transaction.java

@@ -189,6 +189,19 @@ public Split createAutoBalanceSplit(){
         return null;
     }
 
+    /**
+     * Set the GUID of the transaction
+     * If the transaction has Splits, their transactionGUID will be updated as well
+     * @param uid String unique ID
+     */
+    @Override
+    public void setUID(String uid) {
+        super.setUID(uid);
+        for (Split split : mSplitList) {
+            split.setTransactionUID(uid);
+        }
+    }
+
     /**
      * Returns list of splits for this transaction
      * @return {@link java.util.List} of splits in the transaction

app/src/main/java/org/gnucash/android/ui/common/UxArgument.java

@@ -102,12 +102,6 @@ public final class UxArgument {
      */
     public static final String BUDGET_AMOUNT_LIST = "budget_amount_list";
 
-    /**
-     * GUID of splits which have been removed from the split editor
-     */
-    public static String REMOVED_SPLITS = "removed_split_guids";
-
-
     //prevent initialization of instances of this class
     private UxArgument(){
         //prevent even the native class from calling the ctor

app/src/main/java/org/gnucash/android/ui/transaction/SplitEditorFragment.java

@@ -92,8 +92,6 @@ public class SplitEditorFragment extends Fragment {
 
     private BigDecimal mBaseAmount = BigDecimal.ZERO;
 
-    private ArrayList<String> mRemovedSplitUIDs = new ArrayList<>();
-
     CalculatorKeyboard mCalculatorKeyboard;
 
     BalanceTextWatcher mImbalanceWatcher = new BalanceTextWatcher();
@@ -131,13 +129,9 @@ public void onActivityCreated(Bundle savedInstanceState) {
 
         //we are editing splits for a new transaction.
         // But the user may have already created some splits before. Let's check
-        List<String> splitStrings = getArguments().getStringArrayList(UxArgument.SPLIT_LIST);
-        List<Split> splitList = new ArrayList<>();
-        if (splitStrings != null) {
-            for (String splitString : splitStrings) {
-                splitList.add(Split.parseSplit(splitString));
-            }
-        }
+
+        List<Split> splitList = getArguments().getParcelableArrayList(UxArgument.SPLIT_LIST);
+        assert splitList != null;
 
         initArgs();
         if (!splitList.isEmpty()) {
@@ -264,7 +258,6 @@ private void setListeners(Split split){
             removeSplitButton.setOnClickListener(new View.OnClickListener() {
                 @Override
                 public void onClick(View view) {
-                    mRemovedSplitUIDs.add(splitUidTextView.getText().toString());
                     mSplitsLinearLayout.removeView(splitView);
                     mSplitItemViewList.remove(splitView);
                     mImbalanceWatcher.afterTextChanged(null);
@@ -377,14 +370,8 @@ private void saveSplits() {
             return;
         }
 
-        List<Split> splitList = extractSplitsFromView();
-        ArrayList<String> splitStrings = new ArrayList<>();
-        for (Split split : splitList) {
-            splitStrings.add(split.toCsv());
-        }
         Intent data = new Intent();
-        data.putStringArrayListExtra(UxArgument.SPLIT_LIST, splitStrings);
-        data.putStringArrayListExtra(UxArgument.REMOVED_SPLITS, mRemovedSplitUIDs);
+        data.putParcelableArrayListExtra(UxArgument.SPLIT_LIST, extractSplitsFromView());
         getActivity().setResult(Activity.RESULT_OK, data);
 
         getActivity().finish();
@@ -394,8 +381,8 @@ private void saveSplits() {
      * Extracts the input from the views and builds {@link org.gnucash.android.model.Split}s to correspond to the input.
      * @return List of {@link org.gnucash.android.model.Split}s represented in the view
      */
-    private List<Split> extractSplitsFromView(){
-        List<Split> splitList = new ArrayList<>();
+    private ArrayList<Split> extractSplitsFromView(){
+        ArrayList<Split> splitList = new ArrayList<>();
         for (View splitView : mSplitItemViewList) {
             SplitViewHolder viewHolder = (SplitViewHolder) splitView.getTag();
             if (viewHolder.splitAmountEditText.getValue() == null)