Details
-
Type:
Bug
-
Status:
Closed
-
Priority:
Major
-
Resolution: Fixed
-
Affects Version/s: 2.0
-
Fix Version/s: 2.0
-
Component/s: Database SQL/XMLDB
-
Labels:None
-
Affected Branches:MOODLE_20_STABLE
-
Fixed Branches:MOODLE_20_STABLE
Description
It seems that the data->notification field doesn't have one suitable default value and it has caused some problems in Moodle 1.9 (see MDL-14540 and MDL-14323).
For 19_STABLE I fixed that by adding one default value in data_update_instance() but that isn't the definitive change, because that field will be plagued with zeros and nulls (depending of the DB).
So, we need to definitively fix this under Moodle 2.0 (requires DB changes). The plan is:
1) Require permission to MD about to change DB in HEAD (perhaps we are still sticky to 19_STABLE). If permission is given, continue, else, hant until HEAD DB changes are allowed.
2) As part of the data module upgrade script:
a) Upgrade all the data->notifications currently being NULL to 0 (zero).
b) Modify the field to be NOT NULL
c) Modify the field to have a DEFAULT of 0 (zero)
(note b & c must be performed in two steps)
3) Modify install.xml (for new installations) in order to have that field defined as NOT NULL DEFAULT 0
4) Upgrade data module version.php to make the upgrade happen.
All these exclusively under HEAD. As said, plz, confirm with MD if we are free to start changing DB in HEAD.
Ciao 
Attachments
Issue Links
| This issue has a non-specific relationship to: | ||||
| MDL-24470 | Database: upgrade doesn't seem to mark the 'notification' column as not null causing restore problem |
|
|
|
| This issue has been marked as being related by: | ||||
| MDL-14540 | Database activity import preset don't work correctly |
|
|
|
| MDL-14323 | Database Presets do not import templates |
|
|
|
Activity
- All
- Comments
- History
- Activity
- Source
- Test Sessions
Looks correct!
Anyway, some comments... 
0) The two steps are correct, although you can define the whole field specifications in the same $field->setAttributes() line (i.e. the default too). And then you perform both the change_field_notnull() and change_field_default() calls.
1) Have you used the XMLDB Editor (under Admin->Misc->XMLDB Editor) to edit the install.xml file and get PHPcode? It' seems that no, compare it with your code, please. IMO it's always recommended to use the editor to get base code. And then, add "extras" if needed (line the update in this example). I'd encourage you to play some time to get used with it.
2) I think that the "notification" field is also wrong on its "signed/unsigned" part. If I'm not wrong, should be "unsigned" (if it's used to store unix timestamps).
Ciao
0) The two steps are correct, although you can define the whole field specifications in the same $field->setAttributes() line (i.e. the default too). And then you perform both the change_field_notnull() and change_field_default() calls.
1) Have you used the XMLDB Editor (under Admin->Misc->XMLDB Editor) to edit the install.xml file and get PHPcode? It' seems that no, compare it with your code, please. IMO it's always recommended to use the editor to get base code. And then, add "extras" if needed (line the update in this example). I'd encourage you to play some time to get used with it.
2) I think that the "notification" field is also wrong on its "signed/unsigned" part. If I'm not wrong, should be "unsigned" (if it's used to store unix timestamps).
Ciao I've patched also mod/data/restorelib.php, because old backups were sending nulls and that was breaking into 2.0 (where the field is not null). Working now. Tested. Ciao
I attach a patch here.
The changes to the field were performed in two steps, am I right? (The first time use xmldb lib)