Feedback on v12 stable

Hi Ross,

I couldn’t wait longer and simply had to download the brand new v12 stable.

The installation process went smoothly. I wish the installer wouldn’t force to set a database user password. Currently livevalidation and more code in install.php forces the user to set a password. IMHO to set a password or not should rather be up to the administrator.

I regret that the amount of demo data is significantly reduced now. I really miss timetable demo data.

I get the following two error messages:

q=/modules/Markbook/markbook_view.php:
Fatal error: Can’t use method return value in write context in C:\wamp22\www\gibbon-12-stable\modules\Markbook\markbook_view_allClassesAllData.php on line 327

q=/modules/User Admin/import_users.php:
Notice: Undefined variable: gibbonSchoolYearID in C:\wamp22\www\gibbon-12-stable\modules\User Admin\import_users.php on line 118

That’s all I have for now. Thanks a lot for your hard work!

Kind regards,
Roman

Roman,

Glad to hear you were excited for v12, and thanks for all the feedback.

In terms of the database password, this one could be a huge debate! For production systems I really want to promote the use of a database password as a must, otherwise security will be completely compromised. For testing, I can see why you might not want it, although it is still good practice. I guess you could use Inspect Element to disable the password field validation.

Whilst we did reduce the amount of demo data, we did this mostly by pruning out old information. There should be a full TT in there for every student and member of staff, but you might need to skip forward to early September to see it, due to the term dates in place.

In terms of the errors, I cannot recreate the one in the Markbook, and I wonder if this is a Windows-only issue. Can you describe the issue in more detail, and I have asked Sandra Kuipers, who has done a lot with the Markbook of late, to look into it. If you can let us know more steps to recreate, that would be great.

For the PHP Notice issue in the user import page, this has been fixed in the commit below:

https://github.com/GibbonEdu/core/commit/dc055a85cb5f2af7a0478e8ca5f0f03aaad77778

Thanks for your feedback!

Ross

Thanks for the heads up. Looks like a pre PHP 5.5 backwards compatibility issue:

Note: Prior to PHP 5.5, empty() only supports variables; anything else will result in a parse error. In other words, the following will not work: empty(trim($name)). Instead, use trim($name) == false.

I’ve created a fix in the commit below and put in the pull request:

https://github.com/GibbonEdu/core/pull/77/commits/53cb44a0ebf6bd7f780bd77939f236e9c646bab0

Sandra,

Fantastic, thanks for spotting this. I am actually going to up the requirement for Gibbon to 5.5, which most users should be on (at least by now). I’ll take a look at the commit later on.

Roman,

Can you check your PHP version and let us know what you are on? If below 5.5, can you bring it up to 5.5 at least?

Thanks,

Ross

Hi Ross,

I’m currently on 5.4. I tried to upgrade to 5.5.12 but I’m facing too many challenges.

I’ll try Sandra’s fix later on and then report back to you guys…

Kind regards,
Roman

Hi Ross & Sandra

Sandra’s fix works perfectly! Thanks!

I similar change was needed in modules/Markbook/src/markbookView.php, line 282.

Old: if ( !empty($column->getData(‘gibbonPlannerEntryID’))) {
New: if ( $column->getData(‘gibbonPlannerEntryID’) != false ) {

Work well now!

Kind regards,
Roman

Hi folks,

I found another error message very similar to the one mentionned above:

q=/modules/Students/studentEnrolment_manage_add.php:

Notice: Undefined variable: gibbonStudentEnrolmentID in C:\wamp22\www\gibbon-12-stable\modules\Students\studentEnrolment_manage_add.php on line 184

Kind regards,
Roman

@skuipers, fancy taking a look at this one…seeing as you did such a great job last time : )

Hi Ross & Sandra

I haven’t studied the code very well but I had a closer look at Ross’s commit above in regard to the PHP Notice issue of q=/modules/User Admin/import_users.php. That commit simply removes the line causing the notice output.

Therefore I simply tried the same thing here with the PHP Notice of q=/modules/Students/studentEnrolment_manage_add.php. It seems to solve this problem as well.

Please take this just as a hint. Again, I haven’t studied the code very well. This needs to be confirmed by one of you!

Kind regards,
Roman

Roman,

You were right, this line should not have been there. It has now been removed in the v13 branch, as you can see in the following commit:

https://github.com/GibbonEdu/core/commit/6fff192c4f74d3897b51dd25053c8e69d9c652cf

Gibbon used to be riddled in such PHP Notices, because the system it grew out of (something called Muse that I built for one school a long time ago) was built directly on a server with php.ini set to hide such errors. Sometime around v8 (I think, from memory) we did a massive hunt for such issues, and removed most of them…but as you have discovered, a few remain, and we squash them whenever we can.

Thanks,

Ross

Hi Ross,

Thanks for the last message and its explanation!

I have found another bug I guess. It’s in /modules/Staff/staff_manage_edit_contract_add.php.

I get 10 of the following notices:
Notice: Undefined variable: row2 in C:\wamp22\www\gibbon-12-stable\modules\Staff\staff_manage_edit_contract_add.php on line 184

Can you please have a look?

Kind regards,
Roman

Roman,

Thanks for this. It is now fixed, and in the v13 branch:

https://github.com/GibbonEdu/core/commit/30eb68445820fc8aa72924f86f740a8d5d94d938

Thanks continuing to help us find and fix these issues. In future, please can you put them in a new thread, with a title something like “PHP Notice Bug, v12”?

Cheers,

Ross

Hi Ross,

I found another one here:
/modules/User Admin/import_studentEnrolment.php

The usual removal of the line causing the notice should work.

Kind regards,
Roman

Thanks Roman…it is fixed in this commit:

https://github.com/GibbonEdu/core/commit/41b91c2dbdbce7028854d766299a54c24a03e14f

Thanks Ross!