If you use mod_jitsi and you’ve noticed it hasn’t shipped anything new lately, it’s not that I’ve shelved it — quite the opposite. These days I’m deep in the biggest technical-debt cleanup I’ve ever done on the plugin — the one that brings Jitsi video calls into Moodle — and that’s eating all the time that would normally go to new features. It’s all still on the dev branch, unpublished: work in progress.

And something has already happened that sums up why it pays to do this tests-first. I wrote a test for a trivial function — a button that toggles the camera on and off — and the test found a bug that had been lurking there for who knows how long: the function tried to read a property of an object that never actually existed. It just barely doesn’t blow up in production. And it was only the first of six or seven surprises. Here’s how it’s going.

Two files nobody wants to touch #

Every plugin that grows for years accumulates its dark corners. Mine are two:

• classes/external.php: 2,698 lines, a single class with 126 methods (41 AJAX endpoints, each with its three old-style Moodle methods). Adding an endpoint means editing a 2,700-line file and praying you don’t clash on the next merge.
• lib.php: 3,216 lines, 47 global functions where Moodle callbacks, helpers, database logic, YouTube/Google integration, push notifications… and a 924-line function that builds the call’s HTML and JS out of 526 echo statements all live together.

The ironic part is that the rest of the plugin already uses modern namespaces. Only these two are still stuck in 2015.

The plan: tests first, in small batches #

The temptation with a refactor like this is to do it “all in one go”. Bad idea. I’m doing it in small batches and tests-first: write the test against the current code → watch it pass → move the code → back to green → phpcs, bump the version, run the whole suite → commit. I’ve repeated the cycle a couple of dozen times.

That discipline, which feels slow, is exactly what keeps surfacing the bugs.

From a 2,700-line file to 40 small files #

I’m splitting external.php into one class per endpoint, following Moodle’s modern standard (core_external\external_api, autoloading, no old classpath). Each web service’s public name doesn’t change, so the JavaScript and the mobile app keep working without touching anything.

Doesn’t deleting external.php entirely leave a gap? No. In modern Moodle that file plays no role: it isn’t a reserved name — unlike lib.php or db/services.php — it was just where, by tradition, the class holding all the functions lived. Since Moodle 4.2, each function lives in its own autoloaded class under classes/external/ and is declared in db/services.php. The old mechanism (a giant class loaded by hand via classpath) is even flagged for deprecation from Moodle 4.6. So the monolith wasn’t just redundant: it was on its way to obsolete.

And lib.php is already down from 3,216 to 1,086 lines (−66%): its helpers are being spread across classes with clear responsibilities (room, attendance, recording, google, youtube, notification…). The giant external.php has gone all the way to gone (2,698 lines → 0).

The bugs I didn’t know I had #

Here’s the interesting part: I’m not hunting bugs, I’m tidying up. But writing a test for each function before moving it, they keep falling out:

1// The camera button, BEFORE (broken): $jitsi is an int, not an object,
2// and $jitsiob never gets defined
3$event->add_record_snapshot('course', $jitsi->course);
4$event->add_record_snapshot('jitsi', $jitsiob);
5
6// AFTER:
7$jitsiob = $DB->get_record('jitsi', ['id' => $jitsi]);
8$course  = $DB->get_record('course', ['id' => $jitsiob->course]);

So far, six or seven latent bugs. Many are harmless in production (that Moodle check only runs in debug mode), but they’re wrong. And one is genuinely scary: a dead function called get_participants that, had it run, did $jitsiob->name = 'modificado' and saved the record… that is, it would rename the activity to “modificado”. Nobody calls it, but there it sits, like a landmine.

The near-miss: when your own script lies to you #

This is my favourite of the week. After moving the getminutes() function to its new class, I wanted to check there were no old calls left. I used a “clever” grep:

1grep "getminutes(" -r . | grep -v "::"   # ignore the already-migrated ones (attendance::minutes)

The idea: drop the lines with :: because they were already migrated. But there was a forgotten old call in view.php:

1html_writer::tag('div', getminutes($id, $USER->id), ...)

See the problem? That line contains :: (from html_writer::tag), so my “smart” grep hid it. The 102 unit tests didn’t catch it either, because they don’t load view.php. A simple browser smoke test found it: the page blew up with Call to undefined function getminutes().

A double moral, cheap to learn: don’t filter your verification with a grep -v that’s too clever, and unit tests don’t load your pages — you have to open the real view at least once.

(Bonus for detail nerds: macOS sed doesn’t understand \b, so a replacement fails silently and leaves classes without their namespace prefix. Another one you only catch if you look.)

Moving isn’t improving #

A confession about what I’ve decided so far. That 924-line function that builds the video call I’m moving as-is to a new class, without touching its 526 echos. I considered rewriting it with Mustache templates and AMD modules… and I’ve decided not to do it now: it’s high-risk (you have to test real video calls) and the goal of this phase is to empty lib.php, not redesign the player. That’s for later.

Worth being honest: pulling a monster out of one file and into another reduces the noise, but doesn’t fix the monster. It’s debt relocated, not paid off. That said, before moving it I shielded it with tests for the four server types, including verifying the JWT token’s signature (the most critical part of the plugin, and previously without a single test).

Where I am now #

This is a snapshot of work in progress, all on dev:

• lib.php: −66% so far (3,216 → 1,086 lines). external.php: deleted (2,698 → 0).
• 64 → 105 tests (+41), with JWT generation finally covered.
• 6-7 latent bugs fixed along the way and three dead functions (one dangerous) gone.
• Full compatibility: no external client notices the change.

And what’s still ahead: actually rewriting that 924-line function (Mustache + AMD), keeping coupling down, and — once everything is green and tested on real calls — moving it from dev to master.

If there’s something I’m taking from this so far: refactoring tests-first doesn’t just protect you, it finds bugs you didn’t know you had; distrust your own verification shortcuts; and be honest about which debt you’re paying off and which you’re just moving around.

Continues in part two: I keep the “moving isn’t improving” promise and actually take that 924-line function apart into data, templates and modules.