|
Description
Jonathan Druart
2023-09-21 12:45:59 UTC
Created attachment 155991 [details] [review] Bug 34862: Remove blocking_errors where there is no main Created attachment 155992 [details] [review] Bug 34862: [DO NOT PUSH] add script Created attachment 155993 [details] [review] Bug 34862: Include blocking_errors.inc everywhere We create a generic 'messages.inc' that will include blocking_errors.inc We can imagine having more messages here. Created attachment 155997 [details] [review] Bug 34862: [DO NOT PUSH] add script Created attachment 155998 [details] [review] Bug 34862: Include messages.inc Generated using `perl add_blocking_errors.pl` Created attachment 155999 [details] [review] Bug 34862: Add messages.inc Created attachment 156000 [details] [review] Bug 34862: Manual changes Jonathan, do you have a test plan for this? One of these days it would be great if folk would support the idea of using WRAPPER (http://www.template-toolkit.org/docs/manual/Directives.html#section_WRAPPER) with Koha so that we could more easily make template changes to every page... (In reply to David Cook from comment #9) > One of these days it would be great if folk would support the idea of using > WRAPPER > (http://www.template-toolkit.org/docs/manual/Directives. > html#section_WRAPPER) with Koha so that we could more easily make template > changes to every page... git grep WRAPPER? (In reply to Lucas Gass from comment #8) > Jonathan, do you have a test plan for this? This is related to bug 34478 IIRC. I caught places where the "wrong csrf" token message was not displayed. Created attachment 158755 [details] [review] Bug 34862: Remove blocking_errors where there is no main Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 158756 [details] [review] Bug 34862: Include messages.inc Generated using `perl add_blocking_errors.pl` Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 158757 [details] [review] Bug 34862: Add messages.inc Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 158758 [details] [review] Bug 34862: Manual changes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Created attachment 158759 [details] [review] Bug 34862: (follow-up) Fix some misplaced messages and add missing This patch fixes some templates where the messages include was appearing in the wrong place, for instance above the left-hand sidebar instead of at the top of the main content. The patch also adds the new include to some templates which lacked it. (In reply to Jonathan Druart from comment #10) > (In reply to David Cook from comment #9) > > One of these days it would be great if folk would support the idea of using > > WRAPPER > > (http://www.template-toolkit.org/docs/manual/Directives. > > html#section_WRAPPER) with Koha so that we could more easily make template > > changes to every page... > > git grep WRAPPER? Apologies for not being clear. What I mean is a wrapper that includes the header and footer. Every page template would use the same wrapper and just fill in the components needed for that page. Similar to what I did for bug 31339 but more comprehensive. That way blocking_error.inc could be included 1 time in the wrapper and then every page would automagically get it because they're using the wrapper. I have a different system that works that way and it's so nice. It would involve a lot of refactoring though for Koha. That said, we could create that wrapper and start using it for new pages and easy refactors... Created attachment 159472 [details] [review] Bug 34862: [DO NOT PUSH] add script Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 159473 [details] [review] Bug 34862: Remove blocking_errors where there is no main Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 159474 [details] [review] Bug 34862: Include messages.inc Generated using `perl add_blocking_errors.pl` Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 159475 [details] [review] Bug 34862: Add messages.inc Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 159476 [details] [review] Bug 34862: Manual changes Signed-off-by: Owen Leonard <oleonard@myacpl.org> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Created attachment 159477 [details] [review] Bug 34862: (follow-up) Fix some misplaced messages and add missing This patch fixes some templates where the messages include was appearing in the wrong place, for instance above the left-hand sidebar instead of at the top of the main content. The patch also adds the new include to some templates which lacked it. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Sorry, but this no longer applies and I don't see how to fix - please rebase. I'll keep checking and push once ready. Apply? [(y)es, (n)o, (i)nteractive] y Applying: Bug 34862: [DO NOT PUSH] add script Applying: Bug 34862: Remove blocking_errors where there is no main Applying: Bug 34862: Include messages.inc error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/en/modules/admin/searchengine/elasticsearch/mappings.tt). error: could not build fake ancestor Patch failed at 0001 Bug 34862: Include messages.inc hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-34862-Include-messagesinc-tp2ov_ox.patch I did a second run with -i removing the first patch, but still no luck. Created attachment 161666 [details] [review] Bug 34862: Remove blocking_errors where there is no main Created attachment 161667 [details] [review] Bug 34862: Remove blocking_errors where there is no main Created attachment 161668 [details] [review] Bug 34862: [DO NOT PUSH] add script Created attachment 161669 [details] [review] Bug 34862: Include messages.inc Generated using `perl add_blocking_errors.pl` Created attachment 161670 [details] [review] Bug 34862: Add messages.inc Created attachment 161671 [details] [review] Bug 34862: Manual changes Created attachment 161672 [details] [review] Bug 34862: (follow-up) Fix some misplaced messages and add missing This patch fixes some templates where the messages include was appearing in the wrong place, for instance above the left-hand sidebar instead of at the top of the main content. The patch also adds the new include to some templates which lacked it. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> (In reply to Katrin Fischer from comment #25) > I did a second run with -i removing the first patch, but still no luck. This is the kind of patch (190KB) that must not be waiting 2 months before pushing... I push according to priorities, if you know this give me a heads up. Maybe it should have been marked as bug (not enh?) (In reply to Katrin Fischer from comment #34) > I push according to priorities, if you know this give me a heads up. Maybe > it should have been marked as bug (not enh?) blocker of a huge security bug? I can't tell that easily - no dependencies on the bug, marked enh. As I said, give me a heads up, but if it had stood out, I would have been here earlier. I can only get the first patch to apply. Removed the second. Is the sequence correct? Apply? [(y)es, (n)o, (i)nteractive] i Applying: Bug 34862: Include messages.inc Using index info to reconstruct a base tree... M koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation-home.tt M koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt error: insufficient permission for adding an object to repository database .git/objects error: unable to create backing store for newly created file koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation_batch_checkouts.tt error: Did you hand edit your patch? It does not apply to blobs recorded in its index. Patch failed at 0001 Bug 34862: Include messages.inc hint: Use 'git am --show-current-patch=diff' to see the failed patch When you have resolved this problem run "git bz apply --continue". If you would prefer to skip this patch, instead run "git bz apply --skip". To restore the original branch and stop patching run "git bz apply --abort". Patch left in /tmp/Bug-34862-Include-messagesinc-hfhzpu3c.patch Created attachment 161673 [details] [review] Bug 34862: Remove blocking_errors where there is no main Created attachment 161674 [details] [review] Bug 34862: [DO NOT PUSH] add script Created attachment 161675 [details] [review] Bug 34862: Include messages.inc Generated using `perl add_blocking_errors.pl` Created attachment 161676 [details] [review] Bug 34862: Add messages.inc Created attachment 161677 [details] [review] Bug 34862: Manual changes Created attachment 161678 [details] [review] Bug 34862: (follow-up) Fix some misplaced messages and add missing This patch fixes some templates where the messages include was appearing in the wrong place, for instance above the left-hand sidebar instead of at the top of the main content. The patch also adds the new include to some templates which lacked it. Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com> Sorry for the inconvenience - it appears to have been a permission issue on my end. Patches applied now. Pushed for 24.05! Well done everyone, thank you! What about OPAC here ? I choose to backport to avoid future conflicts in templates. Pushed to 23.11.x for 23.11.03 |