Smart menu

[stefanscholz]

This pull request contains an early version for the smart menu feature (#137) as well as some foundational work for the menu bar #136.

[abias]

Hey @stefanscholz and @prasanna-lmsace ,

as you have told me that this PR is ready to be reviewed, Stefan, I will now remove the "Draft" status and will start now reviewing this PR.

Please note that I might push review changes directly to your branch. If you need to add any further commits, please add them on top of my review changes and do not overwrite them. Thank you!

Cheers,
Alex

[wiebkemueller-hsh]

Hi,

during functional testing of the smart menu feature in our local testing environment, we made the following observations:

Specification of the local testing environment
PHP 8.1.20
memory_limit = 128MB > raised to 256MB
memory_limit = 256M in php.ini
DB Postgres 12.11 (including a database dump with real data from our live system)
Moodle 4.1.4 (Build: 20230612)
Boost Union:
First try with this PR#300 which was added to v4.1-r7 and then merged/rebased with v4.1-r8
and in a 2nd try the https://github.com/bdecentgmbh/moodle-theme_boost_union/tree/smart_menu directly to a v4.1-r7

1. A smart menu item "static" and "heading" cannot be saved.
The entry form won't let us save the dialogue, but there is no error message telling us what is wrong.

To reproduce:

type static URL
create new smart menu, location main navigation, type list, mode submenu
add new item, type static URL (our example /local/hsh/request.php)
try to save

type heading
create new smart menu, location main navigation, type list, mode submenu
add new item, type heading
try to save

2. Fallback "extra menu item" leads to system crash

Resulting out of the behaviour described in 1 we tried to implement a fallback which made the whole system crash. We put an extra menu item in the main navigation via the theme settings, that links directly to the URL we wanted to link to in the first place. We had used this functionality before for the time being to provide quick access to the course request form. The idea was to combine this with the smart menu that would then sit next to the custom menu item in the case the static link would take some more time.

To reproduce (see also screen 3):
Go to admin/settings.php?section=themesettings
add URL in custommenuitems (our example is Kurs beantragen|/local/hsh/request.php|Nicht für Studierende)

After this was done the session was destroyed and the user (admin) was logged out. Trying to login and reuse the system constantly reproduces the following errors, see also screen 1 and sometimes screen 2.

Warning: session_start(): Failed to decode session object. Session has been destroyed in /var/www/html/lib/classes/session/handler.php on line 45
Error
error/An error occurred whilst communicating with the server
Debug info:
Error code: An error occurred whilst communicating with the server
$a contents:
Stack trace:

line 159 of /lib/classes/session/manager.php: core\session\exception thrown
line 139 of /lib/classes/session/manager.php: call to core\session\manager::start_session()
line 842 of /lib/setup.php: call to core\session\manager::start()
line 79 of /config.php: cal

We hope the given screenshots and error message help to identify the problem. I tried out the smart menu in https://m41.bdecent.io/ before and adding headings and URLs as static links to the menu worked out fine, so we are really keen on finding out what the problem could be. Are there extra menu items set? I didn’t want to take a risk and crash that system too, so I didn’t add one.

Regards
Wiebke (from HsH)

Screen 1

Screen 2

Screen 3

[abias]

@prasanna-lmsace @stefanscholz - My review of this PR is still ongoing.

Please note that I just rebased the PR branch onto the latest master. Additionally, I squashed the PR branch so that just one commit remains and the PR just increments the theme's version by one. This will help the integration which will happen soon.

[abias]

I just pushed a commit with my latest review results. It does not change the contributed functionality, but helps to integrate the patch into the existing codebase.

[abias]

Hi @prasanna-lmsace ,

thank you very much for pushing some very important fixes and improvements to the smart_menu branch which is the basis for this PR.

As I have written above, I am still in the middle of the code review of this really large piece of code. And I have added a "Review changes" commit on top of your developments which polishes your code in many ways and helps to integrate it into the existing codebase.

However, I had to see that you did not rebase your latest changes properly onto the codebase which was already there in the smart_menus branch. You somehow merged my changes into your codebase and then pushed it.

This makes it really hard for me to understand what you have really changed recently.

Please look at this image:

The commits which I have stroke through must not be there. You should just add additional commits, ideally one commit per fixed issue and improvement.

After speaking to @stefanscholz , I have now foce-pushed my latest reviewed codebase again and have thus reverted your latest changes. I would like to ask you to rebase your work properly and to re-push the commits into the branch.

To help you to do this, I will not push any review changes until your commits are there again.

Thank you for your cooperation :)

[abias]

Thank you @prasanna-lmsace very much for pushing proper commits to the branch now, I highly appreciate your effort!

[abias]

Thank you all for working on and commenting in this PR!

I will provisionally merge this now into the master branch to allow the remaining minor issues which have been discovered internally to be handled within dediated follow-up Github issues over the next days.

An official release on moodle.org/plugins which includes the smart menus will follow as soon as we have burnt the list of follow-up issues down.

Cheers,
Alex