fix stars/notes functionality when country_id is in submission #959
Conversation
MegaphoneJon
force-pushed
the
fix-ajax-submission-edits
branch
2 times, most recently
from
19b20e5 to
3ca416a
Compare
|
That test fail on dev-master looks unrelated, but let me know if not. This fixes using the "stars" and "notes" function on submissions that have WFC and certain option IDs, in my case
[
'civicrm_1_contact_1_first_name' => 'Jon',
'civicrm_1_contact_1_last_name' => 'Goldberg',
'civicrm_1_contact_1_address_country_id' => '1228',
'civicrm_1_contact_1_gender_id' => '2',
]Before data is saved to Civi, this will return: Once the data is saved to Civi, it will be this: Rendering tokens uses both |
MegaphoneJon
force-pushed
the
fix-ajax-submission-edits
branch
from
3ca416a to
1bcfdf2
Compare
|
ok I was able to replicate this with
Applied the patch
But state field is returning id instead of name. 
It displays the name correctly before the patch. |
|
Thanks @jitendrapurohit. I'll write tests for this. I wasn't seeing this in my testing but that probably means there's a workflow I'm not considering. |
|
@jitendrapurohit I wasn't able to replicate this issue, and in fact in testing I found that the patch fixes a different bug. Below is my form (apologies it's so long, the clientside_validation module is at fault). I submitted data, and it's rendering correctly on the "results" page. Additionally, if I click Edit next to the result, the state/province displays. It comes up blank without this patch.
|
|
@jitendrapurohit Hold for now - someone may have a place where the issue you describe is replicable. I'll report in soon. |
|
@jitendrapurohit The report I received dealt with a custom module that does Webform exports without implementing the |
|
Mine is a simple webform with just Name + Address fields. Replicating the issues doesn't take any special steps. Just submit the form after applying the PR and check the view results page. The state is rendered as id and not name. Attached the exported file of the webform here - webform.webform.test_tokens.yml.txt Looks like after this PR, The following patch works for me (applied on top of this PR) diff --git a/webform_civicrm.module b/webform_civicrm.module
index 1befb50..e7cc4b4 100644
--- a/webform_civicrm.module
+++ b/webform_civicrm.module
@@ -177,7 +177,12 @@ function webform_civicrm_theme() {
*/
function webform_civicrm_webform_submission_load($entities) {
foreach ($entities as $entity) {
- $data = _fillCiviCRMData($entity->getData(), $entity);
+ $data = $entity->getData();
+ // Replace tokens if we're not editing
+ if (\Drupal::routeMatch()->getRouteName() == 'entity.webform.results_submissions') {
+ $data = _civiTokenReplacement($data, $entity);
+ }
+ $data = _fillCiviCRMData($data, $entity);
$entity->setData($data);
}
}
@@ -285,7 +290,9 @@ function webform_civicrm_webform_submission_view_alter(array &$build, WebformSub
$webform = $submission->getWebform();
$config = $webform->getHandlers('webform_civicrm')->getConfiguration();
if (!empty($config['webform_civicrm']) && !empty($submission->getData()['civicrm']) && \Drupal::currentUser()->hasPermission('access CiviCRM')) {
- $data = $submission->getData()['civicrm'];
+ $data = _civiTokenReplacement($submission->getData(), $submission);
+ $data = _fillCiviCRMData($data, $submission);
+ $submission->setData($data);
$links = [];
$entity_links = [
'contact' => 'contact/view',Does this make sense? If yes, perhaps we can merge the 2 functions |
@MegaphoneJon We already have a test to verify the group labels here - https://github.com/colemanw/webform_civicrm/blob/6.x/tests/src/FunctionalJavascript/GroupsTagsSubmissionTest.php#L155. It might be easy to extend and include state fields too, perhaps? |
MegaphoneJon
force-pushed
the
fix-ajax-submission-edits
branch
2 times, most recently
from
3262ad6 to
cf05fe2
Compare
|
@jitendrapurohit In just incorporated your suggestion, thank you. I definitely recommend enabling the |
MegaphoneJon
force-pushed
the
fix-ajax-submission-edits
branch
from
cf05fe2 to
a0370ea
Compare
|
Thanks @MegaphoneJon for the update and test. I've retested this & looks like there are still issues with the display of state field value -
I think we can merge this since it fixes the original star/note issue and can fix the above in a follow up PR with few more tests. |
|
Ah, just confirmed the above id issue is already fixed via #941. After that is merged, i think we're good. |
|
Apologies for commenting on a merged/closed issue but while debugging my own issue reported here: https://www.drupal.org/project/webform_civicrm/issues/3462311 ,I've stepped over this one which resembles what I am facing: Upon loading a draft, my values are not numeric anymore, they're becoming strings. This affects so far country_id, state_province_id and all the customfields that are being exposed on the webform and are of type optionvalue (select/radio etc). |
|
@VangelisP That issue you reported says you're using version 6.2.5, but this PR isn't in a released version yet. I think #941 is closer to what you're looking for - maybe try applying that (and this?) as a patch? |
|
You are absolutely right! I must have confused that and missed 941. My apologies, I'll leave this issue as-is. |
Overview
@jitendrapurohit removed this from #880, and it failed tests. But I've found a bug caused by this code.
fillCiviCRMData()is called when:We thought the latter two were unnecessary, since Webform calls the token renderer when loading this page. So we removed it, and it failed a test. But in the third case, it's called before the data is saved, so for any option value fields, we pass the label as the ID.
Many fields will accept this.
country_idwill not. So you get a 500 error on any AJAX webform submission stars/notes when the country is present.I'm submitting this so I can see the exact tests that are failing and see if there's a resolution.