https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10190 --- Comment #959 from Chloé Zermatten <chloe.zermatten@openfifth.co.uk> --- (In reply to Jonathan Druart from comment #958)

(In reply to Chloé Zermatten from comment #955)

...

2: logged_in_user is not available to circulation triggers as it stands. I could make it available, but it also is a lot more data than what circulation triggers really needs. That said we do need to move from consulting the logged in library to looking for the logged in user’s home library instead - and that much is fixed (see commit message).

Indeed, I thought we had it widely available.

...

5: The comment between those two lines reads “It may still be present in DBs that pre-date the atomic update.” I don’t know if you’ve seen it (won’t come up if grepping I don't think?). Does that answer it? I can ask Martin about it otherwise.

It does not make sense to me. If we run the test on this version of the code, the permission shouldn't be in the DB. Ha, btw, the db_rev is not idempotent.

...

10: best practice: I don’t think so - but I do think it’s a step in that direction (the direction of travel so far had been to remove duplicate code, prevent redundant data (re)loads, remove duplicated logic, etc). Further refactoring into more specialised components will be beneficial. 39756 (long overdue cron) can certainly take that refactoring further.

I don't understand why we have most of those functions in "store". They don't touch the store state. They could certainly be moved to a plain JS module, that could be easily unit-tested without Pinia/Vue at all. Note that I am not requiring unit tests, but only flagging what seems to be a problematic pattern. OTOH that does not seem to be a trivial refactoring...

Re-db_rev: noted - I can look at that later today, but more likely tomorrow. Re-extracting methods that do not touch store state out of store: agreed on the plain JS module option, especially from a future testability standpoint. I think the best way to figure out exactly how non-trivial this is is for me to actually have a crack at it, which I can spend some time on tomorrow. I can either add it in as a further commit on here or raise it as follow up bug (which would be a 39756 dependency) - do let me know if you have a preference! -- You are receiving this mail because: You are watching all bug changes.