-
-
Notifications
You must be signed in to change notification settings - Fork 817
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
APIv4 pseudoconstant improvements #21184
Conversation
(Standard links)
|
329ba9f
to
ad8d834
Compare
8694693
to
5762539
Compare
OK - I just tested updating this This is the part as it related to campaign - @colemanw I see it loaded 2 options when I loaded the updated form
|
@eileenmcnaughton in the unit test with this PR, |
@colemanw I can't figure out how to pretty print link you did - but I'm at this url when I go to update a row I see |
Stop the overly-clever guesswork of abbrColumn so that it can always be know if a pseudoconstant has an abbreviation.
This breaks apart the concept of a field having 'options' vs a field supporting suffixes like campaign_id:label. It is now possible for a field to not have options but still support suffixes. This also makes the available suffixes for each field discoverable, e.g. fields like state_province_id support an :abbr suffix.
5762539
to
b1b7d40
Compare
@eileenmcnaughton that is quite puzzling. I get the same result on the demo site, and also http://core-21184-754mg.test-3.civicrm.org:8001/civicrm/api4#/explorer/Contribution/getFields?where=%5B%5B%22name%22,%22%3D%22,%22campaign_id%22%5D%5D&loadOptions=true returns options for the field when it should say And yet,
I've done a rebase and force-push of this PR to update it to the tip of master. Let's see if the new demo site behaves better. |
@eileenmcnaughton it looks like it was a caching thing. Clearing caches on the demo site fixed it and options are now correctly |
@colemanw I'm really confused about how caching would be an issue on the demo site since each version should be a new build off the code. I replicated the issue AGAIN on the demo sidte just now. I'm open to merging this & keeping testing since it doesn't regress anything & some of the changes are still needed - as long as you'll keep working on it if I keep replicating the options loading |
@eileenmcnaughton I really don't understand it either. But I made the issue go away by logging out of |
@colemanw shall we merge and then re-test on dmaster.demo on Monday - my experience is that it's consistent with different behaviour for different users but getting it merged & waiting a day will rule out caching |
@colemanw I just re-tested on demo.master and the load all IS still happening. I specifically logged on as demo because my suspicion is that it loads differently permission dependent (& once it caches it sticks with it) |
FYI @eileenmcnaughton it's the "Preview" tab. The "Response" tab you're on gives the raw output. |
Fixes a bug in civicrm#21184 which was masked in the test by the lack of campaigns to load. The ?? operator handles FALSE differently to the ?: operator so this was casting FALSE to TRUE
Well spotted @eileenmcnaughton !! |
Fixes a bug in civicrm#21184 which was masked in the test by the lack of campaigns to load. The ?? operator handles FALSE differently to the ?: operator so this was casting FALSE to TRUE
Overview
Makes SearchKit handle large option lists more efficiently, and adds APIv4 field metadata about available suffixes.
Before
:label
,:abbr
, etc) is not discoverable.After
options: false
in getFields.:label
,:abbr
, etc) is discoverable via the new "suffixes" field property.options: false
in getFields.Technical Details
This breaks apart the concept of a field having 'options' vs a field supporting suffixes like
campaign_id:label
.It is now possible for a field to not have options (prefetch = false) but still support suffixes.