Skip to content

MXWAR-81: Fix pending clients filter and pagination reset#109

Open
adity-a34 wants to merge 1 commit into
openMF:devfrom
adity-a34:fix-pagination
Open

MXWAR-81: Fix pending clients filter and pagination reset#109
adity-a34 wants to merge 1 commit into
openMF:devfrom
adity-a34:fix-pagination

Conversation

@adity-a34
Copy link
Copy Markdown

@adity-a34 adity-a34 commented Apr 24, 2026

Description

The "Show Pending Clients" checkbox was silently broken: it filtered only the rows already fetched for the current page rather than querying the server, so toggling it on page 2 or beyond would show zero results even when pending clients existed. The pagination counter was also wrong because it still reflected the unfiltered server total.

The fix moves the status filter to the server by switching to ClientApi.retrieveAll21, which accepts a status query parameter. When the checkbox is off the API receives status: 'active'; when it's on the parameter is omitted and the server returns all statuses. The client-side rows.filter() block is removed since it is no longer needed, and the page is reset to 1 whenever the filter, search term, or page size changes so the counter always stays accurate.

Related issues and discussion

#MXWAR-81

Screenshots, if any

Before: bugged, only 17 out of 22 entries visible show pending ineffective

PaginationBug_Total_clients_22_but_shows_only_17.mp4

/////////////////

After: fixed pagination correctly reflects number of clients (22)

PaginationBug_Total_clients_22_now_shows_22_fixed_.mp4

Checklist

Please make sure these boxes are checked before submitting your pull request - thanks!

  • If you have multiple commits please combine them into one commit by squashing them.

  • Read and understood the contribution guidelines at CONTRIBUTING.md.

Summary by CodeRabbit

  • Bug Fixes

    • Improved client search and filtering behavior to ensure accurate results and proper pagination
    • Fixed active/pending status filter to correctly update results when toggled
  • Refactor

    • Optimized client data retrieval and filtering performance

Fixes: MXWAR-81
Signed-off-by: Aditya Tiwari <adityatiwari342005@gmail.com>
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 24, 2026

📝 Walkthrough

Walkthrough

The Clients page is refactored to fetch data from a different API endpoint (ClientApi.retrieveAll21 instead of ClientSearchV2Api.searchByText). This involves updating fetch parameters, response field mapping, removing client-side status filtering, and adjusting React effect dependencies.

Changes

Cohort / File(s) Summary
API Data Source Migration
src/pages/clients/Clients.tsx
Swapped API endpoint from searchByText to retrieveAll21, updated parameter mapping (search text, status selector, offset/limit pagination), changed response field handling (content/totalElementspageItems/totalFilteredRecords), removed client-side status filtering, and adjusted effect dependencies to trigger refetch on includePending toggle.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hops of joy for the API rebirth,
ClientApi now claims its rightful berth,
Parameters dance in pagination's grace,
Filtering moves to its proper place,
One file changed, dependencies aligned—
A cleaner fetch pattern, so refined!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'MXWAR-81: Fix pending clients filter and pagination reset' directly summarizes the main changes: fixing the pending clients filter logic and resetting pagination, which are the core issues addressed in this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/pages/clients/Clients.tsx`:
- Around line 83-86: Replace the explicit any cast by using the generated
OpenAPI response type: treat res.data as GetClientsResponse (the type returned
by retrieveAll21) and derive content and totalElements from that typed object;
update the declarations using GetClientsResponse for data, use (data.pageItems
?? []) mapped/cast to ClientRow[] for content, and derive totalElements from
data.totalFilteredRecords ?? content.length so ESLint no longer flags
`@typescript-eslint/no-explicit-any` and the code uses the correct generated types
(referencing retrieveAll21, GetClientsResponse, data, content, ClientRow, and
totalElements).
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 1da7106d-19f7-4f3c-8220-09763a63fb4d

📥 Commits

Reviewing files that changed from the base of the PR and between 7117d04 and 8af58f6.

📒 Files selected for processing (1)
  • src/pages/clients/Clients.tsx

Comment on lines +83 to +86
const data = res.data as any
const content = (data.pageItems ?? []) as unknown as ClientRow[]
const totalElements: number =
data.totalFilteredRecords ?? content.length
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Confirm the generated return type of retrieveAll21 exposes pageItems / totalFilteredRecords
fd -t f -e ts -e d.ts . -x rg -nP -l 'retrieveAll21' {} \; | head -20
rg -nP -C5 'retrieveAll21\s*\(' --iglob '*.d.ts' --iglob '*api*.ts' | head -80
rg -nP -C3 'totalFilteredRecords|pageItems' --iglob '*client*' | head -80

Repository: openMF/mifos-x-web-app-react

Length of output: 4376


🏁 Script executed:

# Find the retrieveAll21 function definition and its return type
fd -type f -name "*.ts" -o -name "*.d.ts" | xargs rg -l "retrieveAll21" | head -10

Repository: openMF/mifos-x-web-app-react

Length of output: 431


🏁 Script executed:

# Look for the Fineract API client definition
fd -type f \( -name "*api*.ts" -o -name "*client*api*" \) | head -20

Repository: openMF/mifos-x-web-app-react

Length of output: 243


🏁 Script executed:

# Check the referenced example files
cat -n src/pages/accounting/create-journal-entry/SearchJournalEntry.tsx | sed -n '92,99p'

Repository: openMF/mifos-x-web-app-react

Length of output: 505


🏁 Script executed:

# Check Navigation.tsx example
cat -n src/pages/navigation/Navigation.tsx | sed -n '71,77p'

Repository: openMF/mifos-x-web-app-react

Length of output: 446


🏁 Script executed:

# Find where the API clients are defined
fd -name "*.ts" -o -name "*.d.ts" | xargs rg -l "retrieveAll21" -B 5 -A 5

Repository: openMF/mifos-x-web-app-react

Length of output: 491


🏁 Script executed:

# Search for ClientRow type definition
rg "type ClientRow|interface ClientRow" -A 5

Repository: openMF/mifos-x-web-app-react

Length of output: 380


🏁 Script executed:

# Look for API client file
fd -name "*api*.ts" -path "*/api/*" | head -20

Repository: openMF/mifos-x-web-app-react

Length of output: 303


🏁 Script executed:

# Find Fineract API or generated types
fd -type d -name "api" -o -name "types" -o -name "generated"

Repository: openMF/mifos-x-web-app-react

Length of output: 243


🏁 Script executed:

# Find where retrieveAll21 is defined/imported
rg "retrieveAll21" -B 3 -A 3 src/pages/clients/Clients.tsx

Repository: openMF/mifos-x-web-app-react

Length of output: 233


🏁 Script executed:

# Look for API client imports in Clients.tsx
head -50 src/pages/clients/Clients.tsx

Repository: openMF/mifos-x-web-app-react

Length of output: 1488


🏁 Script executed:

# Find API client files
find . -path ./node_modules -prune -o -name "*client*" -type f | grep -i api | head -20

Repository: openMF/mifos-x-web-app-react

Length of output: 54


🏁 Script executed:

# Find fineract-api module
find . -path ./node_modules -prune -o -type d -name "fineract-api" -print

Repository: openMF/mifos-x-web-app-react

Length of output: 54


🏁 Script executed:

# Search for ClientApi class definition
rg "class ClientApi|export.*ClientApi" -A 10 | head -50

Repository: openMF/mifos-x-web-app-react

Length of output: 54


🏁 Script executed:

# Look in src directory structure
ls -la src/

Repository: openMF/mifos-x-web-app-react

Length of output: 993


🏁 Script executed:

# Check tsconfig for path aliases
cat tsconfig.json | grep -A 10 "paths"

Repository: openMF/mifos-x-web-app-react

Length of output: 124


🏁 Script executed:

# Search for fineract-api in vite config
cat vite.config.ts 2>/dev/null || cat vite.config.js 2>/dev/null

Repository: openMF/mifos-x-web-app-react

Length of output: 1197


🏁 Script executed:

# Look for any typescript configuration
find . -maxdepth 2 -name "tsconfig.json" -o -name "vite.config.*" -o -name "vitest.config.*" | xargs cat 2>/dev/null | head -100

Repository: openMF/mifos-x-web-app-react

Length of output: 1496


🏁 Script executed:

# Search for ClientApi definition directly
rg "class ClientApi" -A 20

Repository: openMF/mifos-x-web-app-react

Length of output: 54


🏁 Script executed:

# Look for fineract-api in src
ls -la src/fineract-api/ 2>/dev/null | head -30

Repository: openMF/mifos-x-web-app-react

Length of output: 54


🏁 Script executed:

# Search for retrieveAll21 method signature/type
rg "retrieveAll21" -A 5 src/ 2>/dev/null | head -100

Repository: openMF/mifos-x-web-app-react

Length of output: 1624


🏁 Script executed:

# Look for package.json to understand dependencies
cat package.json | head -50

Repository: openMF/mifos-x-web-app-react

Length of output: 2032


🏁 Script executed:

# Find fineract.yaml
find . -name "fineract.yaml" -o -name "fineract.yml"

Repository: openMF/mifos-x-web-app-react

Length of output: 88


🏁 Script executed:

# Look for any OpenAPI spec files
find . -maxdepth 2 -name "*.yaml" -o -name "*.yml" | grep -v node_modules

Repository: openMF/mifos-x-web-app-react

Length of output: 143


🏁 Script executed:

# Check if src/fineract-api is in gitignore
cat .gitignore | grep -i fineract

Repository: openMF/mifos-x-web-app-react

Length of output: 90


🏁 Script executed:

# Search for retrieveAll21 in fineract.yaml
rg "retrieveAll21" fineract.yaml -A 30 | head -100

Repository: openMF/mifos-x-web-app-react

Length of output: 54


🏁 Script executed:

# Look for the endpoint path and response schema
rg "retrieveAll21|pageItems" fineract.yaml -B 5 -A 15 | head -150

Repository: openMF/mifos-x-web-app-react

Length of output: 4015


🏁 Script executed:

# Find GetClientsPageItemsResponse schema definition
rg "GetClientsPageItemsResponse:" fineract.yaml -A 30 | head -80

Repository: openMF/mifos-x-web-app-react

Length of output: 830


🏁 Script executed:

# Also check GetClientsPageItems to understand the difference
rg "GetClientsPageItems:" fineract.yaml -A 30 | head -80

Repository: openMF/mifos-x-web-app-react

Length of output: 837


🏁 Script executed:

# Look at the full context around line 83-86 to see how the data is being transformed
cat -n src/pages/clients/Clients.tsx | sed -n '75,95p'

Repository: openMF/mifos-x-web-app-react

Length of output: 868


Remove as any and use the generated Fineract API response type.

ESLint flags @typescript-eslint/no-explicit-any here. The retrieveAll21 method returns a typed GetClientsResponse (generated from the OpenAPI spec) with pageItems and totalFilteredRecords properties, so the as any cast is unnecessary. Use the generated response type directly instead of a custom inline type.

♻️ Suggested refactor
-        const data = res.data as any
-        const content = (data.pageItems ?? []) as unknown as ClientRow[]
-        const totalElements: number =
-          data.totalFilteredRecords ?? content.length
+        const { pageItems = [], totalFilteredRecords = 0 } = res.data as GetClientsResponse
+        const content: ClientRow[] = pageItems as ClientRow[]
+        const totalElements: number = totalFilteredRecords
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const data = res.data as any
const content = (data.pageItems ?? []) as unknown as ClientRow[]
const totalElements: number =
data.totalFilteredRecords ?? content.length
const { pageItems = [], totalFilteredRecords = 0 } = res.data as GetClientsResponse
const content: ClientRow[] = pageItems as ClientRow[]
const totalElements: number = totalFilteredRecords
🧰 Tools
🪛 ESLint

[error] 83-83: Unexpected any. Specify a different type.

(@typescript-eslint/no-explicit-any)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/pages/clients/Clients.tsx` around lines 83 - 86, Replace the explicit any
cast by using the generated OpenAPI response type: treat res.data as
GetClientsResponse (the type returned by retrieveAll21) and derive content and
totalElements from that typed object; update the declarations using
GetClientsResponse for data, use (data.pageItems ?? []) mapped/cast to
ClientRow[] for content, and derive totalElements from data.totalFilteredRecords
?? content.length so ESLint no longer flags `@typescript-eslint/no-explicit-any`
and the code uses the correct generated types (referencing retrieveAll21,
GetClientsResponse, data, content, ClientRow, and totalElements).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant