In part one I left you at the worst possible moment: with a lesson half-kept. The moral of that post was “moving isn’t improving”, and I illustrated it with my worst piece of code —a nearly 924-line function that builds the video call— which I did exactly that to: I moved it without touching the inside. It was still there, intact, spitting out HTML and some 600 lines of hand-concatenated JavaScript. Fixing it “later” was the promise.

Well, welcome to “later”. This is the batch where the monster actually gets taken apart. It’s all still on the dev branch, under test, but I can already tell you how it went. And, as always, the scares that showed up on the way.

The plan: split the monster in three #

A function that mixes what data to show, how it looks and what it does is impossible to maintain (and to test). So the idea was to split it into three clean layers:

• Data: pure, testable PHP functions that build what needs to be shown.
• Presentation: the HTML, out of the PHP, in templates (Mustache, Moodle’s system).
• Behaviour: the JavaScript, out of the PHP, in modern modules (AMD, ES6, with its linter).

And while at it, finish off lib.php, which I’d left half-emptied in post 1.

lib.php: from 1,086 to 363 lines #

I started with the easy finish. The last helpers left in lib.php went out to their classes, the chart renderers (a heatmap, a segments bar) moved to Mustache templates, and the “private” version of the video-call function —which in post 1 I hadn’t even moved— finally went to its place.

Result: lib.php went from 1,086 to 363 lines. And the nice part is what those 363 are: now just standard Moodle callbacks. No business logic, no HTML, no JavaScript. The junk drawer stopped being a junk drawer.

Taking the function apart: from generating behaviour to generating data #

The real change shows in any button —the go-live one—. Before, its logic was “printed” as text from PHP, escaping quotes and stitching values by hand:

1// BEFORE: JavaScript built out of echo inside the PHP
2echo "function handleStreamBtn() {\n";
3echo "  btn.disabled = true;\n";
4echo "  require(['jquery','core/ajax'], function($, ajax) {\n";   // ← the 'require' fuse
5echo "    ajax.call([{ methodname: 'mod_jitsi_press_record_button',\n";
6echo "      args: {jitsi:'" . $jitsi->id . "', user:'" . $USER->id . "'} }]);\n";  // ← PHP data stitched in
7echo "  });\n";

Escaped quotes, translatable text shoved in with addslashes(get_string(...)), PHP IDs interpolated in the middle of the JavaScript, a dead variable nobody saw, and an inline require (the fuse for the ReferenceError I’ll get to). Impossible to test, impossible to run through a linter.

After, that button lives in a real ES6 module, and the PHP just hands it data:

1// AFTER: amd/src/session_recording.js (ES6, with linter and real imports)
2const handleStreamBtn = () => {
3    const btn = document.getElementById('streamBtn');
4    if (!btn) { return; }
5    btn.disabled = true;
6    fire('mod_jitsi_press_record_button', {jitsi: config.jitsiid, user: config.userid});
7    btn.classList.contains('btn-warning') ? stopStream() : stream();
8};
9streamBtn.addEventListener('click', handleStreamBtn);   // before: onclick="..." in the HTML

And the PHP side, which was 600 lines of echo, comes down to a call that hands over data, not behaviour:

1$PAGE->requires->js_call_amd('mod_jitsi/session_recording', 'init', [[
2    'jitsiid' => (int) $jitsi->id, 'userid' => (int) $USER->id, 'cmid' => (int) $cmid,
3]]);

And this isn’t cosmetic: the PHP stops manufacturing behaviour and starts delivering data that the module consumes. The onclick disappears (addEventListener replaces it), translations come out through the language system instead of being embedded, and the JavaScript sits under the same linter as any Moodle module. Which is, exactly, the contract that Moodle’s reactive UI (the one heading toward React) expects: data via service, behaviour in modules.

The war story: “require doesn’t exist yet” #

Here’s the most instructive scare of this batch, with a moral I wasn’t expecting.

When I pulled out the first JavaScript module, I loaded it the “obvious” way: a require([...]) at the top of the script. It blew up in production with a cryptic ReferenceError: Can't find variable: require. How? That JavaScript had worked for years…

The key: the original code never called require directly. It always did it inside timers and event handlers, which run later —when Moodle’s module loader is already up—. My new require, instead, ran immediately, before that loader existed. The idiomatic fix: expose the live object on window and let Moodle load the module in the footer, with everything ready.

The lesson, which titles one of today’s: pulling JavaScript out of PHP into a module isn’t moving code, it’s changing when it runs. Inline code runs at once; a module runs when the system resolves it. That timing gap is exactly where you crash. And as a bonus, a second chained bug: since the “hang up the call” redirect now lived in that module that wasn’t loading, hanging up left you trapped on the video-call screen. If you move the exit into a module and the module doesn’t load, you’re left with no way out.

What a refactor uncovers #

Rewriting by hand, line by line, is the best auditing tool I know. Translating the recording JavaScript I found ~80 lines that never ran: a function without a single call and blocks checking a button that no longer exists in any template. You only see it if you force yourself to read every line.

And two traps I dodged by a hair:

• Almost identical code that should NOT be merged. The security token for private rooms looks like a carbon copy of the normal one… except it forces recording to off. “Deduplicating because it’s the same” would have been a security hole: enabling recording on private sessions. Intentional duplication isn’t touched; it gets a comment.
• Another loose end from post 1. While validating, opening the admin page to delete recordings threw a Call to undefined function isDeletable(). An orphaned call site from the previous refactor, hidden in a rarely visited page. Cousin of the misleading grep from part one: the corners that neither your tests nor you visit are where these mines hide.

This batch’s lessons #

1. Pulling inline JS into a module isn’t moving, it’s changing the execution model. Copy-paste isn’t enough: you have to expose the live state and let the framework load the module at its time.
2. When your CI can’t run the feature, the test runner is you. Automated tests don’t join a video call. So I sliced the work into tiny steps (presence, buttons, password, recording) and validated each one in a real call —join, record, hang up, watch the video— before committing. The size of the step isn’t decided by what fits in a commit, but by what you can verify at once.
3. Not everything that repeats gets deduplicated. Almost-identical code that diverges on purpose is a trap disguised as an opportunity.

Where I am and what’s left #

With this, the refactor of the external.php / lib.php / video-call-function trio is done (all on dev, validated in real calls, not yet taken to master). In this second batch’s metrics: 14 commits, lib.php down to 363 lines, from 105 to 130 tests, and around 570 lines of JavaScript pulled out of the PHP into linted modules.

Next target? Already on my list: view.php, another ~970 lines with the same vice (embedded JavaScript, hand-built SQL, everything mixed). Same medicine, another batch.

And closing the loop from post 1: all of this —separating data from presentation and from behaviour, having the JavaScript consume services instead of being stitched in from PHP— is exactly the foundation that Moodle’s new UI, the one heading toward React, needs. So yes: moving wasn’t improving. That’s why this time, instead of moving, I took it apart.

Continues in part three: I refactor the main view and, underneath, a query scanning the whole log table shows up.