Hi,
thank you very much for advancing this module. I'm using it quite heavy on one project and the alpha was a bit worying. ;) But the module does exactly what I was after.

Problem/Motivation

My issue is, that for my use case the text value is only optional and not always available. In mot of the time I just need to select the reference (to person content type). After update the module logic changes and if the text field value `$value === NULL || $value === ''` the whole `function isEmpty()` is set to TRUE. The result is it removes the reference as well.
It does removes the references (with empty text) directly on node edit.
I like the improvement, that missing reference is not throwing WSOD.

Steps to reproduce

  • Install the previous alpha version (4.0.0-alpha4)
  • Add Reference Value Pair field
  • Add few items, some with text, some without
  • Only the reference is required
  • Upgrade to version 5.0.2
  • Edit the node with the fields.
  • Only the values where text is present are preserved
  • If you enter new references without text, they are deleted on safe.

Proposed resolution

Make the text value optional again to preserve data made by older versions.
Or add there a settings in the widget, where I can choose if the text is required or not on a field by field bases.

Thank you for considering this request.

Issue fork reference_value_pair-3601071

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

petr illek created an issue. See original summary.

petr illek opened merge request !9

petr illek’s picture

petr illek
Primary language Czech
commented

I try to create a MR, which reverts the isEmpty to the previous logic.
Hopefully I did it on the right place against the right branch. ;)

kyberman’s picture

kyberman
Location Czech Rep. 🇨🇿
commented
Version: 5.0.2 » 5.0.x-dev
Status: Active » Needs review

Hi Petr, if your goal is to make the text optional, I would say the isEmpty() function should contain only this single line:
return $this->isEntityEmpty();
But then, inside propertyDefinitions(), the "setRequired(TRUE)" should be removed for the "value" property.

The ideal solution from my point of view is to:
1) Make the "value" storage always optional.
2) Add a checkbox to the field settings, switching between optional and required text.
3) Update the isEmpty() logic based on the settings and actual value.
This would be great to discuss with maintainers, fyi @isaacrc, @jjcarrion, @miiimooo.

Best regards
Vit