Address remaining problems with the menu (#418 follow-up)
The background of the problem is as follows: The menu structure was hardcoded in the the access checks, so specific menu item (e.g. menu 1, subitem 2) was toggled to be visible when user was a member of super-admin group. This approach was susceptible to menu changes and the problem was triggered by changes introduced in #357 (closed). #418 (closed) partially addressed the problem, but due to stress, time pressure and other factors was not handled correctly and hence this ticket.
Here are the conclusions:
-
a function that locates menu item is needed. There should be one function that handles both top level and sub-menu. The proposal made here is reasonable and the recommended way to go. While the performance is a factor, the number of menu items is expected to be small. Also, the function is believed to be called small number of times only when the page is first loaded, so it's not a major limitation. On the other hand, the robustness of the code is essential, so we are 100% sure the menu layout will change. @marcin, please take a look at this. -
there should be a system test for this. Surprisingly enough, @wlodek developed the first system test using selenium framework that would test the original problem here. So the effort to deliver system tests is in progress. @wlodek is taking care of this. -
There should be a unit-test that checks if the getItem()
function works appropriately. However, our existing UI unit-test framework is broken. See #164 (closed). So before we can able to tackle this problem, #164 (closed) should be addressed first. @tomek will do the initial attempt here, but most likely @godfryd will need to take this over. -
Testing is an area that was neglected for too long. We had good reasons for that, but the situation is definitely different now and we no longer need to move forward at break neck speed and we have time for thorough tests, doc and comments. @tomek to update the coding guidelines.