Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add serialize metadata to tag used_for field #14096

Merged
merged 2 commits into from
Apr 21, 2019
Merged

Conversation

colemanw
Copy link
Member

Overview

Add schema metadata to the Tag "used_for" field, which is a comma separated list.

Before

Less metadata.

After

More metadata

@civibot
Copy link

civibot bot commented Apr 21, 2019

(Standard links)

@civibot civibot bot added the master label Apr 21, 2019
@colemanw colemanw changed the title Add serialize info to tag used_for field Add serialize metadata to tag used_for field Apr 21, 2019
The used_for field is a comma separated list.
@eileenmcnaughton
Copy link
Contributor

@colemanw the syntaxconformance test might need to be smarter about what data to put in that field to pass

@colemanw
Copy link
Member Author

colemanw commented Apr 21, 2019

So Eileen, according to the test failure, the testCreateSingleValueAlter() function is setting the field's value to 5 which is actually the constant CRM_Core_DAO::SERIALIZE_COMMA. This is due to this line of code which doesn't make sense to me - it is actually setting the value of $field['serialize'] (in this case 5) as the value of the field:

https://github.com/civicrm/civicrm-core/blob/5.13/tests/phpunit/api/v3/SyntaxConformanceTest.php#L1502-L1504

  if (!empty($specs['serialize'])) {
    $updateParams[$field] = $entity[$field] = (array) $specs['serialize'];
  }

@colemanw
Copy link
Member Author

Ok removing those lines fixed the test.

@colemanw colemanw merged commit 223dd31 into civicrm:master Apr 21, 2019
@colemanw colemanw deleted the tags branch April 22, 2019 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants