Skip to content

Commit f833c52

Browse files
mikonsemikonse
authored
Merge pull request #365 from SFTtech/milo/improve-initial-group-loading
refactor(web): remove the need for redirect-guards in account / transaction details
2 parents 7355694 + 1dfaec7 commit f833c52

12 files changed

Lines changed: 45 additions & 40 deletions

File tree

‎apps/web/src/pages/accounts/AccountDetail/AccountDetail.tsx‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,7 @@ import { AccountTransactionList } from "@/pages/accounts/AccountDetail/AccountTr
22
import { BalanceHistoryGraph } from "@/pages/accounts/AccountDetail/BalanceHistoryGraph";
33
import { ClearingAccountDetail } from "@/pages/accounts/AccountDetail/ClearingAccountDetail";
44
import { MobilePaper } from "@/components/style";
5-
import { Loading } from "@abrechnung/components";
6-
import { useQuery, useTitle } from "@/core/utils";
5+
import { useTitle } from "@/core/utils";
76
import { Grid, Typography } from "@mui/material";
87
import * as React from "react";
98
import { Navigate, useParams } from "react-router";
@@ -35,7 +34,6 @@ export const AccountDetail: React.FC<Props> = ({ groupId }) => {
3534

3635
const group = useGroup(groupId);
3736
const account = useAccount(groupId, accountId);
38-
const query = useQuery();
3937

4038
useTitle(
4139
t(account?.type === "clearing" ? "accounts.detail.tabTitleEvent" : "accounts.detail.tabTitleAccount", "", {
@@ -45,11 +43,7 @@ export const AccountDetail: React.FC<Props> = ({ groupId }) => {
4543
);
4644

4745
if (account === undefined) {
48-
if (query.get("no-redirect") === "true") {
49-
return <Loading />;
50-
} else {
51-
return <Navigate to="/404" />;
52-
}
46+
return <Navigate to={`/groups/${groupId}/accounts`} />;
5347
}
5448

5549
if (account.is_wip) {

‎apps/web/src/pages/accounts/AccountDetail/AccountInfo.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ export const AccountInfo: React.FC<Props> = ({ groupId, account }) => {
9090
// we saved a purely local account, i.e. we created a new account -> navigate to account list
9191
navigate(getAccountListLink(groupId, "personal"));
9292
} else if (oldAccountId !== newAccount.id) {
93-
navigate(getAccountLink(groupId, newAccount.type, newAccount.id) + "?no-redirect=true", {
93+
navigate(getAccountLink(groupId, newAccount.type, newAccount.id), {
9494
replace: true,
9595
});
9696
}

‎apps/web/src/pages/accounts/AccountDetail/AccountTransactionList.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ export const AccountTransactionList: React.FC<Props> = ({ groupId, account }) =>
5555
)
5656
.unwrap()
5757
.then(({ transaction }) => {
58-
navigate(`/groups/${groupId}/transactions/${transaction.id}?no-redirect=true`);
58+
navigate(`/groups/${groupId}/transactions/${transaction.id}`);
5959
})
6060
.catch(() => toast.error("Creating a transaction failed"));
6161
};

‎apps/web/src/pages/accounts/ClearingAccountList/ClearingAccountList.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ export const ClearingAccountList: React.FC<Props> = ({ groupId }) => {
8484
dispatch(createAccount({ groupId, type: "clearing" }))
8585
.unwrap()
8686
.then(({ account }) => {
87-
navigate(getAccountLink(groupId, account.type, account.id) + "?no-redirect=true");
87+
navigate(getAccountLink(groupId, account.type, account.id));
8888
});
8989
};
9090

‎apps/web/src/pages/accounts/PersonalAccountList/PersonalAccountList.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ export const PersonalAccountList: React.FC<Props> = ({ groupId }) => {
7878
dispatch(createAccount({ groupId, type: "personal" }))
7979
.unwrap()
8080
.then(({ account }) => {
81-
navigate(`/groups/${groupId}/accounts/${account.id}?no-redirect=true`);
81+
navigate(`/groups/${groupId}/accounts/${account.id}`);
8282
})
8383
.catch((err) => {
8484
toast.error(`Error while creating account: ${err}`);

‎apps/web/src/pages/accounts/SettlementPlanDisplay.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ export const SettlementPlanDisplay: React.FC<Props> = ({ groupId }) => {
4545
)
4646
.unwrap()
4747
.then(({ transaction }) => {
48-
navigate(`/groups/${groupId}/transactions/${transaction.id}?no-redirect=true`);
48+
navigate(`/groups/${groupId}/transactions/${transaction.id}`);
4949
});
5050
};
5151

‎apps/web/src/pages/groups/Group.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@ import {
22
fetchGroupDependencies,
33
selectGroupAccountsStatus,
44
selectGroupTransactionsStatus,
5+
setAccountStatus,
6+
setTransactionStatus,
57
useGroup,
68
} from "@abrechnung/redux";
79
import React, { Suspense } from "react";

‎apps/web/src/pages/transactions/TransactionDetail/TransactionDetail.tsx‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
import { MobilePaper } from "@/components/style";
2-
import { Loading } from "@abrechnung/components";
32
import { api } from "@/core/api";
4-
import { useQuery, useTitle } from "@/core/utils";
3+
import { useTitle } from "@/core/utils";
54
import { useAppDispatch, useAppSelector } from "@/store";
65
import {
76
deleteTransaction,
@@ -38,7 +37,6 @@ export const TransactionDetail: React.FC<Props> = ({ groupId }) => {
3837
const { t } = useTranslation();
3938
const params = useParams();
4039
const dispatch = useAppDispatch();
41-
const query = useQuery();
4240
const navigate = useNavigate();
4341
const transactionId = Number(params["id"]);
4442

@@ -136,7 +134,7 @@ export const TransactionDetail: React.FC<Props> = ({ groupId }) => {
136134
.then(({ oldTransactionId, transaction }) => {
137135
setShowProgress(false);
138136
if (oldTransactionId !== transaction.id) {
139-
navigate(`/groups/${groupId}/transactions/${transaction.id}?no-redirect=true`, { replace: true });
137+
navigate(`/groups/${groupId}/transactions/${transaction.id}`, { replace: true });
140138
}
141139
})
142140
.catch((err) => {
@@ -146,11 +144,7 @@ export const TransactionDetail: React.FC<Props> = ({ groupId }) => {
146144
}, [transaction, setPositionValidationErrors, dispatch, setShowProgress, navigate, groupId, transactionId]);
147145

148146
if (transaction === undefined) {
149-
if (query.get("no-redirect") === "true") {
150-
return <Loading />;
151-
} else {
152-
return <Navigate to="/404" />;
153-
}
147+
return <Navigate to={`/groups/${groupId}`} />;
154148
}
155149

156150
return (

‎apps/web/src/pages/transactions/TransactionList/TransactionList.tsx‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -221,14 +221,14 @@ export const TransactionList: React.FC<Props> = ({ groupId }) => {
221221
dispatch(createTransaction({ groupId, type: "purchase" }))
222222
.unwrap()
223223
.then(({ transaction }) => {
224-
navigate(`/groups/${groupId}/transactions/${transaction.id}?no-redirect=true`);
224+
navigate(`/groups/${groupId}/transactions/${transaction.id}`);
225225
});
226226
};
227227
const onCreateTransfer = () => {
228228
dispatch(createTransaction({ groupId, type: "transfer" }))
229229
.unwrap()
230230
.then(({ transaction }) => {
231-
navigate(`/groups/${groupId}/transactions/${transaction.id}?no-redirect=true`);
231+
navigate(`/groups/${groupId}/transactions/${transaction.id}`);
232232
});
233233
};
234234

‎libs/redux/src/lib/accounts/accountSlice.ts‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -407,14 +407,19 @@ const accountSlice = createSlice({
407407
const s = getGroupScopedState<AccountState, AccountSliceState>(state, groupId);
408408
removeEntity(s.wipAccounts, accountId);
409409
},
410+
setAccountStatus: (state, action: PayloadAction<{ groupId: number; status: StateStatus }>) => {
411+
const { groupId, status } = action.payload;
412+
initializeGroupState(state, groupId);
413+
const s = getGroupScopedState<AccountState, AccountSliceState>(state, groupId);
414+
if (s) {
415+
s.status = status;
416+
}
417+
},
410418
},
411419
extraReducers: (builder) => {
412420
builder.addCase(fetchAccounts.pending, (state, action) => {
413421
const groupId = action.meta.arg.groupId;
414-
if (!state.byGroupId[groupId]) {
415-
// TODO: add separate base action to do this
416-
initializeGroupState(state, groupId);
417-
}
422+
initializeGroupState(state, groupId);
418423
});
419424
builder.addCase(fetchAccounts.rejected, (state, action) => {
420425
const s = getGroupScopedState<AccountState, AccountSliceState>(state, action.meta.arg.groupId);
@@ -423,10 +428,6 @@ const accountSlice = createSlice({
423428
builder.addCase(fetchAccounts.fulfilled, (state, action) => {
424429
const accounts = action.payload;
425430
const groupId = action.meta.arg.groupId;
426-
if (!state.byGroupId[groupId]) {
427-
// TODO: add separate base action to do this
428-
initializeGroupState(state, groupId);
429-
}
430431
const s = getGroupScopedState<AccountState, AccountSliceState>(state, groupId);
431432
// TODO: optimize such that we maybe only update those who have actually changed??
432433
const byId = accounts.reduce<{ [k: number]: Account }>((byId, account) => {
@@ -475,7 +476,13 @@ const accountSlice = createSlice({
475476
// local reducers
476477
const { advanceNextLocalAccountId } = accountSlice.actions;
477478

478-
export const { wipAccountUpdated, accountAdded, accountEditStarted, copyAccount, discardAccountChange } =
479-
accountSlice.actions;
479+
export const {
480+
wipAccountUpdated,
481+
accountAdded,
482+
accountEditStarted,
483+
copyAccount,
484+
discardAccountChange,
485+
setAccountStatus,
486+
} = accountSlice.actions;
480487

481488
export const { reducer: accountReducer } = accountSlice;

0 commit comments

Comments
 (0)