-
Notifications
You must be signed in to change notification settings - Fork 1
Fix BUG-666: Add transaction type filter with pagination #1
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
base: main
Are you sure you want to change the base?
Conversation
Implemented filtering by transaction type (credit/debit) on the frontend. Added pagination controls to navigate through filtered results. Fetching all transactions to enable client-side filtering for better UX.
| setLoading(true); | ||
| const backendUrl = import.meta.env.VITE_BACKEND_URL; | ||
| const response = await fetch(`${backendUrl}/api/v1/transactions/`); | ||
| const response = await fetch('http://localhost:8000/api/v1/transactions/?limit=1000'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Hardcoded localhost:8000 can break in different environments. We should use an environment variable or config.
- I think limit=1000 fetches too many records upfront. As it is a client-side pagination, this will be slow and can waste bandwidth. I think we should consider server-side pagination or a more reasonable limit.
| setTransactions(data); | ||
| } catch (e: unknown) { | ||
| setError(e instanceof Error ? e.message : 'An error occurred'); | ||
| } catch (e: any) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should avoid any. We can use unknown and narrow the error as original.
| const [error, setError] = useState<string | null>(null); | ||
| const [filterType, setFilterType] = useState<string>('all'); | ||
| const [currentPage, setCurrentPage] = useState<number>(1); | ||
| const pageSize = 10; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about making this configurable?
| const filteredTransactions = transactions.filter(transaction => { | ||
| if (filterType === 'all') return true; | ||
| return transaction.type === filterType; | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This runs every render. We can use useMemo here.
| const totalPages = Math.ceil(filteredTransactions.length / pageSize); | ||
| const startIndex = (currentPage - 1) * pageSize; | ||
| const endIndex = startIndex + pageSize; | ||
| const paginatedTransactions = filteredTransactions.slice(startIndex, endIndex); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use useMemo here as well.
| <select | ||
| id="type-filter" | ||
| value={filterType} | ||
| onChange={handleFilterChange} | ||
| className="border border-gray-300 rounded-md px-3 py-1 text-sm focus:outline-none focus:ring-2 focus:ring-blue-500" | ||
| > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can add aria-label for screen readers.
What I Did
This PR fixes the transaction display bug by adding a filter dropdown for transaction types (All/Credit/Debit) and implements client-side pagination for the transaction list. The filtering and pagination are handled entirely in the frontend React component.
How to Test
docker compose up --build.http://localhost:3000.Architectural Decision Record (ADR)
To address the display bug, I decided to handle filtering directly in the frontend component. This approach simplifies the backend and leverages the client's processing power for immediate filtering without additional API calls. For pagination, I implemented client-side pagination by slicing the filtered transaction array, which provides a responsive user experience without requiring backend changes. The frontend fetches up to 1000 transactions at once to support this client-side approach.
AI Usage Summary
I used AI tools for minor code refactoring and to generate some boilerplate for the frontend pagination component.