Getting ready to upgrade and just wondering.

For instance, I installed the CiviCRM core patch, CustomValueTable.patch, to 4.0.6. Will that also be needed in 4.0.7, or has it been rolled in? Anything else to be aware of re: Webform processing?

BTW, the GUI on the newest .dev is really awesome!

Comments

colemanw’s picture

colemanw commented
Status: Active » Fixed

Glad you like it! Be sure to check out the new state/province dynamic form element that loads options based on chosen country.

From this module's pov, 4.0.6 and 4.0.7 are the same. So any patch you applied to one will need re-applying to the other. The fixes aren't scheduled for release until 4.1.

m.e.’s picture

m.e. commented

OK, all updated and so far so good.

I haven't reapplied the patch yet and, oddly, I am not seeing a need for it. Maybe my testing is insufficient? I have blanked out several optional fields (both core fields like middle name and custom ones involving checkboxes) and the removal of the previously stored data seems to be holding. I still can't blank out a phone field, but this was also the case with the patch applied.

(I did save the Contact 1 tab for the form I was testing -- not sure if this was necessary or not.)

Any idea what this means? (I am on the .dev version from just before the security update discussed in the past couple days' thread.)

colemanw’s picture

colemanw commented
Title: Anything to consider before upgrading CiviCRM 4.0.6 > 4.0.7? » Work on how we delete data from Civi
Category: support » task
Status: Fixed » Active

You know, the more we discuss it, the more I realize how complex deleting existing data is. The "expected behavior" is going to be different based on the circumstances. Three scenarios for a contact are:

  1. They view the form as anonymous, and a new contact is created when they submit it.
  2. They view the form as a known user (logged in or hashed link), the form was auto-filled for them, and their contact record gets updated.
  3. They view the form as anonymous (so it wasn't auto-filled), but when they submit the form their info is matched to an existing contact (via civi's strict dedupe method) and that record is updated.

Currently this module treats all of those scenarios the same way, but they are pretty different in terms of what you want to happen.
In the first scenario it doesn't matter what happens with blank fields; since the contact is new there's no existing data to delete.
In the second, blank fields should cause a delete in the civi db. Since the form was auto-filled, you would have had to clear the field manually, so we assume you did it on purpose.
In the third, blank fields should not cause a delete in the civi db. Since the form was not auto-filled, we can't be sure whether you were trying to erase data when you left something blank, or were simply too lazy to fill it out.

At the moment, this module doesn't distinguish between these latter two situations.
Also at the moment, there is an inconsistency between how core and custom fields are handled.

  • Blank custom fields will always cause a delete
  • Blank core fields will cause a delete too... except
  • If all fields of a particular location (address, email, phone, or website) are left blank, it will get ignored (so no delete)

So it looks like I've got a job to do cleaning up the way this is all treated and getting the expected behavior to happen consistently across all types of fields. I'll try to get this coded and into -dev in the next couple weeks. If you'd be available to do a little testing and q/c, that would be wonderful (testing really helps speed things along to get dev code into a release).

m.e.’s picture

m.e. commented

Your 3 scenarios sound about right to me. I'll be happy to do whatever testing I can help with. My current forms are all of type 2, but if you don't have anyone testing situations 1 and 3, I can probably come up with a way to do that since the site hasn't yet launched.

colemanw’s picture

colemanw commented

OK, the new code for this has been committed to both branches. The new behavior is:

  • Blank fields will be ignored if the contact is unknown, so any existing data will stay as-is for that field.
  • Blank fields will be saved as blank if the contact is known, overwriting existing data.
  • Additionally, an entirely blank location (email, address, phone, website) will be completely deleted if the contact is known.

Testing needed!
Please test and post results here. Version 2.2 is shaping up, but I won't publish it until I get some solid feedback here by people testing the latest -dev.

m.e.’s picture

m.e. commented

Haven't had much time on my site this week, but getting ready to update and test this new code now.

Also - above I said I was getting by without the CustomValueTable patch after upgrading CiviCRM, but although blanking out a previously populated field did seem to be working without the patch, my money fields have stopped updating as was the case some time ago.

Can you refresh my memory - was this one of the problems remedied by this patch? I tried to implement it just now and got the error, "Hunk #1 FAILED at 494.
1 out of 1 hunk FAILED -- saving rejects to file civicrm/CRM/Core/BAO/CustomValueTable.php.rej"

The content of the .rej file is this:

*** CRM/Core/BAO/CustomValueTable.php	2011-08-11 02:25:10.000000000 -0700
--- civicrm/CRM/Core/BAO/CustomValueTable.php	2011-09-30 21:17:57.224456596 -0700
***************
*** 494,503 ****
          $cvParams  = array( );
  
          while ( $dao->fetch( ) ) {
-             // ensure that value is of the right data type
              $dataType = $dao->data_type == 'Date' ? 'Timestamp' : $dao->data_type;
              foreach ( $fieldValues[$dao->cf_id] as $fieldValue ) {
!                 if ( CRM_Utils_Type::escape( $fieldValue['value'],
                                               $dataType, false ) === null ) {
                      return CRM_Core_Error::createAPIError( ts( 'value: %1 is not of the right field data type: %2',
                                                                 array( 1 => $fieldValue['value'],
--- 494,523 ----
          $cvParams  = array( );
  
          while ( $dao->fetch( ) ) {
              $dataType = $dao->data_type == 'Date' ? 'Timestamp' : $dao->data_type;
              foreach ( $fieldValues[$dao->cf_id] as $fieldValue ) {
!                 // Format null values correctly
!                 if ( $fieldValue['value'] === null || $fieldValue['value'] === '' ) {
!                   switch ( $dataType ) {
!                     case 'String':
!                     case 'Int':
!                     case 'Link':
!                     case 'Boolean':
!                       $fieldValue['value'] = '';
!                       break;
!                     case 'Timestamp':
!                       $fieldValue['value'] = null;
!                       break;
!                     case 'StateProvince':
!                     case 'Country':
!                     case 'Money':
!                     case 'Float':
!                       $fieldValue['value'] = (int) 0;
!                       break;
!                   }
!                 }
!                 // Ensure that value is of the right data type
!                 elseif ( CRM_Utils_Type::escape( $fieldValue['value'],
                                               $dataType, false ) === null ) {
                      return CRM_Core_Error::createAPIError( ts( 'value: %1 is not of the right field data type: %2',
                                                                 array( 1 => $fieldValue['value'],
m.e.’s picture

m.e. commented

Update: the money problem above was resolved by the latest -dev, so I'm assuming the patch is indeed unnecessary.

I can blank out optional fields, both custom and core (phone, at least), for an authenticated user, and the change is carried through to CiviCRM and the next reload of the webform.

So the new version is doing well for me. I haven't tested an unknown contact, though - my forms are all for authenticated users.

There is one odd behavior/bug that I wasn't seeing before the upgrade, but in case it's specific to my site, I'll write to you offlist about it.

saparker’s picture

saparker commented

I've tried the new dev version with updating a known contact using an anonymous form (scenario 3 in comment #3 above). This is now much better as it doesn't update fields that are left blank, but rather leaves them at their present value. (although this doesn't seem to be how it was written in comment #5 as "Blank fields will be saved as blank if the contact is known, overwriting existing data.")

The only exception seems to be for multi-select fields (eg "Privacy" or "Preferred Method(s)" for a contact). In this situation, if the form is completed without any option being ticked, then it will update the setting to 'no boxes ticked'. This isn't exactly what I'd imagined (because someone who'd just not bothered to fill out the field would erase their previous setting), but then I think it has to be this way or there would be no way of deselecting all the options.

The updating seems to be consistent across core and custom fields.

So in summary, I think that this now works very well for updating a known contact using an anonymous form. Thanks colemanw.

colemanw’s picture

colemanw commented

Thanks very much for the testing. Just to clarify the terminology as I've been using it...
"Known" means the contact ID is known before the form is submitted (so the form was auto-filled).
"Unknown" means the user was anonymous, and the form was not auto-filled. After the form is submitted, the CID will be determined either by matching to an existing contact, or by creating a new one.

saparker’s picture

saparker commented

Ok, fine. Using your terminology, updating an unknown contact using an anonymous form seems to work well and is in line with the behaviour specified in comment #5

colemanw’s picture

colemanw commented
Status: Active » Fixed

Great, thanks again. I think we can mark this one as fixed.
Note that these improvements did not make it into 2.1, but will be in 2.2.

Status: Fixed » Closed (fixed)

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