Comments (8)
Since this undesired behavior goes all the way down to the DAO, we need to work around the core behavior when passing Api params to the BAO for create/update operations.
One idea is to patch core with a setting/property in the DAO like $dao->_nullMode = TRUE
which disables the special handling of 'null'.
from org.civicrm.api4.
I used a test creating a contact with first_name "Tom" and last_name "Null".
In this test setting, contact last_name to "Null" worked fine. However changing it to "null" resulted in an empty last_name. This is because in CRM_Contact_BAO_Individual::format()
explicitly checks for "null" and sets it to an empty string because:
// "null" value for example is passed by dedupe merge in order to empty.
// Display name computation shouldn't consider such values.
Even after removing this block, this last_name is still set to null. However display name is now set to "Tom null".
Inside the DataObject::insert()
method there is a check for null strings. It is conditional on the disable_null_strings
setting not being set. It only checks if the string is 'null' and allows 'Null'.
It is possible to change the behavior and by adding:
global $_DB_DATAOBJECT;
$_DB_DATAOBJECT['CONFIG']['disable_null_strings'] = FALSE;
anywhere before the call to CRM_Core_DAO::save()
the last_name was stored as string null.
@colemanw what do you suggest doing here? If we want to do this only from api4 then we could set the global 'disable_null_stringsto FALSE before doing an API request and revert it afterwards. It seems a bit hacky though and the issue in
CRM_Contact_BAO_Individual::format()` will still cause problems for contacts. Seems like this issue would be better suited to a core patch, but because of the comments on usage of string null in the dedupe merge we might run into problems in other areas of the code.
from org.civicrm.api4.
I'm in favor of your suggestion to set and unset the global. I agree it's hacky but the alternative is to go down a very deep rabbit hole in core.
As for the "feature" in CRM_Contact_BAO_Individual::format()
I think a refactor in core might be possible but haven't looked at the code. What about adding a param to that function to flag the behavior, and only pass that param as true from the dedupe merger code?
Alternatively, a cheap but effective hack would be to ucfirst()
the string 'null' in a contact name from within api4, and comment the hell out of it.
from org.civicrm.api4.
I've added a toggle function in the Create
class which globally allows insertion of null strings. It's still failing because of the CRM_Contact_BAO_Individual::format()
which actually doesn't affect the last_name, just the display_name, so if I use "Joe null" it will result in "null" for last_name, but just "Joe" for display_name.
It would be nice to understand the reasons for using string null in the dedupe classes. If we could clean this up I think it would be a much nicer option than adding yet another confusing if ($allowNullStrings)
inside the CRM_Contact_BAO_Individual::format()
class.
from org.civicrm.api4.
Regarding updating a value using NULL. This goes down right to the Pear DB_DataObject handling of updates.
If we update an existing value to NULL it will fail because DateObject::update()
checks !isset($this->$field)
before continuing e.g. $this->last_name
, which will be false if it's NULL. If we changed this to update anyway it would also update every other value including those that were not included in the update request. There doesn't seem to be a way to update only certain values without using string nulls.
It would be easy to change NULL to string 'null' in api4, but then we have this situation just from this ticket:
- If creating then string null will be saved
- If updating then string null will result in unsetting the value. True NULL will be converted to string null and also unset the value.
There's no consistency there at all and it would lead to a very frustrating experience (admittedly for a small number of users). I would much prefer to discuss on how to handle this better in core, even if that means using something instead of the PEAR DB_DataObject in the future.
from org.civicrm.api4.
I've implemented a sort of compromise:
- If you set true
NULL
as a value it will be converted to string 'null' so the value will be unset - If you set string 'null' as a value it will be converted to 'Null' since that's the only variation of string null that is allowed.
I've updated the test to reflect this. To summarize:
// create a contact with string null as last name
$contact = Contact::create()
->setValue('first_name', 'Joseph')
->setValue('last_name', 'null')
->setValue('contact_type', 'Individual')
->execute();
// string null is converted to "Null"
$this->assertSame('Null', $contact['last_name']);
$this->assertSame('Joseph Null', $contact['display_name']);
// set the last name to true NULL
$contact = Contact::create()
->setValue('id', $contactId)
->setValue('last_name', NULL)
->execute();
// last name is unset, display name is updated
$this->assertSame(NULL, $contact['last_name']);
$this->assertSame('Joseph', $contact['display_name']);
from org.civicrm.api4.
@mickadoo ok is there a PR for that?
from org.civicrm.api4.
@colemanw sure, it's here. I didn't want to create it while another one was open. I wanted to use the structure I created in the other PR to handle the null values. If you want you can review the other one first and I can update this one with those changes. If you do this one first it's sort of like reviewing both together.
from org.civicrm.api4.
Related Issues (20)
- What is the deal with ad hoc actions HOT 5
- apiv4 regression HOT 4
- Joining on entity_tag doesn't work HOT 2
- Activity 'Date' Custom Field ignores the time HOT 1
- Documentation link - what should it be
- Contact::update() clears contact subtype HOT 2
- Error when traying to retrieve participants / gender
- REST.php not working in 5.13.0 HOT 2
- Relationship.Update gives error 500 if there are no records to update
- Bulk create/update action HOT 2
- Support custom date ranges
- Can't join contribution entity to contacts HOT 5
- Mapping api does not support a name in mapping_type_id HOT 2
- ORDER BY some_custom_field not working
- Organize next release of APIv4 HOT 12
- Can we 'removeWhere' HOT 1
- Authorization failed for anoymous WordPress User HOT 6
- "IN" operator for addWhere() not returning data HOT 3
- Keep getting errors "DB Error: no such field"
- It should be noted someplace here and on the civi extensions page that this has been rolled into core with 5.19 HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from org.civicrm.api4.