Fix list pagination and loading state - #165
Merged
Aman-Mittal merged 5 commits intoJul 31, 2026
Merged
Conversation
The data table read its raw `pageIndex` @input in server-side mode, but the list components fetch by offset and never bind it back, so it stayed 0. Three symptoms followed from that one fact: the range label was always "1 - 10", "previous" was always disabled, and — because goTo() computes from pageIndex() — "next" resolved to page 1 every time. Page 2 was reachable and nothing beyond it. Track page state in the component in both modes. ngOnChanges still syncs from the @input, so a parent that does drive pageIndex keeps control. Search and sort now reset to the first page in both modes too, matching what the server-side parents already do with their offset. That alone fixes next/previous/first/last everywhere. Filters are projected content, though, so the table cannot see them: changing a filter refetched from offset 0 while the label still read "51 - 54 of 54". The list components now expose a pageIndex signal, kept in step with their currentPage, and bind it — so a filter reset moves the paginator with the rows. Verified against a local Fineract with 54 clients: 1-10 → 11-20 → 21-30, back to 11-20, last page 51-54, and a filter change returns to 1-10.
Two faults compounded. GET /userdetails 404s on the current Fineract, and
`isLoading` was a plain field written from the subscribe callback — mutating
one there never marks the view dirty, so the error handler ran but the
spinner kept going. The page showed a heading and nothing else, with no clue
that anything had failed.
Read from GET /users/{id} instead, which this Fineract does serve and which
carries the same fields plus the user's selected roles. The id comes from the
session established at login rather than asking the server who we are.
Make isLoading a signal, add a loadError signal, and render an explicit error
when the request fails, so a failure is visible rather than indistinguishable
from a slow load. displayName and email now come from real response fields
instead of untyped index lookups.
Verified against a local Fineract: username, display name, office, email and
roles all render.
The Profile page was the reported symptom, but the same shape appears in 23 other components: a plain `isLoading`/`loading` field written from a subscribe callback and read by the template. Mutating a plain field there never marks the view dirty, so whatever the handler decides — success or failure — the spinner keeps running. Convert them to signals. Also clear the compiler warnings this surfaced: - NG8107 in loan-form and business-dates: both `?.` sit inside an @if that has already narrowed the value, so the operator is dead weight. - NG8113 unused imports. Chasing these found a second component named ConfirmDialogComponent declared inline in holidays-list.component.ts, distinct from the shared one, carrying six imports its template never uses. Trimmed to what it renders; the file's other component still needs the import statements. Also dropped an unused IonInput/IonLabel from three more. theme.service.spec was failing before this branch and would fail for anyone on a dark-mode machine: with no saved theme the service honours the OS preference via matchMedia, which the spec never stubbed. Stub it, and add the prefers-dark case that was silently untested. Unit tests: 639 passing, up from 635 passing with 3 failures.
Aman-Mittal
changed the title
codeql-action init/analyze move to v4.37.1. Both were pinned to a SHA commented only as "# v4"; the comment now names the exact release. eslint-plugin-sonarjs 4.0.1 -> 4.2.0 brings 119 new findings: - 114 sonarjs/prefer-specific-assertions — expect(x.length).toBe(n) rewritten to expect(x).toHaveSize(n), which reports the actual contents on failure rather than just a number mismatch. - 3 sonarjs/assertions-in-tests in dialog.service.spec — false positives. The tests do assert, via expectAsync().toBeResolvedTo(), which the rule does not recognise. Disabled for that describe with the reason recorded rather than restructuring working tests to satisfy the check. - 2 sonarjs/no-hardcoded-passwords in login.component.spec — form fixtures, not credentials. Disabled per line with a justification. Unit tests: 639 passing. @ngx-translate is deliberately left at v17. Dependabot apache#131 bumps only http-loader, but 18.x requires core >= 18, and v18 is a much larger change than the PR implies: TranslateModule is gone in favour of TranslatePipe plus provideTranslateService, which is ~400 files. I tried it, and it fails for a reason worth recording before anyone retries: provideTranslateService resolves the loader eagerly, so HttpClient is constructed during bootstrap, runs errorInterceptor, which injects NotificationService, which injects TranslateService — NG0200 circular dependency, and no translations ever load. Untangling that needs its own change, not a version bump.
Unit tests for the data table's server-side page state, and a first spec for UserProfileComponent, which had none. The pagination tests are real guards, not decoration: reverting effectivePageIndex to the version that read the raw @input makes two of them fail, and restoring the fix makes them pass again. They cover advancing without the parent binding pageIndex back, a parent still being able to drive it, and the reset on search and on sort. The profile tests assert what the bug actually broke — that a failed request settles isLoading and renders an error, rather than leaving a spinner running — plus the happy path and the no-session-user case. e2e/list-pagination.spec.ts covers the same ground through the UI against page.route() mocks, so it needs no backend: paging forward twice (the second step was the no-op), back, jumping to the last page, and the label agreeing with the rows after a filter or search resets to page 1. It also covers the profile page loading and its error state. expectPagerDisabled asserts the reflected JS property: ion-button is a custom element, so Playwright's toBeDisabled() does not apply to it. Unit tests: 647 passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.