Make savedSearch bao sane

[colemanw]

Overview

Makes the savedSearch create function act in a more standard way so that tests pass and the APIv4 can use it.

Before

BAO create would overwrite values on update for no reason I can fathom. It also tries to save a non-existent field (is_active was removed in v1.5!) which doesn't give me much confidence that it was doing these things for a good reason.

After

Make it act like a normal bao create function.

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[eileenmcnaughton]

I think we should add an api alias to the v3 api so that formValues still works - since that's how we roll with v3 - it's just adding to the spec

$spec['form_values']['api.aliases'] = ['formValues'];

& the test would be that the above change would not be needed - acutally that's only true if the above calls apiv3 but still

[colemanw]

@eileenmcnaughton I've added the api alias although formValues was never officially supported by the api; the api expected form_values, so I've just pushed that expectation down to the BAO as well.

[eileenmcnaughton]

cool

[eileenmcnaughton]

@colemanw test fails relate - I think the field serialization can be defined in he xml

[colemanw]

@colemanw test fails relate - I think the field serialization can be defined in he xml

Oh cool. Right, if I pass TRUE as the 2nd param to copyValues() then it takes care of it automatically.

[eileenmcnaughton]

@colemanw am I right in thinking that the idea of saving an api savedSearch is tangental to api smart groups - in the sense that they might be leveraged by smart groups but we would also be able to save an api search WITHOUT a smart group - e.g for an entity that does not relate to contacts. I don't have a specific use case but it feels more logical & powerful if 'any' api search can be saved

[eileenmcnaughton]

This looks generally good - once tests are passing I'll check it more carefully but it looks like the right changes to clean this up to me

[eileenmcnaughton]

This function is only ever called from the unit tests! We could probably move it over to the test class

[eileenmcnaughton]

I was a bit mystified why there seem to be so few calls to this create function - it seems the code is here

civicrm-core/CRM/Contact/Form/Task/SaveSearch.php

Lines 166 to 189 in 34634ee

	//save the search
	$savedSearch = new CRM_Contact_BAO_SavedSearch();
	$savedSearch->id = $this->_id;
	$queryParams = $this->get('queryParams');
	// Use the query parameters rather than the form values - these have already been assessed / converted
	// with the extra knowledge that the form has.
	// Note that we want to move towards a standardised way of saving the query that is not
	// an exact match for the form requirements & task the form layer with converting backwards and forwards.
	// Ideally per CRM-17075 we will use entity reference fields heavily in the form layer & convert to the
	// sql operator syntax at the query layer.
	if (!$isSearchBuilder) {
	CRM_Contact_BAO_SavedSearch::saveSkippedElement($queryParams, $formValues);
	$savedSearch->form_values = serialize($queryParams);
	}
	else {
	// We want search builder to be able to convert back & forth at the form layer
	// to a standardised style - but it can't yet!
	$savedSearch->form_values = serialize($formValues);
	}
	$savedSearch->mapping_id = $mappingId;
	$savedSearch->search_custom_id = $this->get('customSearchID');
	$savedSearch->save();

Maybe once we have the v4 api for saved search maybe we can update the form to use it!

[eileenmcnaughton]

So my digging says the BAO function is only called by the tests in core! I think this is good to merge if the tests pass this time but we should do some follow ups