Repository navigation
E2E tests for admin webhooks, admin users, admin dashboard, and notifications - #759
Merged
DeFiVC merged 4 commits intoOct 26, 2026
Merged
Conversation
…hainLearnOfficial#615) webhook.routes.ts registered only adminGuard on this router, never authGuard. adminGuard reads request.authUser.isAdmin directly, and only authGuard sets authUser, so every single request to any admin webhook endpoint (authenticated or not, admin or not) threw "Cannot read properties of undefined (reading 'isAdmin')" and 500'd, instead of the clean 401/403 every other admin router (admin-users, dashboard) returns. Confirmed pre-existing via git stash against the unmodified upstream tree, and consistent with this repo's own recent CI runs on main, which have been failing. Fixed by adding the missing authGuard onRequest hook, matching the other admin routers exactly. Added tests/e2e/admin-webhooks.test.ts covering CRUD operations, secret rotation (confirms the secret is never returned from any read endpoint, changes value on each rotation, and persists to the DB), delivery-attempt listing (including the cursor-based pagination path added in ChainLearnOfficial#482), and stats computation. All 29 tests pass; 0 new failures introduced elsewhere (same 40 pre-existing repo-wide build errors and 36 pre-existing failing e2e suites before and after, confirmed via git stash comparison). The existing tests/e2e/webhooks.test.ts (a different, non-admin-scoped file, out of scope for ChainLearnOfficial#615) improves from 3/17 to 11/17 passing as a side effect, since it was hedging its assertions across multiple status codes to tolerate this exact bug.
…hainLearnOfficial#616) Adds E2E coverage for the three entrypoints ChainLearnOfficial#616 named that this file didn't yet exercise: ban (confirms the reason is persisted and the banned user's own JWT is rejected with 403 on the very next request, ChainLearnOfficial#347), credits grant/deduct (confirms the before/after balance in the response matches the DB, and that a deduction exceeding the balance is rejected with VALIDATION_ERROR and leaves the balance unchanged), and the user activity feed (confirms an enrollment surfaces as an activity entry with the right courseId). ChainLearnOfficial#616 also asks for unban coverage, but no unban/reinstate entrypoint exists anywhere in this codebase (confirmed by searching the whole repo for "unban" and for any route that clears bannedAt): the ban is one-way as shipped. Added a test that documents this gap explicitly (asserts the route 404s) rather than silently dropping that half of the issue's scope; it will start failing, as a signal to replace it with real coverage, the moment an unban route is added.
Covers the dashboard's one endpoint end to end: auth/admin rejection, the full stats shape (totals, new-user windows, quiz completion rate, credentials, rewards claimed), the week-over-week trend metrics, and caching behavior specifically, since that was named explicitly in scope. The caching tests go past "a cache key exists" to verify the actual behavior that matters: a second request within the 5-minute TTL returns the byte-identical cached response even after new data is written directly to the DB in between, and clearing the cache entry makes the very next request pick up that new data. Both are driven against the real Redis instance the app itself talks to, not a reimplementation of the cache logic.
Covers all three entrypoints mounted under /api/v1/users/me/notifications (wired into user.routes.ts, not a dedicated notifications.routes.ts): listing (ordering, per-user isolation, pagination, the unreadCount field staying correct independent of the current page), marking a single notification read (including the 404 when a user tries to mark another user's notification, confirmed to leave that row genuinely untouched in the DB, not just hidden in the response), and mark-all-read (confirms the returned count, that it only touches the authenticated user's own rows, and the 0 case when nothing is unread).
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
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.
Summary
closes #615
closes #616
closes #618
closes #619
Changes
webhook.routes.tsregistered onlyadminGuardon this router, neverauthGuard.adminGuardreadsrequest.authUser.isAdmindirectly, and onlyauthGuardsetsauthUser, so every single request to any admin webhook endpoint (authenticated or not, admin or not) crashed withCannot read properties of undefined (reading 'isAdmin')and returned 500, instead of the clean 401/403 every other admin router returns. Confirmed pre-existing viagit stashagainst the unmodified upstream tree, and consistent with this repo's own recent CI runs onmain, which have been failing. Fixed by adding the missingauthGuardhook. Addedtests/e2e/admin-webhooks.test.tscovering CRUD operations, secret rotation, delivery-attempt listing (including cursor-based pagination), and stats computation, 29 tests.admin-users.test.tswith coverage for ban, credit grant/deduct, and the activity feed. One scope note: the issue asks for "ban/unban" coverage, but there is no unban/reinstate entrypoint anywhere in this codebase, the ban is one-way as shipped. Added a test documenting that gap explicitly (asserts the route 404s) instead of silently dropping it.tests/e2e/admin-dashboard.test.tscovering the stats shape, trend metrics, and caching behavior specifically, verified against the real Redis instance (a second request within the TTL returns byte-identical cached data even after new DB writes; clearing the cache entry picks up the new data).tests/e2e/notifications.test.tscovering listing, mark-single-read, and mark-all-read, including per-user isolation and the unreadCount field.Test plan
tests/e2e/suite before/after comparison (viagit stash): baseline had 36 failed suites / 121 failed tests; this branch has 35 failed suites / 83 failed tests, fewer failures and 56 more passing tests, confirming no regressions. The remaining failures are pre-existing and out of scope for these 4 issues.npm run build: 40 TypeScript errors before and after this branch (identical count viagit stashcomparison), confirming this repo's build is broken repo-wide independent of this PR, and this PR doesn't add to it.npx eslinton all touched files: clean.tests/e2e/webhooks.test.tsimproves from 3/17 to 11/17 passing, since it had been hedging its assertions across multiple status codes to tolerate the Add E2E tests for admin webhook management #615 bug.