From 4515566189a7af6f0a0a70e200cda228d0cf9b4a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Aaron=20D=C3=B6tsch?= <aaron@fsmpi.rwth-aachen.de>
Date: Sun, 6 Aug 2023 21:36:15 +0200
Subject: [PATCH] Fix pagination when fetching user history
Until now the timestamp of the last listed item was used to query the next entries with an earlier
timestamp. That works well most of the time but sometimes it can happen that entries have the
exact same timestamp, especially for item transactions for when you buy multiple products at once.
Now it is using the ids for each type respectively and not the timestamp anymore, which should
make it work perfectly.
---
src/components/UserHistory.svelte | 11 ++--
.../admin/user/[id]/history/+page.server.js | 6 +-
src/routes/api/[slug]/+server.js | 15 ++++-
.../api/{transactions.js => transactions.ts} | 57 +++++++++++--------
src/routes/history/+page.server.js | 2 +-
5 files changed, 56 insertions(+), 35 deletions(-)
rename src/routes/api/{transactions.js => transactions.ts} (74%)
diff --git a/src/components/UserHistory.svelte b/src/components/UserHistory.svelte
index 6ff874d..9fe19e2 100644
--- a/src/components/UserHistory.svelte
+++ b/src/components/UserHistory.svelte
@@ -17,11 +17,14 @@
function fetchMoreTransactions(){
if(!hasMoreTransactions || isFetching) return;
isFetching = true;
- fetch("/api/transactions?" + new URLSearchParams({
+ fetch("/api/transactions?" + new URLSearchParams(Object.fromEntries(Object.entries({
userId,
- before: transactions[transactions.length-1].createdAt.getTime(),
- balance: transactions[transactions.length-1].balance-transactions[transactions.length-1].difference
- }), {
+ balance: transactions[transactions.length-1].balance-transactions[transactions.length-1].difference,
+ lastItemTransactionId: transactions.findLast(t=>t.type=="item")?.id,
+ lastMoneyTransactionId: transactions.findLast(t=>t.type=="transaction")?.id,
+ lastMoneyTransferId: transactions.findLast(t=>t.type=="transfer")?.id,
+ lastVoucherUseTime: transactions.findLast(t=>t.type=="voucher")?.id
+ }).filter(([,value])=>value!==undefined))), {
method: "GET"
}).then(r=>r.text()).then(devalue.parse).then(({transactions: prevTransactions, hasMore})=>{
if(prevTransactions.length > 0) transactions = [...transactions, ...prevTransactions];
diff --git a/src/routes/admin/user/[id]/history/+page.server.js b/src/routes/admin/user/[id]/history/+page.server.js
index 0ddcab0..52f6f6e 100644
--- a/src/routes/admin/user/[id]/history/+page.server.js
+++ b/src/routes/admin/user/[id]/history/+page.server.js
@@ -1,9 +1,9 @@
-import { fetchTransactions } from '../../../../api/transactions.js';
-import { getUser } from '../../../api/users.js';
+import { fetchTransactions } from '../../../../api/transactions';
+import { getUser } from '../../../api/users';
export async function load(event) {
const userId = parseInt(event.params.id);
const user = await getUser(userId);
- const { transactions, hasMore } = await fetchTransactions(userId, user.balance, Date.now(), 100);
+ const { transactions, hasMore } = await fetchTransactions(userId, user.balance, {}, 100);
return { transactions, hasMore, user };
};
diff --git a/src/routes/api/[slug]/+server.js b/src/routes/api/[slug]/+server.js
index d8362ed..3f0c87b 100644
--- a/src/routes/api/[slug]/+server.js
+++ b/src/routes/api/[slug]/+server.js
@@ -117,15 +117,24 @@ export async function GET(req){
try {
const userId = parseInt(req.url.searchParams.get("userId"));
const balance = parseInt(req.url.searchParams.get("balance"));
- const before = parseInt(req.url.searchParams.get("before"));
- if (isNaN(userId) || isNaN(balance) || isNaN(before)) {
+ const lastItemTransactionId = req.url.searchParams.get("lastItemTransactionId");
+ const lastMoneyTransactionId = req.url.searchParams.get("lastMoneyTransactionId");
+ const lastMoneyTransferId = req.url.searchParams.get("lastMoneyTransferId");
+ const lastVoucherUseId = req.url.searchParams.get("lastVoucherUseId");
+ const beforeIds = {
+ itemTransactions: lastItemTransactionId ? parseInt(lastItemTransactionId) : undefined,
+ moneyTransactions: lastMoneyTransactionId ? parseInt(lastMoneyTransactionId) : undefined,
+ moneyTransfers: lastMoneyTransferId ? parseInt(lastMoneyTransferId) : undefined,
+ voucherUses: lastVoucherUseId ? parseInt(lastVoucherUseId) : undefined
+ }
+ if (isNaN(userId) || isNaN(balance) || Object.values(beforeIds).some(id => id!==undefined && isNaN(id))) {
return new Response(JSON.stringify({ message: "Invalid fields" }), { status: 400 });
}
if(userId !== req.locals.user.id && !(req.locals.user.perms & Flags.ADMIN)){
return new Response(JSON.stringify({ message: "You can't view other users' transactions" }), { status: 403 });
}
try {
- const transactions = await fetchTransactions(userId, balance, before, 100);
+ const transactions = await fetchTransactions(userId, balance, beforeIds, 100);
// use devalue because JSON.stringify/JSON.parse doesn't really support Date
return new Response(devalue.stringify(transactions), { status: 200 });
} catch (error) {
diff --git a/src/routes/api/transactions.js b/src/routes/api/transactions.ts
similarity index 74%
rename from src/routes/api/transactions.js
rename to src/routes/api/transactions.ts
index 850ec90..3e62f31 100644
--- a/src/routes/api/transactions.js
+++ b/src/routes/api/transactions.ts
@@ -94,45 +94,50 @@ export async function useVoucher(userId, voucherCode){
});
}
-export async function getMoneyTransactions(userId, before=Date.now(), limit=100) {
+export async function getMoneyTransactions(userId, beforeId?: number, limit=100) {
return db.transaction.findMany({
take: limit,
- where: { userId, createdAt: { lt: new Date(before) } },
- orderBy: { createdAt: "desc" }
+ where: { userId, id: { lt: beforeId } },
+ orderBy: { id: "desc" }
});
}
-export async function getItemTransactions(userId, before=Date.now(), limit=100) {
+export async function getItemTransactions(userId, beforeId?: number, limit=100) {
return db.itemTransaction.findMany({
take: limit,
- where: { userId, createdAt: { lt: new Date(before) } },
- orderBy: { createdAt: "desc" },
+ where: { userId, id: { lt: beforeId } },
+ orderBy: { id: "desc" },
include: { item: true }
});
}
-export async function getMoneyTransfers(userId, before=Date.now(), limit=100) {
+export async function getMoneyTransfers(userId, beforeId?: number, limit=100) {
return db.moneyTransfer.findMany({
take: limit,
- where: { OR: [{ fromId: userId }, { toId: userId }], createdAt: { lt: new Date(before) } },
- orderBy: { createdAt: "desc" },
+ where: { OR: [{ fromId: userId }, { toId: userId }], id: { lt: beforeId } },
+ orderBy: { id: "desc" },
include: { from: true, to: true }
});
}
-export async function getUsedVouchers(usedById, before=Date.now(), limit=100) {
+export async function getUsedVouchers(usedById, beforeId: number, limit=100) {
+ const args: {skip: 1, cursor: {id: number}}|{} = beforeId ? { skip: 1, cursor: { id: beforeId } } : {}; // skip cursor if present
return db.voucher.findMany({
take: limit,
- where: { usedById, createdAt: { lt: new Date(before) } },
- orderBy: { usedAt: "desc" }
+ ...args,
+ where: { usedById },
+ orderBy: [{ id: "desc" }, { usedAt: "desc" }] // in case usedAt is the same for multiple vouchers, order by id to make sure the order is consistent
});
}
-// TODO somehow fix this
-// this skips transactions if `before` is the same as the createdAt date of the previous transaction
-// e.g. you could sort by id instead of createdAt and provide the last id of each transaction type as
-// replacement for `before`
-export async function fetchTransactions(userId, balanceAfterwards, before, limit) {
+type BeforeIds = {
+ moneyTransactions?: number,
+ itemTransactions?: number,
+ moneyTransfers?: number,
+ voucherUses?: number
+};
+
+export async function fetchTransactions(userId: number, balanceAfterwards: number, before: BeforeIds, limit: number) {
/*
For every type of transaction that changes the user's balance the limit of transactions to return
is used. This means at most we fetch 4*limit transactions from the database.
@@ -149,32 +154,36 @@ export async function fetchTransactions(userId, balanceAfterwards, before, limit
/*
[limit]+1 items are fetched because then we can check if there are more items to fetch.
*/
- const moneyTransactions = (await getMoneyTransactions(userId, before, limit+1)).map(transaction => ({
+ const moneyTransactions = (await getMoneyTransactions(userId, before.moneyTransactions, limit+1)).map(transaction => ({
type: "transaction",
name: transaction.amount > 0 ? "Aufladen" : "Auszahlung",
difference: transaction.amount,
createdAt: transaction.createdAt,
- verified: !!transaction.verifiedById
+ verified: !!transaction.verifiedById,
+ id: transaction.id
}));
- const itemTransactions = (await getItemTransactions(userId, before, limit+1)).map(transaction => ({
+ const itemTransactions = (await getItemTransactions(userId, before.itemTransactions, limit+1)).map(transaction => ({
type: "item",
name: transaction.item.name,
difference: -(transaction.price + transaction.premium),
premium: transaction.premium,
createdAt: transaction.createdAt,
image: transaction.item.image,
+ id: transaction.id
}));
- const moneyTransfers = (await getMoneyTransfers(userId, before, limit+1)).map(transfer => ({
+ const moneyTransfers = (await getMoneyTransfers(userId, before.moneyTransactions, limit+1)).map(transfer => ({
type: "transfer",
name: transfer.fromId === userId ? `Gesendet an ${transfer.to.name}` : `Empfangen von ${transfer.from.name}`,
difference: transfer.fromId === userId ? -transfer.amount : transfer.amount,
- createdAt: transfer.createdAt
+ createdAt: transfer.createdAt,
+ id: transfer.id
}));
- const voucherUses = (await getUsedVouchers(userId, before, limit+1)).map(voucher => ({
+ const voucherUses = (await getUsedVouchers(userId, before.voucherUses, limit+1)).map(voucher => ({
type: "voucher",
name: `Gutschein ${voucher.code} eingelöst`,
difference: voucher.amount,
- createdAt: voucher.usedAt // use usedAt because voucher creation time is not relevant
+ createdAt: voucher.usedAt, // use usedAt because voucher creation time is not relevant
+ id: voucher.id
}));
let transactions = [...moneyTransactions, ...itemTransactions, ...moneyTransfers, ...voucherUses]
.sort((a, b) => b.createdAt - a.createdAt);
diff --git a/src/routes/history/+page.server.js b/src/routes/history/+page.server.js
index 6a1fe6a..dc72094 100644
--- a/src/routes/history/+page.server.js
+++ b/src/routes/history/+page.server.js
@@ -3,6 +3,6 @@ import { fetchTransactions } from "../api/transactions";
export async function load(event) {
const userId = event.locals.user.id;
const balance = event.locals.user.balance;
- const {transactions, hasMore} = await fetchTransactions(userId, balance, Date.now(), 100);
+ const {transactions, hasMore} = await fetchTransactions(userId, balance, {}, 100);
return { transactions, hasMore, userId };
};
--
GitLab