The Drupal 7 Date Repeat Field module stores the rrule string as type TEXT, while the Recurring Dates Field module stores the rrule string as a VARCHAR(255). This means that migrations fail when the rrule is greater than 255 characters.

Tasks

  • Field migration for base and attached fields
  • Site builders should be able to configure maximum length on a field storage level (not database).
  • A field constraint must be added to enforce the length.
  • A sensible default length should be added. 255 chars?
  • Basic field validation tests.
  • Migration test from old schema for various types of entities (base, attached), non revisionable, revisionable etc.
  • Widget validation test: ensure validation error shows if over configured length.
CommentFileSizeAuthor
#6 date_recur_8.1_rrule_schema_change-2994902-3.patch4.08 KBtasc
#6 date_recur_8.1_rrule_schema_change-2994902-3.patch4.08 KBtasc
#4 date_recur_8.1_rrule_schema_change-2994902-2.patch3.22 KBtasc
#2 date_recur_rrule_schema_change-2994902-1.patch548 bytesstephen-cox

Comments

stephen-cox created an issue. See original summary.

stephen-cox’s picture

stephen-cox commented
Title: Migration from Drupal 7 date_repeat fail when rrule is greater than 255 characters » Migration from Drupal 7 fails when rrule is greater than 255 characters
Issue summary: View changes
StatusFileSize
new date_recur_rrule_schema_change-2994902-1.patch548 bytes

Here is a little patch to change the DB schema for the rrule column from VARCHAR(255) to TEXT. It needs to be applied before the module is installed, otherwise the module needs to be re-installed and all date_recur field data will be lost.

dpi’s picture

dpi
Location Perth, Australia
commented
Title: Migration from Drupal 7 fails when rrule is greater than 255 characters » Support RRULE's with more than 255 characters
Status: Active » Needs work

Thanks, there should be a update-path/migration though.

tasc’s picture

tasc
Primary language German
Location Berlin
commented
StatusFileSize
new date_recur_8.1_rrule_schema_change-2994902-2.patch3.22 KB

This is my 8.1-version of the patch with an update_hook to change the schema for existing data.

dpi’s picture

dpi
Location Perth, Australia
commented
Priority: Normal » Major
Issue summary: View changes

Added various tasks around this.

This ticket is a blocker for 2.x stability.

Patch above doesnt conform to Drupal coding standards. (including, odd arrow spacing, snake_case)

Are you able to update patch with new tasks for this issue?

tasc’s picture

tasc
Primary language German
Location Berlin
commented
StatusFileSize
new date_recur_8.1_rrule_schema_change-2994902-3.patch4.08 KB
new date_recur_8.1_rrule_schema_change-2994902-3.patch4.08 KB

Reviewed the code and updated it to also alter the storage schema. But it's still varchar(255) => text.

Concerning the tasks I'm afraid I can't help a lot. Maybe with the configurable lengths.
But I don't really see why it can't be text. Is this for db-performance/size reason?
At least including/excluding dates add minimum 17 chars to the rrule, so users will always be restricted here.

tasc’s picture

tasc
Primary language German
Location Berlin
commented
dpi’s picture

dpi
Location Perth, Australia
commented

> But I don't really see why it can't be text.

It should be TEXT, but the max length should be enforced by the app to prevent abuse.

  • dpi committed ee73dfd on 8.x-2.x authored by stephen-cox 
    Issue #2994902 by stephen-cox: Support RRULE's with more than 255...
artis’s picture

artis commented
Status: Needs work » Fixed

This has been committed.

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Fixed » Active

Further tasks to be addressed as outlined in IS

dpi’s picture

dpi
Location Perth, Australia
commented
Status: Active » Fixed

Split out tasks to new issues:

dpi’s picture

dpi
Location Perth, Australia
commented

Max length and schema update path committed.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.