refactor: dedupe event-payload & request-guard helpers; fail lint on warnings - #164
Merged
dev-fani merged 1 commit intoAug 30, 2026
Conversation
…n warnings Consolidates three small, independently-tracked duplication/tooling gaps. closes fanilabs#133 closes fanilabs#131 closes fanilabs#132
|
@willy-de7 Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits. You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀 |
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.
Resolves three small, independently-tracked tech-debt items in one pass. Each is a mechanical change with no runtime behaviour change (except CI strictness in #133).
closes #133 —
pnpm lintnever fails on ESLint warnings"lint": "eslint ."exits0for a run that produces only warnings.eslint.config.jshas exactly onewarn-severity rule (no-console), so a reintroducedconsole.loginsrc/would pass bothpnpm lintand CI'slint-and-typecheckjob silently.package.json→"lint": "eslint . --max-warnings=0", matching the all-or-nothing enforcement offormat:check/typecheck/test.Acceptance
pnpm lintnow exits non-zero on any warning.closes #131 — blockchain-event payload parsers copy-pasted across ~7 files
parseAddresswas defined verbatim in the application-layer event handlers of deliveries, disputes, escrow, fleet, fraud-detection, notifications, reputation (7 copies). A parallelBigInt-id parser was defined independently in deliveries (parseDeliveryId), escrow (parseDeliveryId), fleet (parseFleetId) and disputes (parseBareDeliveryId).src/shared/events/parse.tsexportingparseAddressand a single generically-namedparseBigIntId, re-exported from the existingsrc/shared/eventsbarrel (already an allowed dependency for theapplicationlayer pereslint.config.js). Lives next toBlockchainEventEnvelopebecause this is event-payload-shape parsing, not module business logic.parseAddressdefinitions replaced with the shared import.parseDeliveryId/parseFleetId/parseBareDeliveryIdcall sites now useparseBigIntId; the local definitions are removed.parseTupleWrappedDeliveryIdas a thin, contract-specific wrapper — it now JSON-unwraps and delegates toparseBigIntId.parseId(its read model keys by the decimal string form) as a two-line wrapper overparseBigIntId.parseAmountis left local (amounts, not ids — out of scope).Acceptance
parseAddressdefined in exactly one place.BigInt-parsing logic defined in exactly one place; module-specific wrapping (disputes tuple-unwrap, notifications string form) kept local.closes #132 —
requireUser/requireUserIdrequest-guard copy-pasted across 4 route filesadmin,disputes,notificationsanduserseach defined their own near-identical helper (same "unreachable in practice" comment) to narrowrequest.userafter anauthenticatepreHandler.requireUser(request): { id: string; role: UserRole }tosrc/shared/http/plugins/auth-guard.ts— next toauthenticate/requireRole, and to therequest.usertype augmentation it already declares — and re-exported it fromsrc/shared/http/index.js.requireUserId(request)call sites becomerequireUser(request).id.UnauthorizedError(and, in disputes,UserRole) imports from the four route files.Acceptance
authenticate/requireRole.Verification
Pure extraction / mechanical change — no behaviour change. Verified locally that this branch adds zero new
pnpm lint,pnpm typecheck, orvitestfailures relative tomain(the local dev environment has some pre-existing dependency-resolution drift; the diff introduces none of it, and touches no failing file).