Skip to content

Commit

Permalink
Fix a navigation bug and a crash from the account pages (#3932)
Browse files Browse the repository at this point in the history
* Don't crash when making a txn from the uncat page

* Always navigate consistently from the txn add/edit page

* Add release notes

* Attempt to fix functional tests

* Update VRT

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
  • Loading branch information
jfdoming and github-actions[bot] authored Dec 6, 2024
1 parent 5e7538f commit df7bc5d
Show file tree
Hide file tree
Showing 14 changed files with 91 additions and 30 deletions.
8 changes: 8 additions & 0 deletions packages/desktop-client/e2e/page-models/mobile-navigation.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { MobileAccountPage } from './mobile-account-page';
import { MobileAccountsPage } from './mobile-accounts-page';
import { MobileBudgetPage } from './mobile-budget-page';
import { MobileTransactionEntryPage } from './mobile-transaction-entry-page';
Expand All @@ -22,6 +23,13 @@ export class MobileNavigation {
return new MobileAccountsPage(this.page);
}

async goToUncategorizedPage() {
const button = this.page.getByRole('button', { name: /uncategorized/ });
await button.click();

return new MobileAccountPage(this.page);
}

async goToTransactionEntryPage() {
const link = this.page.getByRole('link', { name: 'Transaction' });
await link.click();
Expand Down
77 changes: 72 additions & 5 deletions packages/desktop-client/e2e/transactions.mobile.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,11 +48,8 @@ test.describe('Mobile Transactions', () => {
);
await expect(page).toMatchThemeScreenshots();

const accountPage = await transactionEntryPage.createTransaction();

await expect(accountPage.transactions.nth(0)).toHaveText(
'KrogerClothing-12.34',
);
await transactionEntryPage.createTransaction();
await expect(page.getByLabel('Transaction list')).toHaveCount(0);
await expect(page).toMatchThemeScreenshots();
});

Expand Down Expand Up @@ -82,4 +79,74 @@ test.describe('Mobile Transactions', () => {
'KrogerClothing-12.34',
);
});

test('creates an uncategorized transaction from `/accounts/uncategorized` page', async () => {
// Create uncategorized transaction
let transactionEntryPage = await navigation.goToTransactionEntryPage();
await transactionEntryPage.amountField.fill('12.35');
// Click anywhere to cancel active edit.
await transactionEntryPage.header.click();
await transactionEntryPage.fillField(
page.getByTestId('account-field'),
'Ally Savings',
);
await transactionEntryPage.createTransaction();

const uncategorizedPage = await navigation.goToUncategorizedPage();
transactionEntryPage = await uncategorizedPage.clickCreateTransaction();

await expect(transactionEntryPage.header).toHaveText('New Transaction');

await transactionEntryPage.amountField.fill('12.34');
// Click anywhere to cancel active edit.
await transactionEntryPage.header.click();
await transactionEntryPage.fillField(
page.getByTestId('payee-field'),
'Kroger',
);

await transactionEntryPage.createTransaction();

await expect(uncategorizedPage.transactions.nth(0)).toHaveText(
'KrogerUncategorized-12.34',
);
await expect(page).toMatchThemeScreenshots();
});

test('creates a categorized transaction from `/accounts/uncategorized` page', async () => {
// Create uncategorized transaction
let transactionEntryPage = await navigation.goToTransactionEntryPage();
await transactionEntryPage.amountField.fill('12.35');
// Click anywhere to cancel active edit.
await transactionEntryPage.header.click();
await transactionEntryPage.fillField(
page.getByTestId('account-field'),
'Ally Savings',
);
await transactionEntryPage.createTransaction();

const uncategorizedPage = await navigation.goToUncategorizedPage();
transactionEntryPage = await uncategorizedPage.clickCreateTransaction();

await expect(transactionEntryPage.header).toHaveText('New Transaction');

await transactionEntryPage.amountField.fill('12.34');
// Click anywhere to cancel active edit.
await transactionEntryPage.header.click();
await transactionEntryPage.fillField(
page.getByTestId('payee-field'),
'Kroger',
);
await transactionEntryPage.fillField(
page.getByTestId('category-field'),
'Clothing',
);

await transactionEntryPage.createTransaction();

await expect(uncategorizedPage.transactions.nth(0)).toHaveText(
'(No payee)Uncategorized-12.35',
);
await expect(page).toMatchThemeScreenshots();
});
});
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,9 @@ export function AccountTransactions({
)
}
leftContent={<MobileBackButton />}
rightContent={<AddTransactionButton accountId={accountId} />}
rightContent={
<AddTransactionButton accountId={account ? accountId : undefined} />
}
/>
}
padding={0}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,6 @@ const TransactionEditInner = memo(function TransactionEditInner({
[unserializedTransactions, dateFormat],
);
const { grouped: categoryGroups } = useCategories();
const { state: locationState } = useLocation();

const [transaction, ...childTransactions] = transactions;

Expand Down Expand Up @@ -538,20 +537,7 @@ const TransactionEditInner = memo(function TransactionEditInner({
}

onSave(transactionsToSave);

const isAddingFromAccountPage = isAdding && locationState?.accountId;
if (!isAddingFromAccountPage || hasAccountChanged.current) {
const { account: accountId } = unserializedTransaction;
const account = accountsById?.[accountId];
if (account) {
navigate(`/accounts/${account.id}`, { replace: true });
} else {
// Handle the case where account is undefined
navigate('/accounts');
}
} else {
navigate(-1);
}
navigate(-1);
};

if (unserializedTransaction.reconciled) {
Expand All @@ -568,15 +554,7 @@ const TransactionEditInner = memo(function TransactionEditInner({
} else {
onConfirmSave();
}
}, [
accountsById,
isAdding,
dispatch,
locationState?.accountId,
navigate,
onSave,
unserializedTransactions,
]);
}, [isAdding, dispatch, navigate, onSave, unserializedTransactions]);

const onUpdateInner = useCallback(
async (serializedTransaction, name, value) => {
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/3932.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Bugfix
authors: [jfdoming]
---

Fix a navigation bug and a crash from the account pages

0 comments on commit df7bc5d

Please sign in to comment.