webform_civicrm icon indicating copy to clipboard operation
webform_civicrm copied to clipboard

#3095675 Show Line Item in same order as form field are configured

Open sunilpawar opened this issue 5 years ago • 5 comments

Overview

https://www.drupal.org/project/webform_civicrm/issues/3095675

Before

Line Item table does not follow weight flag of field

After

Line Item table does follow weight flag of field

Technical Details

Line Item table follow weight flag of field

sunilpawar avatar Nov 20 '19 13:11 sunilpawar

Hi @sunilpawar - thanks for this. Will definitively take a look in near future.

KarinG avatar Nov 20 '19 16:11 KarinG

@sunilpawar - I just tested this and ran into some issues. Here's my Webform:

image

And these are my Line Items afterwards with this PR 278: image

Please note -> not only is the order not the same as on the Webform (which is what this PR is attempting), note that Line Item Amount 2 is missing.

If I git stash -> to before 278.diff -> Line Item Amount 2 is there: image

Note I've not yet tested multiple Participants or multiple Memberships.

Conclusion: this would certainly be nice but in order to make this mergeable (after fixing the ordering and after handling multiple line items) please show some testing with (in addition to what I just did) multiple Memberships and multiple Participants as well.

KarinG avatar Jan 05 '20 01:01 KarinG

PS - sorry for the delay on reviewing this. I'm finally catching up.

KarinG avatar Jan 05 '20 01:01 KarinG

@KarinG thanks for reviewing PR, i will check multiple scenario and get back to you with additional fix.

sunilpawar avatar Jan 05 '20 04:01 sunilpawar

Looking at the code here, I have a suggestion. Although it's very clever to index the array by component weight and then ksort() at the end, it results in overall more complex code, as the $formFieldOrder variable has to be used everywhere a line item is added to the array. Seems to make the code harder to read & understand.

I would suggest that instead, you index the line-item array by $fid and then add a utility function to do the reordering at the end. That function would look up component weights by $fid and then ksort. That function should also involve a fallback in case the component is undefined.

To prevent items disappearing, you could also suffix the array keys like [$fid . '-a'] and [$fid . '-b']. The utility function would just need to be smart enough to handle that.

colemanw avatar Jan 10 '20 17:01 colemanw