-
-
Notifications
You must be signed in to change notification settings - Fork 824
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
dev/core#2486 Add survey v4 api #21355
Conversation
(Standard links)
|
@@ -214,6 +214,13 @@ | |||
<default>NULL</default> | |||
<comment>Used to store option group id.</comment> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eileenmcnaughton I'm pretty sure the comment here is incorrect. This field appears to be used to store the option value of the option group configured for this survey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@colemanw I created a survey & got a value of 106 in that field - it correlated to the option group id - then when you phone people up you create activities & they have a field (result?) that maps to the option value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, no, wait, I get it. This stores the option_group_id of the option group being displayed on this form.
xml/schema/Campaign/Survey.xml
Outdated
<keyColumn>id</keyColumn> | ||
<labelColumn>title</labelColumn> | ||
<nameColumn>name</nameColumn> | ||
<prefetch>FALSE</prefetch> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked the core code and it uses a pretty dumb hack which we might as well do here too (and then no need for disabling prefetch):
<prefetch>FALSE</prefetch> | |
<condition>name LIKE "civicrm_survey_%"</condition> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the dumb hack I'm referring to:
civicrm-core/CRM/Campaign/BAO/Survey.php
Line 694 in 0419bf7
$query = "SELECT id, {$valueColumnName} FROM civicrm_option_group WHERE name LIKE 'civicrm_survey_%' AND is_active=1"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@colemanw ok - I've made that change
534bb70
to
af0d884
Compare
af0d884
to
223671d
Compare
This caused singleValueAlter test to start failing - I just added Survey to the skip line. That test has gotten really slow & doesn't add the value it did when our code was more chaotic |
Overview
dev/core#2486 Add survey v4 api
Before
Survey api missing for v4
After
Tada
Technical Details
@colemanw I created this to help with testing out your patch. I defined the option group pseudoconstant but it is pretty fugly & I don't know it helps much since with it configured I get this ( I think that is without prefetch set to false).
I also notice there is one html field that is rendered as html - ideally we would detect that & not show it in all it's taggy glory.
Given this is a barely used api I don't want to put too much into it - it's just a question of whether I do or don't add a not-that-great pseudoconstant on the result fields at this point
I guess tests will fail
Comments