Problem/Motivation
When exporting views using Graph API, only the most basic settings are included (the to/from mapping). This is a problem in particular if you want to export your views and push them in code to another site, for example by using Features.
Proposed resolution
Allow Graph API to declare all the necessary Views settings in the option_definition() method in the style plugin – also fetching settings from the graphing engine being used.
Remaining tasks
Do an independent test of the patch submitted below, by creating a view on one site, exporting it, and importing it on another site.
User interface changes
(none)
API changes
For graphing engines to make use of the support for export, their engine settings in the configuration form should be stored on the form $form[ENGINE_NAME][ELEMENT].
This is a sane approach, and the engines I've seen do this already.
| Comment | File | Size | Author |
|---|---|---|---|
| #26 | 1513198-26-export_graphapi_engine_settings.patch | 421 bytes | itangalo |
| #10 | 1513198-10-export_graph_engine_settings_in_views.patch | 2.28 KB | itangalo |
| #2 | 1513198-1-views_export_of_all_settings.patch | 1.52 KB | itangalo |
| #1 | 1513198-1-views_export_of_all_settings.patch | 1.52 KB | itangalo |
Comments
Comment #1
itangalo commentedPatch attached.
Comment #2
itangalo commentedSomething really weird just happened. Sorry if this becomes a double post.
Comment #3
clemens.tolboomThanks for reporting this issue. I consider this a bug though.
The suggested patch is not how I would solve this as invoking the form to extract the default values seems wrong.
We need to add a new hook for this I guess. What do you think?
One question regarding your patch is:
Why only the selected engines settings? All default settings are needed when _configuring_ the display format. It is a little awkward though to have all settings available when _exporting_ the view I guess but I'm a little confused how views-export operates.
Comment #4
itangalo commentedI agree – it makes more sense to have a hook that defines the settings to export. The solution I made works with the engines I've seen, which was why I chose it, but I think it is worth changing the API to allow explicit declarations of options to export.
That I filtered out only the used engine was, as you suspect, to minimize the exported code. I don't see when non-used engines would require configuration (and if so, they should provide default settings) but it might still be a good idea to export them. A few extra bytes are cheap. :-)
Comment #5
clemens.tolboomI pushed graphapi.api.php with the hooks including this new one. Feel free to patch the graphapi.api.php file ;)
I try to fix this new hook later this week.
Comment #6
itangalo commentedSweet! I'll patch The JIT first, since I use one of its engines in a project I want to launch. But patching .api.php at the same time won't be much extra effort. (Not today, though.)
Thanks for quick response!
Comment #7
clemens.tolboomWhat is the issue # for the jit? If none please create one ... as I need to apply that patch too.
Comment #8
itangalo commentedIssue just created! #1513462: Implement new API from Graph API to allow exporting engine settings
Comment #9
clemens.tolboomThis issue is still open. I just updated graphapi.api.php which links back to this issue.
Both @Itangalo and @clemens.tolboom plan to work on both thejit and graphapi the next (few) weeks.
Comment #10
itangalo commentedPatch attached, which updates graphapi.api.php and also calls the above described hook.
Comment #11
itangalo commentedA thought: Is it really desired to export the style settings even for the engine you are not using? It means more settings that might conflict and cause overridden features, and as far as I can see no real gain.
(Maaaybe there's a gain if you dynamically change the engine in one of View's hooks – so you would need to export the settings from more engines than the ones selected in the Views config form.)
Comment #12
clemens.tolboomThe way I grasp option_definition
is to provide placeholders for views to map it's value on. If one misses and named option the are not used. Thus we cannot drop possible settings as the choice of the engine is done through the settings in one go.
It is indeed not nice and even confusing when staging as on dev more engines could be enabled.
So on must disable unused engine before exporting / featurize :(
Patch looks good ... I try to apply friday
Comment #13
clemens.tolboomAccording to http://api.drupal.org/api/views/includes!base.inc/function/views_object%... we are not doing this ok.
We need to have
I'm working/testing this right now.
Comment #14
clemens.tolboomSteps I do:
Comment #15
clemens.tolboomI committed the adjusted patch from #10 with the comments from #13
Pushed on master(!?!) : acff8ef..903b796
Comment #16
clemens.tolboomThanks @Itangalo
Comment #17
itangalo commentedAs I interpret the 'contains' parameter, it can be used for storing *additional* settings in an option – for example if you have more settings for (say) default argument values or something. The Graph API settings are not this deep, so just using 'default' should work fine.
I *think* it should still work with the 'contains' key in between, but I am damn sure I right now can't export all the settings I need. I vote for reverting to the older patch, based on the old proof-of-the-pudding argument.
Comment #18
clemens.tolboomPlease tell me what your settings and steps are.
I exported the view manually so see all settings are in place as expected.
Comment #19
itangalo commentedI'm using the JIT module, to test the hook that Graph API provides. Steps to reproduce:
* Enable required modules (including JIT spacetree and Graph API in latest dev).
* Create a new view. Add uid and nid fields, just to have something that Graph API can use for mapping.
* Change style to graph rendering, selecting spacetree – theJIT. Set nid as source fields and uid as target fields.
* Save the view.
* Export the view.
-> The only parts of the Graph settings exported are:
(Basically all settings provided by thejit_spacetree are missing, as are the global width, height and background color settings.)
Comment #20
clemens.tolboomI think views think of defaults differently then we do right now.
Can you test by changing your global settings for spacetree then export. Or change a lot of settings for the view.
I've seen a lot of settings getting exported.
What we had was an array with values which is not views intention to export.
Comment #21
clemens.tolboomBtw .. I added to many ['contains'] as I want to add translatable later on.
Comment #22
itangalo commentedTried changing basically every setting in thejit_spacetree, and *some* of the settings are definitely picked up – but not all of them.
The enable_full_screen and enable_hiding settings are being exported when enabled, but for example not enable_node_info or the orientation settings – which is not even stored when saving the view. The global settings are stored, but not exported.
(I'm no expert on Views, but I think the 'default' value for the declared options are used when setting up unconfigured options – either for export or for new views.)
Comment #23
clemens.tolboomI pushed #1528540: Make a demo page for all engines. so we maybe see better what's wrong.
Comment #24
clemens.tolboomI've fixed the global settings for height, width and background-color.
(commit hash: eb8d9a9..288d0bc)
I'll commit the same resolution for thejit modules but I'm still puzzling with those settings. Esp. why some settings are exported.
The way I see views defaults are indeed when values are missing. I guess that's a problem when staging a view where ie width is not exported. Maybe you could check the ctools / views / features issue queue for this.
Comment #25
itangalo commentedIt just occured to me that the 'default' thing might *not* be the default settings to export – but the value to compare against to see if there is anything to export. I'll have a closer look at it tomorrow, and will try setting all default values to NULL.
Comment #26
itangalo commentedI can't really explain why, but everything works just fine when options are declared directly in ['default'] instead of in ['contains']. My best guess is that the answer to this lies hidden within (possibly lacking) documentation of the Views module.
In any case – it seems clear to me that the option definitions should be stored without the ['contains'] key. Attached is a patch changing this.
With this small change, all exports show up nicely. (Nice abstraction of the option definitions, by the way! Being able to change *all* settings declarations in just one line is sweet.)
Comment #27
clemens.tolboom@Itangalo: What is the export line for options when leaving out the 'contains'?
From #drupal-views @ irc : dawehner suggest not to use values when generating the form. Let views handle that.
This does not solve the why aren't the other values exported right?
Comment #28
itangalo commentedThe exported lines reads like this (and seems to capture every required setting):
I'm not sure, but I think you might be misinterpreting the options_definitions() method and have it read the actual settings from the view. It only needs to *declare* which settings are present, and then Views takes care of fetching and exporting those settings.
Comment #29
clemens.tolboomYeah ... that does not look like a normal export. At least to me. Ie diff.module will have a hard time and other parts of the export don't contain array structures.
One would expect similar structure for similar data. These outputs
are not similar. I'm not sure what's wrong but ie width is now exported ok.
My guess now is the options maybe misaligned when export is checking for the values somehow so export only sees default values thus does not export? I'll try to debug the export code tomorrow as I noticed the option_definition is called a zillion times when generating an export which is bad too.
Comment #30
clemens.tolboomI'm progressing a little. Debugging function option_definition() into views_plugin_style then into base.inc I found:
- Booleans are treated differently.
We do have a lot of booleans which are treated as number but should be defined with
- our defaults are not views defaults
Our defaults for graph_phyz / thejit_* are configurable by the site builders so are strickly no defaults.
Zapping all defaults to NULL leads to exporting more settings. But this still misses the booleans.
I'll try to refactor graph_phyz to fit this scheme using boolean first.
Comment #31
clemens.tolboomI've created two views patches:
(views) #1532986: base.inc views_object:option_definition() documentation is outdated.
(views) #1532216: option_definition is called oh so many times
to learn we can add a special handler when exporting to dump all values. I'm not in favor of doing this but it is as simple as calling
views_var_export($var, $prefix)Each engine must have default values in _code_, These values can then be used when configuring engine settings using views.
Changing the global settings for an engine should not be used for the defaults when using views.
I'm trying to fix graph_phyz first then see what's best.
Comment #32
itangalo commentedJust wanted to say thanks for working on this.
Comment #33
clemens.tolboomComment #34
clemens.tolboomThis needs to get fixed. Period :-/
Comment #35
clemens.tolboomWe have two commits done in the past
Comment #36
clemens.tolboomHmmm ... I don't get it anymore.
Building a view part of the export contains engine specific values like
So graph_phyz seems doing well. But thejit is not.
Comment #37
clemens.tolboomI need some user feedback. This issue has 5 followers.
Please help fixing this if still applicable.
Add please add a
to the issue summary ... I'm kinda derailed with this issue :-/