Code review (Floxum): chunk backfill migration, pagination caps, GET-path write, length guards, admin/TS cleanups

[karl-bullock]

Floxum code review of the referral extension. Verified every finding below against the current source — all nine hold. None block publishing; the migration one is the only thing with real downtime risk on a big forum. Fixes for each are in the comments.

Summary: backend is architecturally sound and idiomatic for Flarum 2.x (scoped container binding, concurrency-safe getOrCreateForUser fallback, denormalised referral_count to dodge N+1 on post-author lists, proper NULL handling in the unique index). Frontend is functional but plain ES5/ES6 rather than TypeScript. Two things to weigh up: the unbatched backfill UPDATE, and a DB INSERT happening on a GET serialization path.

Medium — production risk

• Unbatched correlated-subquery UPDATE across the whole users table in migration (migrations/2026_05_27_000001_add_referral_count_to_users.php:32). The backfill runs a correlated (SELECT COUNT(*) FROM referral_invited_user WHERE …) for every users row in one statement. On a forum with tens of thousands of users this holds a write lock on users for the full count, queueing registrations/profile requests and risking timeouts during migration. Chunk it (chunk(500, …) over user IDs), or only update users that actually appear in referral_invited_user to keep the working set small.

Low — production risk

• ListCampaignCodesController fetches all campaign codes with no pagination (src/Api/Controller/ListCampaignCodesController.php:18). whereNull('user_id')->orderBy('id','desc')->get() returns every code in one query/response. Ongoing campaigns could accumulate hundreds. Add a hard ->limit(200) (plus a total meta field) or cursor/offset pagination.
• DB INSERT issued during GET serialization (getOrCreateForUser on read path) (src/Api/UserResourceFields.php:43). The referralCode getter may INSERT on every /api/users/me for an eligible user who's never opened their profile — a write inside a GET breaks HTTP semantics and a caching proxy could serve a 200 that still triggered an upstream write. Split generation from serialization: an explicit POST /api/referral/my-code that generates on demand, with the attribute returning null until a row exists; frontend calls it once when the referrals tab opens.

Low — robustness

• No max-length guard on cookie-supplied invite code before DB lookup (src/Http/CaptureReferralCookieMiddleware.php:28). The raw cookie is trimmed and stored with no length cap, then handed to InviteCode::where('code', strtoupper($code))->first(). Prepared statements stop injection, but an attacker can send an arbitrarily long cookie on every unauthenticated POST /api/users with no early rejection. Valid codes are ≤8 chars — reject strlen($code) > 16 in the middleware (set code to null).
• No max-length validation on campaign code label before insert (src/Api/Controller/CreateCampaignCodeController.php:28). Label is trimmed but unconstrained; the column is VARCHAR(255). A >255-char label throws a DB truncation/constraint error surfacing as an unhandled 500 instead of a clean 422. Add a length check throwing ValidationException(['label' => [trans('linkrobins-referral.api.label_too_long')]]).

Low — conventions

• Admin panel registered via override on ExtensionPage rather than app.extensionData (js/admin.js:16). Patching the shared ExtensionPage.prototype.content and guarding on extension.id affects all admin extension pages. Use app.extensionData.for('linkrobins-referral').registerContent(() => m(ReferralAdminPage)) with an extracted component and drop the prototype override.
• flarum-tsconfig devDependency present but source is plain JS (js/package.json:9). admin.js/forum.js are plain ES5/ES6, so the tsconfig dep is unused and misleading. Either migrate to TS (rename .ts, import from @flarum/core) or drop flarum-tsconfig from devDependencies.

Low — dead code / technical debt

• joinTime attribute registered on UserModel but never used (js/forum.js:26). UserModel.prototype.joinTime = Model.attribute('joinTime', Model.transformDate) is never read, and core exposes joinedAt (not joinTime), so it always resolves to undefined. Remove the line; use user.joinedAt() if account age is ever needed.
• Pervasive inline styles in ReferralsPage instead of LESS classes (js/forum.js:120). ReferralsPage.content() embeds layout/typography as inline style attributes (~40 lines). Inline styles can't be themed without !important and are invisible to the extension's own LESS. Extract into named classes (.ReferralProfile-code, .ReferralProfile-count) in less/forum.less, register via (new Extend\Frontend('forum'))->css(...), and swap to className.

[karl-bullock]

Notes to self — fixes for each finding.

Chunk the backfill migration

Only users who actually referred someone need a non-zero count, so scope the UPDATE to them instead of the whole table — small working set, no full-table lock:

$connection->table('users')
    ->whereIn('id', function ($q) use ($invited) {
        $q->select('referred_by_user_id')->from($invited);
    })
    ->update([
        'referral_count' => $connection->raw(
            "(SELECT COUNT(*) FROM {$invited} WHERE {$invited}.referred_by_user_id = {$users}.id)"
        ),
    ]);

referral_count already defaults to 0, so users with no referrals are correct without touching them. If I'd rather keep the full sweep, chunk it: ->orderBy('id')->chunk(500, ...) and update each batch.

Cookie invite-code length guard

In CaptureReferralCookieMiddleware::process(), reject overlong cookies before storing (valid codes are ≤8 chars):

$code = isset($cookies['referral_code']) ? trim((string) $cookies['referral_code']) : '';

if ($code !== '' && strlen($code) <= 16) {
    $this->container->make(PendingReferralState::class)->setCode($code);
}

Campaign label length validation

In CreateCampaignCodeController, after resolving $label:

if ($label !== null && strlen($label) > 255) {
    throw new ValidationException([
        'label' => [$this->translator->trans('linkrobins-referral.api.label_too_long')],
    ]);
}

Add linkrobins-referral.api.label_too_long to locale/en.yml.

Pagination cap on campaign codes list

In ListCampaignCodesController, cap and expose a total:

$query = InviteCode::whereNull('user_id');
$total = $query->count();
$codes = $query->orderBy('id', 'desc')->limit(200)->get();
// return total in the meta so the admin UI can show "showing 200 of N"

Cursor/offset pagination if it ever actually grows past a few hundred.

Move code generation off the GET path

Stop INSERTing in the referralCode getter (UserResourceFields.php:43). Return null when no row exists:

->get(fn ($user) => InviteCode::where('user_id', $user->id)->value('code'))

Add a small POST /api/referral/my-code controller that calls getOrCreateForUser($actor) and returns the code. Frontend hits it once when the referrals tab opens, then renders the returned code.

Admin panel via extensionData

Extract the admin UI into a ReferralAdminPage component and register it properly instead of patching the shared prototype in admin.js:

app.extensionData
   .for('linkrobins-referral')
   .registerContent(() => m(ReferralAdminPage));

Drop the override(ExtensionPage.prototype, 'content', ...). This pairs naturally with a TS/ESM migration of the frontend (see next).

tsconfig / TypeScript

Two options: either migrate admin.js/forum.js to .ts with @flarum/core imports (and actually use the flarum-tsconfig dep), or drop flarum-tsconfig from devDependencies so it stops implying TS. Given the other JS findings here, the TS/ESM migration is the better long-term call — same shape as the batch-2 migrations I did on the other extensions.

Remove dead joinTime

Delete js/forum.js:26 (UserModel.prototype.joinTime = ...). Core exposes joinedAt; use user.joinedAt() if account age is ever needed.

Inline styles → LESS

Pull the ~40 lines of inline style: out of ReferralsPage.content() into named classes (.ReferralProfile-code, .ReferralProfile-count, …) in a new less/forum.less, register via (new Extend\Frontend('forum'))->css('less/forum.less'), and swap the style strings for className. Do this together with the TS/ESM migration so it's one frontend pass.

Plan: backend fixes (backfill, both length guards, list cap, GET-path write) in one pass — small and self-contained. Frontend (extensionData + inline styles + joinTime + tsconfig) as a TS/ESM migration pass, batched like the other extensions.

[karl-bullock]

Progress update: the TS5 bump is done and pushed to main (54c6b3f). The substantive findings here are not done yet: chunking the backfill migration, the pagination cap on campaign codes, moving the code-generating write off the GET serialization path, the cookie and label length guards, and the admin/TS cleanups. They're still on my list and I'll update here as I work through them.

[karl-bullock]

Update: worked through all nine findings, committed and pushed to main (not released yet).

Medium

• Chunked the backfill migration (39465eb). The backfill no longer runs a correlated COUNT() across the whole users table in one locked UPDATE. It updates only the users who actually referred someone, in chunks of 500, and leaves everyone else at the column default of 0. Verified with seed data: a referrer of two gets 2, a non-referrer stays 0.

Low - production risk

• Capped the campaign-codes list (f8ad111) at 200 newest-first, with the real total in meta so the UI can flag truncation.
• Moved code generation off the GET path (8e7b13b). referralCode is now a pure read; an explicit POST /api/referral/my-code generates the code, and the referrals tab requests one on demand when an eligible user has none. Verified end to end: deleting a user's code and reloading the tab generates it via the POST, and a GET no longer writes a row.

Low - robustness

• Length guards (0a6ecf1): reject an over-long referral_code cookie before the DB lookup, and reject an over-long campaign label with a 422 instead of a database truncation 500 (verified).

Low - conventions / dead code

• Dedicated admin page (5d7312e): replaced the ExtensionPage.prototype.content override with a page registered only for this extension. One note for the record: the suggested registerContent API doesn't exist in Flarum 2.0; the equivalent is app.registry.for(id).registerPage(...), which is what I used. Verified the page renders with zero console errors.
• Dropped flarum-tsconfig/typescript/tsconfig.json since the source is plain JavaScript (build still produces an identical bundle).
• Removed the dead joinTime attribute and extracted the profile's inline styles into less/forum.less. Verified the profile renders with zero console errors.

I'll close this once the release goes out.

[karl-bullock]

All substantive findings from this review have shipped in v1.3.0: code generation moved off the GET path to a dedicated endpoint, the campaign list is capped with a truncation flag, the backfill migration is chunked, and the cookie and label length guards are in. Note the admin page uses app.registry.for(id).registerPage(), since the registerContent API suggested here does not exist in Flarum 2.0. Closing.