Problem/Motivation

Its not obvious that option list machine names starting with likert_ are available for the likert element

Proposed resolution

Add a boolean flag to option lists 'use as likert' that allows the likert element to use an entity query instead

Remaining tasks

User interface changes

API changes

Data model changes

CommentFileSizeAuthor
#10 2931888-likert-options-10.patch19.43 KBjrockowitz
#7 2931888-likert-options-7.patch19.4 KBjrockowitz
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan created an issue. See original summary.

jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented

Maybe we need to add a 'likert' property to the WebformOptions entity and a checkbox to the WebformOptionsEntityForm that make options available to Likert elements.

Tasks

  • Add likert boolean WebformOptions
  • Update \Drupal\webform\WebformOptionsForm
  • Add note that 'Likert: ' prefix will be removed.
  • Add likert column to \Drupal\webform\WebformOptionsListBuilder
  • Use EntityQuery for \Drupal\webform\WebformOptionsStorage::getLikerts
  • Write update hook.

Notes

  • We should still remove 'Likert: ' prefix.
  • Probably does not need any new test coverage.
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Right, that would make it more explicit and less magic - agree

jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented
Assigned: Unassigned » jrockowitz

Yes, it is time to remove some of the 'magic'. It is not a lot of work.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Title: Document in the options form that machine names that start with likert_ are available for the likert element » Add a boolean flag 'use as likert' for options list and remove the machine name pattern matching in likert element
Assigned: jrockowitz » Unassigned
Issue summary: View changes

Thanks, new issue title.

Ping me for a review (here is fine)

  • jrockowitz committed 0a243c2 on 2931888-likert-options 
    Issue #2931888: Add a boolean flag 'use as likert' for options list and...
jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented
Status: Active » Needs review
FileSize
2931888-likert-options-7.patch19.4 KB

One or two tests might fail and need to updated.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented 
  1. +++ b/config/install/webform.webform_options.languages.yml
--- a/config/install/webform.webform_options.likert_agreement.yml
+++ b/config/install/webform.webform_options.likert_agreement.yml

    missed the flag here? 

  2. +++ b/src/Utility/WebformArrayHelper.php
@@ -332,4 +332,50 @@ class WebformArrayHelper {
+  public static function insertBefore(array &$array, $target_key, $new_key, $new_value) {
...
+  public static function insertAfter(array &$array, $target_key, $new_key, $new_value) {

    is this needed, I think the config schema dictates this, but either way - is order that important? 

  3. +++ b/src/WebformOptionsForm.php
@@ -76,6 +76,13 @@ class WebformOptionsForm extends EntityForm {
+      '#default_value' => $webform_options->get('likert'),

+++ b/src/WebformOptionsListBuilder.php
@@ -61,6 +62,7 @@ class WebformOptionsListBuilder extends ConfigEntityListBuilder {
+    $row['likert'] = $entity->get('likert') ? $this->t('Yes') : $this->t('No');

    can use the isLikert method

  • jrockowitz committed 81e9255 on 2931888-likert-options 
    Issue #2931888: Add a boolean flag 'use as likert' for options list and...
jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented
FileSize
2931888-likert-options-10.patch19.43 KB

1. and 3. Fixed!

2. I like to maintain a consistent order for exported config because it makes it easier to scan. I also wanted these array helper functions available.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

Do you normally add update path tests to the module?

jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented

I have not been adding update path tests. Once Webform module has an RC and stable release, I am planning on increasing test coverage and maintaining 'Change records'.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented

no worries, makes sense

this looks good to me then, thanks

  • jrockowitz committed 524cb0e on 8.x-5.x 
    Issue #2931888 by jrockowitz: Add a boolean flag 'use as likert' for...
jrockowitz’s picture

jrockowitz CreditAttribution: jrockowitz as a volunteer and at The Big Blue House commented
Status: Needs review » Fixed

I like to say that "I am rounding a few corners"

Status: Fixed » Closed (fixed)

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