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.

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

Fix #891: new lines in description and memo are no longer exported to QIF #903

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions app/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -229,14 +229,14 @@ dependencies {
}


testImplementation 'org.robolectric:robolectric:4.3.1'
testImplementation 'org.robolectric:robolectric:4.5.1'

testImplementation(
'junit:junit:4.12',
'joda-time:joda-time:2.9.4',
'org.assertj:assertj-core:3.14.0'
)
testImplementation 'org.robolectric:shadows-multidex:4.3.1'
testImplementation 'org.robolectric:shadows-multidex:4.5.1'

androidTestImplementation (
'com.android.support:support-annotations:' + androidSupportVersion,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import static android.support.test.espresso.Espresso.onView;
import static android.support.test.espresso.action.ViewActions.clearText;
import static android.support.test.espresso.action.ViewActions.click;
import static android.support.test.espresso.action.ViewActions.replaceText;
import static android.support.test.espresso.action.ViewActions.typeText;
import static android.support.test.espresso.assertion.ViewAssertions.matches;
import static android.support.test.espresso.matcher.RootMatchers.withDecorView;
Expand Down Expand Up @@ -147,7 +148,7 @@ public static void prepareTestCase(){
}

@Before
public void setUp() throws Exception {
public void setUp() {
mAccountsDbAdapter.deleteAllRecords();
mAccountsDbAdapter.addRecord(mBaseAccount, DatabaseAdapter.UpdateMethod.insert);
mAccountsDbAdapter.addRecord(mTransferAccount, DatabaseAdapter.UpdateMethod.insert);
Expand Down Expand Up @@ -645,7 +646,7 @@ public void testLegacyIntentTransactionRecording(){
transactionIntent.setType(Transaction.MIME_TYPE);
transactionIntent.putExtra(Intent.EXTRA_TITLE, "Power intents");
transactionIntent.putExtra(Intent.EXTRA_TEXT, "Intents for sale");
transactionIntent.putExtra(Transaction.EXTRA_AMOUNT, new BigDecimal(4.99));
transactionIntent.putExtra(Transaction.EXTRA_AMOUNT, new BigDecimal("4.99"));
transactionIntent.putExtra(Transaction.EXTRA_ACCOUNT_UID, TRANSACTIONS_ACCOUNT_UID);
transactionIntent.putExtra(Transaction.EXTRA_TRANSACTION_TYPE, TransactionType.DEBIT.name());
transactionIntent.putExtra(Account.EXTRA_CURRENCY_CODE, "USD");
Expand Down Expand Up @@ -868,6 +869,63 @@ public void editingTransferAccount_shouldKeepSplitAmountsConsistent() {

}

/**
* In this test we check that new lines in the transaction description are replaced
* with spaces after focus change.
*/
@Test
public void noNewLinesinDescriptionAfterFocusChange() {
setDoubleEntryEnabled(true);
setDefaultTransactionType(TransactionType.DEBIT);
validateTransactionListDisplayed();

onView(withId(R.id.fab_create_transaction)).perform(click());

final String description = "\r\nText with\r\n new lines\n";
clickOnView(R.id.input_transaction_name);
onView(withId(R.id.input_transaction_name)).perform(replaceText(description));

Espresso.closeSoftKeyboard();
clickOnView(R.id.input_transaction_amount);
onView(withId(R.id.input_transaction_amount)).perform(typeText("899"));
Espresso.closeSoftKeyboard();

final String expectedDescription = " Text with new lines ";
onView(withId(R.id.input_transaction_name)).check(matches(withText(expectedDescription)));
}

/**
* In this test we check that editing new lines in the transaction description are replaced
* with spaces when saving.
*/
@Test
public void noNewLinesinDescriptionAfterSave() {
setDoubleEntryEnabled(true);
setDefaultTransactionType(TransactionType.DEBIT);
validateTransactionListDisplayed();

onView(withId(R.id.fab_create_transaction)).perform(click());

final String description = "\r\nText with\r\n new lines\n";
onView(withId(R.id.input_transaction_name)).perform(replaceText(description));

onView(withId(R.id.input_transaction_amount)).perform(typeText("899"));
Espresso.closeSoftKeyboard();

onView(withId(R.id.menu_save)).perform(click());

validateTransactionListDisplayed();

List<Transaction> transactions = mTransactionsDbAdapter.getAllTransactionsForAccount(TRANSACTIONS_ACCOUNT_UID);
assertThat(transactions).hasSize(2);
Transaction transaction = transactions.get(0);

// during save the description is also trimmed
final String expectedDescription = "Text with new lines ";
assertThat(transaction.getDescription().equals(expectedDescription));
}


/**
* Simple wrapper for clicking on views with espresso
* @param viewId View resource ID
Expand All @@ -893,7 +951,7 @@ public void run() {
}

@After
public void tearDown() throws Exception {
public void tearDown() {
if (mTransactionsActivity != null)
mTransactionsActivity.finish();
}
Expand Down
19 changes: 11 additions & 8 deletions app/src/main/java/org/gnucash/android/export/qif/QifExporter.java
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public List<String> generateExport() throws ExporterException {
currentAccountUID = accountUID;
writer.append(QifHelper.ACCOUNT_HEADER).append(newLine);
writer.append(QifHelper.ACCOUNT_NAME_PREFIX)
.append(cursor.getString(cursor.getColumnIndexOrThrow("acct1_full_name")))
.append(QifHelper.sanitizeQifLine(cursor.getString(cursor.getColumnIndexOrThrow("acct1_full_name"))))
.append(newLine);
writer.append(QifHelper.ENTRY_TERMINATOR).append(newLine);
writer.append(QifHelper.getQifHeader(cursor.getString(cursor.getColumnIndexOrThrow("acct1_type"))))
Expand All @@ -158,12 +158,15 @@ public List<String> generateExport() throws ExporterException {
.append(newLine);
// Payee / description
writer.append(QifHelper.PAYEE_PREFIX)
.append(cursor.getString(cursor.getColumnIndexOrThrow("trans_desc")))
.append(QifHelper.sanitizeQifLine(cursor.getString(cursor.getColumnIndexOrThrow("trans_desc"))))
.append(newLine);
// Notes, memo
writer.append(QifHelper.MEMO_PREFIX)
.append(cursor.getString(cursor.getColumnIndexOrThrow("trans_notes")))
.append(newLine);
String memo = QifHelper.sanitizeQifLine(cursor.getString(cursor.getColumnIndexOrThrow("trans_notes")));
if (!memo.isEmpty()) {
writer.append(QifHelper.MEMO_PREFIX)
.append(memo)
.append(newLine);
}
// deal with imbalance first
double imbalance = cursor.getDouble(cursor.getColumnIndexOrThrow("trans_acct_balance"));
BigDecimal decimalImbalance = BigDecimal.valueOf(imbalance).setScale(2, BigDecimal.ROUND_HALF_UP);
Expand All @@ -186,10 +189,10 @@ public List<String> generateExport() throws ExporterException {
// amount associated with the header account will not be exported.
// It can be auto balanced when importing to GnuCash
writer.append(QifHelper.SPLIT_CATEGORY_PREFIX)
.append(cursor.getString(cursor.getColumnIndexOrThrow("acct2_full_name")))
.append(QifHelper.sanitizeQifLine(cursor.getString(cursor.getColumnIndexOrThrow("acct2_full_name"))))
.append(newLine);
String splitMemo = cursor.getString(cursor.getColumnIndexOrThrow("split_memo"));
if (splitMemo != null && splitMemo.length() > 0) {
String splitMemo = QifHelper.sanitizeQifLine(cursor.getString(cursor.getColumnIndexOrThrow("split_memo")));
if (!splitMemo.isEmpty()) {
writer.append(QifHelper.SPLIT_MEMO_PREFIX)
.append(splitMemo)
.append(newLine);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,4 +83,11 @@ public static String getQifHeader(AccountType accountType){
public static String getQifHeader(String accountType) {
return getQifHeader(AccountType.valueOf(accountType));
}

static String sanitizeQifLine(String line) {
if (line == null) {
return "";
}
return line.replaceAll("\\r?\\n", " ");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -454,9 +454,27 @@ public void onItemClick(AdapterView<?> adapterView, View view, int position, lon
}
});

mDescriptionEditText.setOnFocusChangeListener(new View.OnFocusChangeListener() {
@Override
public void onFocusChange(View view, boolean hasFocus) {
if (!hasFocus && view instanceof AutoCompleteTextView) {
TransactionFormFragment.this.sanitizeDescription();
}
}
});

mDescriptionEditText.setAdapter(adapter);
}

/**
* Removes new line characters from the description text field and replaces them with spaces.
*/
private void sanitizeDescription() {
String original = mDescriptionEditText.getText().toString();
mDescriptionEditText.setText(original.replaceAll("\\r?\\n", " "));
}


/**
* Initialize views in the fragment with information from a transaction.
* This method is called if the fragment is used for editing a transaction
Expand Down Expand Up @@ -845,6 +863,8 @@ private void saveNewTransaction() {
return;
}

sanitizeDescription();

Transaction transaction = extractTransactionFromView();
if (mEditMode) { //if editing an existing transaction
transaction.setUID(mTransaction.getUID());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import org.gnucash.android.export.ExportFormat;
import org.gnucash.android.export.ExportParams;
import org.gnucash.android.export.qif.QifExporter;
import org.gnucash.android.export.qif.QifHelper;
import org.gnucash.android.model.Account;
import org.gnucash.android.model.Book;
import org.gnucash.android.model.Commodity;
Expand Down Expand Up @@ -156,13 +157,39 @@ public void memoAndDescription_shouldBeExported() throws IOException {
String expectedDescription = "my description";
String expectedMemo = "my memo";

checkGivenMemoAndDescription(expectedDescription,expectedDescription,expectedMemo,expectedMemo);
}

/**
* Test that new lines in the memo and description fields of transactions are not exported.
*/
@Test
public void memoAndDescription_doNotExportNewLines() throws IOException {
final String lineBreak = "\r\n";
final String expectedLineBreakReplacement = " ";

final String descriptionFirstLine = "Test description";
final String descriptionSecondLine = "with 2 lines";
final String originalDescription = descriptionFirstLine + lineBreak + descriptionSecondLine;
final String expectedDescription = descriptionFirstLine + expectedLineBreakReplacement + descriptionSecondLine;

final String memoFirstLine = "My memo has multiply lines";
final String memoSecondLine = "This is the second line";
final String memoThirdLine = "This is the third line";
final String originalMemo = memoFirstLine + lineBreak + memoSecondLine + lineBreak + memoThirdLine;
final String expectedMemo = memoFirstLine + expectedLineBreakReplacement + memoSecondLine + expectedLineBreakReplacement + memoThirdLine;

checkGivenMemoAndDescription(originalDescription,expectedDescription,originalMemo,expectedMemo);
}

private void checkGivenMemoAndDescription(final String originalDescription, final String expectedDescription, final String originalMemo, final String expectedMemo) throws IOException {
AccountsDbAdapter accountsDbAdapter = new AccountsDbAdapter(mDb);

Account account = new Account("Basic Account");
Transaction transaction = new Transaction("One transaction");
transaction.addSplit(new Split(Money.createZeroInstance("EUR"), account.getUID()));
transaction.setDescription(expectedDescription);
transaction.setNote(expectedMemo);
transaction.setDescription(originalDescription);
transaction.setNote(originalMemo);
account.addTransaction(transaction);

accountsDbAdapter.addRecord(account);
Expand All @@ -179,8 +206,8 @@ public void memoAndDescription_shouldBeExported() throws IOException {
File file = new File(exportedFiles.get(0));
String fileContent = readFileContent(file);
assertThat(file).exists().hasExtension("qif");
assertThat(fileContent.contains(expectedDescription));
assertThat(fileContent.contains(expectedMemo));
assertThat(fileContent).contains(QifHelper.PAYEE_PREFIX + expectedDescription);
assertThat(fileContent).contains(QifHelper.MEMO_PREFIX + expectedMemo);
}

@NonNull
Expand Down