Achecker-3:Change all "0000-00-00 00:00:00" values to NULL #81
Conversation
updater/index.php
Outdated
| @@ -43,7 +43,7 @@ function print_patch_row($patch_row, $row_id, $enable_radiotton) | |||
| <td><?php if (!isset($patch_row['status'])) echo _AC("not_installed"); else echo htmlspecialchars($patch_row["status"]); ?></td> | |||
| <td><?php echo htmlspecialchars($patch_row["available_to"]); ?></td> | |||
| <td><?php echo htmlspecialchars($patch_row["author"]); ?></td> | |||
| <td><?php if (isset($patch_row['status'])) echo ($patch_row["installed_date"]=='0000-00-00 00:00:00')?_AC('na'):htmlspecialchars($patch_row["installed_date"]); ?></td> | |||
| <td><?php if (isset($patch_row['status'])) echo ($patch_row["installed_date"] == NULL)?_AC('na'):htmlspecialchars($patch_row["installed_date"]); ?></td> | |||
There was a problem hiding this comment.
Would be better to use is_null().
| @@ -51,7 +51,6 @@ public function Create($userID, $title, $abbr, $long_name, $published_date, $ear | |||
| $long_name = trim($long_name); // $addslashes is not necessary as it's called in LanguageTextDAO->Create() | |||
| $earlid = $addslashes(trim($earlid)); | |||
| $preamble = $addslashes(trim($preamble)); | |||
| if ($published_date == '' || is_null($published_date)) $published_date = '0000-00-00'; | |||
There was a problem hiding this comment.
Shall this line still be kept as if ($published_date == '') $published_date = NULL; to ensure the initial date would be set to NULL when the date value is not provided?
There was a problem hiding this comment.
Thanks, will work on it
There was a problem hiding this comment.
The column that accepts the published_date already have a default value of NULL, but will still implement this.
There was a problem hiding this comment.
The concern is, if the public_date value provided to this function is empty, the empty value will then be inserted and override the default NULL.
There was a problem hiding this comment.
Ok, now I understand, thanks, I have made the required changes
…ges to GuidelinesDAO.class.php and updater/index.php
| @@ -29,7 +29,7 @@ CREATE TABLE `checks` ( | |||
| `test_failed_result` text, | |||
| `func` text, | |||
| `open_to_public` tinyint(4) NOT NULL DEFAULT '0', | |||
| `create_date` datetime NOT NULL, | |||
| `create_date` datetime DEFAULT NULL, | |||
There was a problem hiding this comment.
There should always be a create_date value. The original datetime NOT NULL should be correct.
There was a problem hiding this comment.
When updating it to "NOT NULL", should I also update all the create_date columns in the check table to "0" instead of the previous NULL value
There was a problem hiding this comment.
that is to remove all current NULL values and replace it with something else
There was a problem hiding this comment.
I just discovered giving the 0 value, change it to 0000-00-00 00:00:00
There was a problem hiding this comment.
Yes, you are right. It's a historical issue that create_date for system level checks were set to '0000-00-00 00:00:00'. Let's use your approach of DEFAULT NULL. Thanks for the discovery.
There was a problem hiding this comment.
Ok, thanks, will do that
install/db/achecker_schema.sql
Outdated
| @@ -308,7 +308,7 @@ CREATE TABLE `themes` ( | |||
| `title` varchar(80) NOT NULL DEFAULT '', | |||
| `version` varchar(10) NOT NULL DEFAULT '', | |||
| `dir_name` varchar(20) NOT NULL DEFAULT '', | |||
| `last_updated` date NOT NULL DEFAULT '0000-00-00', | |||
| `last_updated` date DEFAULT NULL, | |||
There was a problem hiding this comment.
There should always be a last_updated value. The original NOT NULL should be correct. Just remove DEFAULT '0000-00-00'.
There was a problem hiding this comment.
Same question as above
There was a problem hiding this comment.
last_updated here can be set to DEFAULT NOT NULL as its default value for the rows inserted in achecker_schema.sql does have the initial value. What do you think?
There was a problem hiding this comment.
I think it is ok here, would add a new commit now
There was a problem hiding this comment.
would remove the DEFAULT so that it would be just NOT NULL, since it is not null, so no default value
|
|
||
|
|
||
|
|
||
| ALTER TABLE `checks` MODIFY `create_date` datetime DEFAULT NULL; |
There was a problem hiding this comment.
Modify the upgrade SQL accordingly once above two comments are addressed.
install/db/achecker_schema.sql
Outdated
| (213, 0, 'input', 0, '', '_CNAME_213', '_ERR_213', '_DESC_213', NULL, NULL, '', '_HOWTOREPAIR_213', '', '', '', '', '_PROCEDURE_213', '_EXPECTEDRESULT_213', '_FAILEDRESULT_213', 'return !(BasicFunctions::getAttributeValueInLowerCase(\"type\")==\"text\" && !BasicFunctions::associatedLabelHasText());', 1, '0000-00-00 00:00:00'), | ||
| (214, 0, 'textarea', 2, '', '_CNAME_214', '_ERR_214', '', NULL, NULL, '', '', '', '_QUESTION_214', '_DECISIONPASS_214', '_DECISIONFAIL_214', '_PROCEDURE_214', '_EXPECTEDRESULT_214', '_FAILEDRESULT_214', 'return false;', 1, '0000-00-00 00:00:00'), | ||
| (216, 0, 'input', 0, '', '_CNAME_216', '_ERR_216', '_DESC_216', NULL, NULL, '', '_HOWTOREPAIR_216', '', '', '', '', '_PROCEDURE_216', '_EXPECTEDRESULT_216', '_FAILEDRESULT_216', 'return !(BasicFunctions::getAttributeValueInLowerCase(\"type\")==\"file\" && !BasicFunctions::associatedLabelHasText());', 1, '0000-00-00 00:00:00'), | ||
| (217, 0, 'input', 2, '', '_CNAME_217', '_ERR_217', '', NULL, NULL, '', '', '', '_QUESTION_217', '_DECISIONPASS_217', '_DECISIONFAIL_217', '_PROCEDURE_217', '_EXPECTEDRESULT_217 |
There was a problem hiding this comment.
Something like this, changing the NULL value
…value of themes table and last_updated column to NOT NULL
|
|
||
| UPDATE `checks` SET create_date = NULL WHERE create_date = '0000-00-00 00:00:00'; | ||
|
|
||
| UPDATE `themes` SET last_updated = NULL WHERE last_updated = '0000-00-00 00:00:00'; |
There was a problem hiding this comment.
Shall this line be removed?
There was a problem hiding this comment.
Yes, the themes line
…checker upgrade script
|
Change the version number to 1.5 so you can test the upgrade script. Make sure to test all changes. |
|
Would do that |
|
AChecker database after installing version 1.4 |
|
After Upgrading to version 1.5 |
|
Please commit the change to the version number in include/constants.inc.php so that the upgrade would work consistently in the master when this pull request is merged. |
…the version to 1.5
|
Done |
https://issues.fluidproject.org/projects/ACHECKER/issues/ACHECKER-3?filter=allopenissues