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.Hello everyone,
first, I'd like to congratulate the maintainers for this great module.
Second, I'd like to report an unexpected error when we try to set conditions greater than 64 characters.
The website encountered an unexpected error. Please try again later.Drupal\Core\Database\DatabaseExceptionWrapper: SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'name' at row 1:
UPDATE {abjs_condition} SET
name=:db_update_placeholder_0,
script=:db_update_placeholder_1,
changed=:db_update_placeholder_2,
changed_by=:db_update_placeholder_3
WHERE cid = :db_condition_placeholder_0;
Array
(
[:db_update_placeholder_0] => The path is /redacted-123456789-123456789-123456789-123456789-123456789-123456789-123456789
[:db_update_placeholder_1] => return window.location.pathname == '/redacted-123456789-123456789-123456789-123456789-123456789-123456789-123456789';
[:db_update_placeholder_2] => 1563545945
[:db_update_placeholder_3] => 1
[:db_condition_placeholder_0] => 5
)
There is some specific reason to have this limit of characters?
How should we handle this problem?
Comments
Comment #2
mhavelant CreditAttribution: mhavelant at Brainsum for Tieto commentedHi!
The name field is added by the base module
abjs.I haven't looked at the code for a while, but glancing through it I can see that when a condition is added we auto-generate the name with
"The path is $conditionPath". Note, there's a todo about displaying proper validation messages.Adding a new field for condition names, so users have to enter them manually, would probably be a proper solution for this.
Maybe truncating the auto-generated name to 64 characters, but that might lead to some confusion for users in some cases.
Alternatively, you could open an issue for abjs for allowing longer names (in this case a schema update would be needed).
Comment #3
andre.bonon CreditAttribution: andre.bonon at CI&T commentedHi @mhavelant,
As you said, creating a new field for condition name is a better solution, but it'll demand more effort regarding implementation and testing.
I analysed the code and I think that if we truncate the condition name it'll not cause unexpected issues.
The condition name is displayed on AB JS condition list page only.
So, I created a patch to fix that and it is working fine.
Please take a look and tell me what do you think.
Comment #4
mhavelant CreditAttribution: mhavelant at Brainsum for Tieto commentedI tested the patch, works great, thank you!
Comment #5
mhavelant CreditAttribution: mhavelant at Brainsum for Tieto commentedComment #7
mhavelant CreditAttribution: mhavelant at Brainsum for Tieto commentedFixed in the new beta7 release.