maths / moodle-qtype_stack

Stack question type for Moodle
GNU General Public License v3.0
138 stars 147 forks source link

JSXG with 4.5 #1086

Closed Bjoern-Ge closed 5 months ago

Bjoern-Ge commented 7 months ago

After the update off moodle (4.1.6->4.1.7) and updating stack to 4.6.0 (2023121101) all JSXG inside stack questions stopped working. JSXG in moodle text fields outside of stack is still working.

I do get the error (chrome): first.js:4728 Uncaught TypeError: Cannot read properties of null (reading 'addEventListener') at first.js:4728:243 at _exports.init (first.js:4728:1258) at attempt.php?attempt=23650&cmid=1353:623:101 at Object.execCb (require.min.js:5:12861) at b.check (require.min.js:5:6615) at b.enable (require.min.js:5:9363) at b.init (require.min.js:5:5721) at require.min.js:5:11235

The JSXG in the question looks like this: [[jsxgraph]] var board = JXG.JSXGraph.initBoard(BOARDID, { boundingbox: [-6.5, 5, 6.5, -5], axis: true, showCopyright: false }); var f1t = board.jc.snippet('{#f#}', true, 'x', true); var f1 = board.create('functiongraph', [f1t]); var g1 = board.create('glider', [0.5, 1.2, f1]); var g2 = board.create('glider', [1, 1.2, f1]); // var t1 = board.create('tangent', [g1], {strokeColor: '#66CC33'}); // var t2 = board.create('tangent', [g2], {strokeColor: '#66CC33'}); var l1 = board.create('line', [g1, g2], {strokeColor: '#CC0033'}); var A = board.create('glider', [1,2,l1], {label: {offset: [0,-15]}}); var tangent = board.create('tangent', [A]); var st = board.create('slopetriangle', [tangent], {toppoint: {face: 'plus', withLabel: false}}); board.unsuspendUpdate(); [[/jsxgraph]]

aharjula commented 7 months ago

Unfortunately there is a bug with the sizing of the graph. A fix exists, but we really want to check if we broke anything else before releasing it. The fix can be applied by simply changing a few characters in STACKs code, but as it also requires questions to be recompiled one needs to also do some extra actions to trigger the recompilation. Basically, run a CLI command:

 > php question/type/stack/cli/clearcascache.php --ccc 

Once we have tested enough and Chris rolls out a new release upgrading to that will do all the necessary actions automatically.

sangwinc commented 7 months ago

I'm very sorry about this. We've cherry-picked a fix into the master branch. Either update from master, or pull commit v4.5.0-hf1 https://github.com/maths/moodle-qtype_stack/releases/tag/v4.5.0-hf1 Chris

daniil-berg commented 5 months ago

Sorry to bump this. I just wanted to give some constructive feedback. Instead of making a special "hotfix" tag, IMHO this small change should have been released as regular new patch version that includes the proper upgrade code to clear that cache.

@aharjula

we really want to check if we broke anything else before releasing it

Pardon, but I find this reasoning very misguided. If you know you fixed a bug, especially one that breaks so much, why not just release a new patch version? You can always release another later on, if you find more bugs and fix them. It is not like a new patch version has some "minimum code change threshold".

Also, it took us a long time to figure out that there is a super-special cache for STACK that can only be cleared with the CLI command above. Why is this not part of the STACK admin UI? Is this documented somewhere else?

Sorry, if this sounds frustrated, but it took us a while to figure out how to actually fix the broken questions from v4.5.0. We saw the v4.5.0-hf1 tag and assumed we just needed to check that one out. When that did not work, we cleared all the caches (or so we thought) and were mystified why the problem still persisted. After a lot of trial and error we stumbled across this (closed!) issue and saw that there is this special cache-clearing command.

This was closed more than a month ago, but no real patch (fixed code + cache clearing upgrade code) was released to this day. This is what the "patch" part of the semantic versioning scheme is for, to communicate to users: "there was a bug in the previous version, installing this new version fixes it". Please don't wait this long to release existing fixes.

tilman-schieber commented 5 months ago

Thank you all for the great work and the quick fix but nonetheless I have to echo Daniil's frustration. After setting up a fresh Moodle server with a fresh Stack install all the JSXGraph tasks were not working. I was halfway through writing a bug report when I got the tip from a colleague that there was this hotfix. After installing it, the problem persisted. Now I had to find a closed issue that contains the necessary information to get basic features working.

Please, if you do not want to bump the minor version at least include a notice in the readme or the moodle plugin directory. Everyone who installs Stack at the moment will run into this problem!

aharjula commented 5 months ago

Indeed, we really need to make it so that no one (@sangwinc) ever pulls these hotfixes to the master without a proper version number change that triggers that particular cache clearing. Unfortunately, that is what seems to have happened. The difference (https://github.com/maths/moodle-qtype_stack/compare/v4.5.0...v4.5.0-hf1) between those versions seems to not include anything touching the version number.

These lines need to execute: https://github.com/maths/moodle-qtype_stack/blob/d2db440e6dba06e69730fdc3c14d5850402168c4/db/upgrade.php#L980-L981 And they only execute if someone changes the version number.

And the special tool with the special switch clears that cache only on special requests as unlike the normal CAS evaluation cache, this caches content should never change when running the same version number, and as it is expensive to regenerate, it should not be cleared without a good reason.

aharjula commented 5 months ago

Just to clarify this:

Everyone who installs Stack at the moment will run into this problem!

As far as I know, this particular unfortunate problem applies to those who managed to install STACK 4.5.0 between its release on December 11th and the "release" of the hotfix on the 13th, and managed to populate their caches with values generated with the faulty original 4.5.0. If you are aware of this applying to anyone else please let us know.

tilman-schieber commented 5 months ago

I have installed moodle on my server on January 23rd and I installed Stack some days after, using the newest version offered at https://moodle.org/plugins/qtype_stack/versions (currently "2023121100")

This is the official plugin directory and is the place where I assume most moodle admins will look for the addons.

I have redownloaded it right now, and the offending line is still in place:

in jsxgraph.block.php:

$astyle = "width:calc(100% - 3px);height:calc(100% - 3px);";
aharjula commented 5 months ago

Well that is interesting, I was under the impression that that had been released there... @sangwinc Chris!? This really needs a proper release, with version numbers and it really needs to go all the way to the plugin registry. Otherwise there will be plenty of people with issues and no route to upgrade to through official plugin directory.

sangwinc commented 5 months ago

Thanks for the nudge. I released a tagged version here: https://github.com/maths/moodle-qtype_stack/releases/tag/v4.5.0-hf1 This didn't make into the plugin directory. With hindsight, it should have done. Sorry.

However, the plugin directory for moodle requires different version numbers (reasonably). I've bumped the version number and tagged the new code. I'll release this officially once the continuous integration has completed.

Thanks for prompting this.

sangwinc commented 5 months ago

There is a new release available through the Moodle database. Thanks for your patience. Chris