I created this D8-focused issue to avoid confusion between D7 and D8 patches for the issue described in #1934972: Allow open range (from without to or to without from).

CommentFileSizeAuthor
#18 range-allow-open-range-2885516-18-D8.patch5.99 KBmsielski
#14 range-allow-open-range-2885516-14-D8.patch5.23 KBAron Novak
#5 range-allow-open-range-2885516-5-D8.patch10.58 KBjdleonard
#2 2885516-range-disable-both-values-required.patch2.49 KBjdleonard
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jdleonard created an issue. See original summary.

jdleonard’s picture

jdleonard
he/him
Primary language English
CreditAttribution: jdleonard commented
FileSize
2885516-range-disable-both-values-required.patch2.49 KB

I don't have time to provide a RTBC patch right now, but I attach a sledgehammer approach for all Range fields that disables the constraint validator that ensures that both the "from" and "to" value exist. This patch doesn't do anything else (e.g. adjust formatting logic). Obviously, this patch is not suitable to be committed as-is, but perhaps it will help someone else, who, like me, just needs to let folks optionally omit either the "from" or "to" sub-field.

jdleonard’s picture

jdleonard
he/him
Primary language English
CreditAttribution: jdleonard commented
Status: Active » Needs work

I thought #2 was a good quick fix, but my flawed testing using an Inline Entity Form (I hit the "Update [entity-label]" button, but not the "Save" button) obscured the fact that the field definition requires that both "from" and "to" exist, resulting in empty "from"/"to" turning into zeros. A bunch of other changes are necessary to make this work, including a hook_update_N() implementation to update existing database tables.

Taran2L’s picture

Taran2L
Primary language Ukrainian
Location Lviv
CreditAttribution: Taran2L at Lemberg Solutions, Drupal Ukraine Community commented

And updates to views, new tests, new configuration options... this feature is not that easy

jdleonard’s picture

jdleonard
he/him
Primary language English
CreditAttribution: jdleonard commented
Status: Needs work » Needs review
FileSize
range-allow-open-range-2885516-5-D8.patch10.58 KB

Here's a more complete patch, which I have tested lightly. The following still needs to be done:

  • Provide upgrade path from existing fields
  • Configuration options to toggle validation for from/to sub-fields being non-empty
  • Update/add tests
  • Greater configurability of formatters when from/to is empty

For now, this can be used with existing fields by manually editing the relevant field/revision database tables to allow null values for the from/to columns.

Setting to "needs review" for feedback. Thanks!

droprocker’s picture

droprocker CreditAttribution: droprocker commented

I get an error if I try to patch with composer:

Could not apply patch! Skipping. The error was: Cannot apply patch https://www.drupal.org/files/issues/range-allow_single_values-1934972-4....

Using Drupal 8.5.0 and Range Module 1.x-dev.

jdleonard’s picture

jdleonard
he/him
Primary language English
CreditAttribution: jdleonard commented

@der_wilko: It looks like you're applying the wrong patch. Try https://www.drupal.org/files/issues/range-allow-open-range-2885516-5-D8....

droprocker’s picture

droprocker CreditAttribution: droprocker commented

@jdleonard: Thanks. The patch is now successfully applied. But I'm not quite sure what to do with this information

For now, this can be used with existing fields by manually editing the relevant field/revision database tables to allow null values for the from/to columns.

Could you explain, what to do!? Because right now I still get the error "Both range values (FROM and TO) are required." For new an existing fields.

Thanks in advance...

jdleonard’s picture

jdleonard
he/him
Primary language English
CreditAttribution: jdleonard commented

@der_wilko You'll need to use a database management tool (e.g. phpMyAdmin or Sequel Pro) to edit the structure of the field data and field revision tables for the relevant range field. Specifically, you'll need to change the structure of these tables' "from" and "to" columns to allow NULL values.

markdc’s picture

markdc
his lordship
Primary language English
CreditAttribution: markdc commented

I need this feature, but I get the error "This value should be of the correct primitive type." Steps to reproduce:

  1. Apply patch #5 to dev version (I also tried 1.1)
  2. Install module for first time
  3. Add Range (integer) field to Node
  4. No special config (not required, no min/max, no prefix/suffix)
  5. Add 'From' value only
  6. Save form
  • Drupal 8.5.5
  • PHP 7.0.30
  • MySQL 5.7.22
asak’s picture

asak CreditAttribution: asak commented

This patch applies and works, but it's not enough to allow open range values to be saved. The constraints that come with the module need to be overridden to complete the functionality. I duplicated the RangeBothValuesRequiredConstraint and RangeFromGreaterToConstraint classes and implemented hook_validation_constraint_alter() to complete the functionality.

droprocker’s picture

droprocker CreditAttribution: droprocker commented

Patch doesn't apply anymore on 8.x-1.2. But issue seems to be solved. Can anyone confirm? This issue is not mentioned in the Changelog of 1.2.

Taran2L’s picture

Taran2L
Primary language Ukrainian
Location Lviv
CreditAttribution: Taran2L at Lemberg Solutions, Drupal Ukraine Community commented
Status: Needs review » Needs work
Issue tags: +Needs tests, +Needs change record

hi @der_wilko, this issue has not been solved as of yet, as it requires significant work in order to make it right, including Views integration, new formatter settings, tests updates, etc

Patch provided in #2 just removes the validation, and does not touch any other part of the module. Patch from #5 does a better job, but still there are much more to do.

This feature is changing the core behavior of the module and having ~1500 active installs that may break does not allow me to commit it as is.

If someone needs this, then please vote for it, contribute and I will pick it up.

Aron Novak’s picture

Aron Novak
Primary language Hungarian
Location Hungary, Budapest
CreditAttribution: Aron Novak commented
FileSize
range-allow-open-range-2885516-14-D8.patch5.23 KB

I re-rolled the patch in #5, so it can be applied to the latest dev, it has the same limitations that are explained in https://www.drupal.org/project/range/issues/2885516#comment-12145660

Aron Novak’s picture

Aron Novak
Primary language Hungarian
Location Hungary, Budapest
CreditAttribution: Aron Novak commented

For the record, you need to execute 4 SQL commands to change one field to work with the patch (only for existing fields), for example:

ALTER TABLE node__field_age MODIFY field_age_from INT(11);
ALTER TABLE node__field_age MODIFY field_age_to INT(11);
ALTER TABLE node_revision__field_age MODIFY field_age_from INT(11);
ALTER TABLE node_revision__field_age MODIFY field_age_to INT(11)
droprocker’s picture

droprocker CreditAttribution: droprocker commented

Patch in #14 applies to 1.3. But the patch renders NULL values as zero instead of just not showing them at all. It's a big difference if I say the range is "up to 2" or "0 to 2".
The patch in #5 included this behaviour perfectly.

a.milkovsky’s picture

a.milkovsky
he/him
Location UTC +2
CreditAttribution: a.milkovsky at k-digital commented

I propose next solution:

  • Make this feature optional and deactivated by default, so that other websites, that use this module, will not be affected.
  • Provide 2 new settings on the field storage/field widget: "Make 'From' value optional", "Make 'To' value optional".
  • Adapt views integration, field display, field widget, write new tests.
msielski’s picture

msielski
he/him
CreditAttribution: msielski at CivicActions for National Science Foundation commented
FileSize
range-allow-open-range-2885516-18-D8.patch5.99 KB

I updated patch #14 to address the "value should be of correct primitive type" error we were receiving. I used a solution similar to https://www.drupal.org/project/drupal/issues/2220381#comment-12771574, adding a massageFormValues to the widget to unset empty values. You'll still manually need to update existing DB fields to remove their NOT NULL constraints.