Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Reproduce: Enable tracker module
Expected behaviour: no error
When enabling tracker module the following error occured:
Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[22007]: Invalid datetime format: [error]
1366 Incorrect integer value: '' for column 'published' at row 1: INSERT INTO
{tracker_node} (nid, published, changed) VALUES (:db_insert_placeholder_0,
:db_insert_placeholder_1, :db_insert_placeholder_2); Array
(
[:db_insert_placeholder_0] => 1493
[:db_insert_placeholder_1] =>
[:db_insert_placeholder_2] => 1497601800
)
in tracker_cron() (line 78 of
/var/www/*/htdocs/core/modules/tracker/tracker.module).
apache
PHP 7.0.15-0ubuntu0.16.04.4 (cli) ( NTS )
Copyright (c) 1997-2017 The PHP Group
Zend Engine v3.0.0, with Zend OPcache v7.0.15-0ubuntu0.16.04.4,
Comment | File | Size | Author |
---|---|---|---|
#26 | 2887490-19.patch | 1.69 KB | cilefen |
#19 | 2887490-19.patch | 1.69 KB | michaellenahan |
#18 | 2887490-18.patch | 923 bytes | michaellenahan |
#12 | 2887490-12.patch | 2.28 KB | michaellenahan |
#10 | 2887490-10.patch | 1.49 KB | michaellenahan |
Comments
Comment #2
cilefen CreditAttribution: cilefen commentedI have optimistically renamed this because I think based on reproducing the error, that you have got unpublished nodes. Is that true? As a workaround, you could:
Comment #3
cilefen CreditAttribution: cilefen commentedIf you do that, you are going to get that error logged every time cron runs as long as this bug exists.
Comment #4
cilefen CreditAttribution: cilefen commentedComment #5
Rop CreditAttribution: Rop as a volunteer commentedYou are correct. I do have many unpublished nodes.
Also, I 'm using moderation, I should have mentioned that I suppose.
In the current moderation state the nodes are not public. Unfortunately I cannot make them public in bulk, since actions for moderation are not available.
I'll test your suggestion later on a clean install.
thank you for the quick reply!
By the way: The module was, despite the error, successfully enabled.
Comment #6
cilefen CreditAttribution: cilefen commentedTest the patch.
Comment #7
Rop CreditAttribution: Rop as a volunteer commentedpatch seems to do the trick. No Errors.
Comment #8
Rop CreditAttribution: Rop as a volunteer commentedComment #9
catchWe should add some test coverage here. Also should it just cast the bool to int instead of the ternary? Functionally the same but less logic.
Comment #10
michaellenahan CreditAttribution: michaellenahan at erdfisch commentedHi. This is my first time adding a drupal test. I'd be grateful for feedback.
This patch contains the test but no fix for the original problem, so I expect this test to fail, as it did locally.
Next, I'll add a patch with the fix and the test, which should pass.
Comment #11
michaellenahan CreditAttribution: michaellenahan at erdfisch commentedStatus: Needs review to get testbot to test.
Comment #12
michaellenahan CreditAttribution: michaellenahan at erdfisch commentedHere is the patch with the fix. I am casting to int, following the suggestion at #9.
Comment #14
michaellenahan CreditAttribution: michaellenahan at erdfisch commentedTest #12 passed, so back to RTBC.
Comment #15
michaellenahan CreditAttribution: michaellenahan at erdfisch commentedSorry. I don't think I'm allowed to do that myself. So back to Needs Review.
Comment #16
dawehnerSo this code runs in cron, so shouldn't we explicitly trigger cron in that case?
Comment #17
michaellenahan CreditAttribution: michaellenahan at erdfisch commentedYou mean, we should trigger cron in the test?
Comment #18
michaellenahan CreditAttribution: michaellenahan at erdfisch commentedYes good point. So I can create a failing test by amending
testTrackerCronIndexing()
, and don't actually need to test the enabling of the module explicitly.Here's the failing test.
Comment #19
michaellenahan CreditAttribution: michaellenahan at erdfisch commentedAnd here's the test with the fix, so this patch should be green.
Comment #21
Pascal- CreditAttribution: Pascal- commentedTested #19 against 8.4.4 and it works.
I have some concerns though.
1) How is this a core module with a bug this big in it?
2) On installation, it seems to go through your whole site adding new records for every single node on your website. This seems risky at best for large websites.
3) Anyone can just open the /activity link? What?
Do we need new issues for this?
This really does not seem ok to me ...
Comment #22
cilefen CreditAttribution: cilefen as a volunteer commented@Pascal- Yes, we would need a new issue to overhaul what this module does when it is enabled.
Comment #23
cilefen CreditAttribution: cilefen as a volunteer commented#2951030: Invalid text representation: 7 ERROR: invalid input syntax for integer
Comment #24
joachim CreditAttribution: joachim as a volunteer commentedPatch looks good.
(Though this is a bit of a WTF in the DB system - can we file a follow-up to either fix or document it?)
Comment #25
cilefen CreditAttribution: cilefen as a volunteer commentedComment #26
cilefen CreditAttribution: cilefen as a volunteer commentedI am re-uploading the #19 patch to get a test run.
Comment #27
alexpottNice find and fix!
Crediting @rOprOprOp for finding and reporting the bug.
Crediting @catch for insisting on test coverage and suggesting simpler logic.
Re #24 - you think we need to do something in drupal_schema_get_field_value() to change boolean values automatically to integers. I'm not sure that that is going to be a simple change.
Re #21.1 - tracker is not the most used module - it is not enabled in the standard profile for example. Bugs happen and it's great when they are reported and fixed!
.2 It does not go through all nodes. There's a configurable limit per cron run and this limit is used on module install.
.3 It's a feature of the module - it just lists the content a user has access to on the site. It does not list any content the user could not find with google.
Committed and pushed 713294e96e to 8.6.x and 384488fa5e to 8.5.x. Thanks!
Comment #30
Rop CreditAttribution: Rop as a volunteer commentedThank you for the fix!