When we use this widget with the services module, there are some issues as to how this field is quite different to the core select widget.

The main issue I have is that the NULL or (-None-) selection option is set via the following JSON.

    "field_name": {
        "und": { "select": "" }
    }

Where as the core drupal select list is set as follow for a NULL or (-None-) selection option.

    "field_name": {
        "und": { "select": "_none" }
    }

This might not seem like much but for other systems trying to interact with a REST web service it is difficult to explain why there is a difference.

Also we have found that when setting the value of other to a known value that it gets appended to other rather than falling back to the predefined value though that ticket is open here http://drupal.org/node/1818876.

I would request that we better align with Drupal core's way of handling select input values via REST (in my case i have been using services 3.x).

CommentFileSizeAuthor
#14 interdiff.txt662 byteslegolasbo
#14 align_with_drupal_core-1830090-14.patch12.19 KBlegolasbo
#13 align_with_drupal_core-1830090-13.patch12.69 KBlegolasbo
#11 1830090-11-add-none-to-validation.patch842 bytesmthomas
#10 1830090-3-align-with-drupal-core-null-followup.patch1.82 KBmrfelton
#8 1830090-illegal_choice_in_validate.patch625 bytesMrHaroldA
#5 select_or_other-1830090-5.patch1.91 KBstar-szr
#5 select_or_other-none_duplication.png9.42 KBstar-szr
#3 1830090-3-align-with-drupal-core-null.patch1.83 KBlathan
#1 1830090-2-align-with-drupal-core-null.patch630 byteslathan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

lathan’s picture

lathan
Location Chile
CreditAttribution: lathan commented
FileSize
1830090-2-align-with-drupal-core-null.patch630 bytes

Here is a patch to fix this.

lathan’s picture

lathan
Location Chile
CreditAttribution: lathan commented
Status: Active » Needs review

change status

lathan’s picture

lathan
Location Chile
CreditAttribution: lathan commented
FileSize
1830090-3-align-with-drupal-core-null.patch1.83 KB

I should have also altered the saved values back to NULL, this patch fixes that.

haydeniv’s picture

haydeniv CreditAttribution: haydeniv commented
Status: Needs review » Fixed

Patch looks good to me and everything still appears to work.

Committed to dev commit 22ebcb5.
Thanks!

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Component: Webform » Forms API (Developers)
Status: Fixed » Needs review
FileSize
select_or_other-none_duplication.png9.42 KB
select_or_other-1830090-5.patch1.91 KB

Unfortunately this introduced a number of field widget bugs relating to required fields - see screenshot.

The attached patch simplifies the handling of the _none option a bit and does not add an additional option for required fields.

I think some refactoring could be done here to rely more on FAPI, but I wanted to address the bug first, the refactoring could be done in a separate issue.

select_or_other-none_duplication.png

star-szr’s picture

star-szr
he/him
Primary language English
CreditAttribution: star-szr commented
Component: Forms API (Developers) » CCK / Field API widget

Setting correct component.

haydeniv’s picture

haydeniv CreditAttribution: haydeniv commented
Status: Needs review » Needs work

Blast. That will teach me to get commit happy. Commited #5 commit #354d35f.
Back to needs work for refactoring instead of a followup so we keep the history here.

MrHaroldA’s picture

MrHaroldA CreditAttribution: MrHaroldA commented
FileSize
1830090-illegal_choice_in_validate.patch625 bytes

I found another issue with the Drupal core transition to '_none'. The attached patch fixes the empty string value in select_or_other_field_widget_validate to '_none' to prevent the infamous "An illegal choice has been detected. Please contact the site administrator."-error.

I'm not 100% sure, but I think the value of the "none"-option never should be '' (empty string), but '_none' so the widget will need some extra attention. Keeping it at 'needs work'.

haydeniv’s picture

haydeniv CreditAttribution: haydeniv commented

Looks like there is still some discussion about this on the core side #1585930: Options module uses '_none' as a special value. I'd like to see where they go with that issue before big form handling changes.

mrfelton’s picture

mrfelton CreditAttribution: mrfelton commented
FileSize
1830090-3-align-with-drupal-core-null-followup.patch1.82 KB

I'm still using 7.x-2.15 with the patch from #3 and I dont have time to do a full update now. But, I do need to fix an issue with that patch where numerical sting keys were being converted to integers as opposed to remaining as strings due to use of array_merge(). Here is the updated version of patch in #3

mthomas’s picture

mthomas CreditAttribution: mthomas commented
FileSize
1830090-11-add-none-to-validation.patch842 bytes

My immediate problem with this is that forms are failing to validate because "_none" is being evaluated by select_or_other_field_widget_validate. Excepting "_none" values from validation works for me, patch attached. Any thoughts on this?

Also, do these lines (starting on 358 in dev) serve any purpose? I don't see where $items is set in the function.

      // Filter out 'none' value (if present, will always be in key 0)
      if (isset($items[0]['value']) && $items[0]['value'] === '') {
        unset($items[0]);
      }

  • Commit 22ebcb5 on 7.x-2.x, 7.x-3.x, 8.x-3.x by haydeniv: 
    Issue #1830090 by jucallme: Align with Drupal core select list, NULL (-...
  • Commit 354d35f on 7.x-2.x, 7.x-3.x, 8.x-3.x by haydeniv:
    Issue #1830090 by Cottser: Fix align drupal core null bad commit.
legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo commented
Issue summary: View changes
Status: Needs work » Needs review
FileSize
align_with_drupal_core-1830090-13.patch12.69 KB

Attached patch should fix this properly.

I've removed the custom handling of the empty options and use Drupal's functionality, except for the radiobuttons since Core doesn't provide an empty value for those.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo commented
FileSize
align_with_drupal_core-1830090-14.patch12.19 KB
interdiff.txt662 bytes

Removed unnecessary #empty_option assignment. Drupal core adds this when omitted.

legolasbo queued 14: align_with_drupal_core-1830090-14.patch for re-testing.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo as a volunteer commented
Version: 7.x-2.x-dev » 7.x-3.x-dev

Let's do this refactoring in 3.x

  • legolasbo committed fc5edcb on 7.x-3.x 
    Issue #1830090: Align with Drupal core select list, NULL (-None-) value
legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo as a volunteer commented
Status: Needs review » Fixed

Rewritten empty options from scratch + test coverage.

Status: Fixed » Closed (fixed)

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