Currently, there is no system to ensure that the modules needed for a behavior are actually present. This extremely important for exporting presets.

Here is my intial though on how to do this, though it could easy not be the best. I would imagine, there is some way to get the module that provides a behavior from the CTools handling. From there, we could just add an array of module dependencies to the map. This can then be checked on rendering, and maybe a message on the preset form and/or list.

CommentFileSizeAuthor
#14 756692.patch1.29 KBWill White
#13 fix_multiple.patch2.77 KBtmcw
#9 features_dependencies.patch2.01 KBtmcw
#7 features_dependencies.patch1.81 KBtmcw

Comments

tmcw’s picture

tmcw commented

Huh? It's quite important to read the documentation of the relevant modules - here's the API for features which explains what we need to do.

zzolo’s picture

zzolo commented

Like I said, those were just some initial thoughts. But more importantly, your comment basically suggests that exports are not supported without Features. So, if someone wants to export a preset and import it in the interface or in their own code, then it could break because they have not transferred over a module. If this is the case, then we should add Features as a dependency. Maybe I am confused about something here.

tmcw’s picture

tmcw commented

Not quite. Exports through the UI are supported regardless and missing pieces should be (and are) detected when you try to build a map without the requisite modules. But simulating the module requirement system *within the export code* is not a good idea, since exports != modules. Thus the integration with features is required for features, not pure exported code, because pure exported code will never be able to handle dependencies in a clean way.

zzolo’s picture

zzolo commented

I just want to be clear about all this. How are missing pieces detected and how would the user know about them? I would assume that Ctools does some checking, but as far as I can see in the code we are not telling the user about this. So, if someone has a map that refers to a behavior that is not installed, then that behavior just doesn't show up? So, overall what you are saying is that Behaviors are and will not be exportable (without Features) and no module dependency code is or should be implemented?

tmcw’s picture

tmcw commented

As before, behaviors are not exportables and therefore are not exportable. Functional code is not exportable and should not be. I think that Ctools throws an error when behaviors are not found - I will check this in a bit.

zzolo’s picture

zzolo commented
Title: Behavior module dependency » Ensure layer types and behaviors module dependency is handled correctly

Changing title. So, given all this information, we need to make sure that if a layer type or behavior is used and the module that creates is not present, then some how the user is notified.

One question is, do we continue to render the map in case of this sort of error?

tmcw’s picture

tmcw commented
Status: Active » Needs review
StatusFileSize
new features_dependencies.patch1.81 KB

Here's a patch that makes sure that modules which provide layer types are included as dependencies in exports including generated layers with those layer types. Please test and review.

zzolo’s picture

zzolo commented
Status: Needs review » Needs work

Shouldn't the function name be function openlayers_features_export, at least this is what I would assume form the name of the hook.

Also, not trying to be a jerk, but it would be cool if you provided patches with cvs diff -up so that myself and others that use Dreditor can more easily review patches.

tmcw’s picture

tmcw commented
StatusFileSize
new features_dependencies.patch2.01 KB

No, the hook is called per-export type, so it is called based on the export name. Thus layers.

Here's a patch made with -up.

tmcw’s picture

tmcw commented
Status: Needs work » Needs review
zzolo’s picture

zzolo commented
Status: Needs review » Needs work

Thanks for the -up. I don't know too much about the features hook. From just a visual review, the logic looks sounds.

Looks like there is a double semi-colon:

    $export['features']['openlayers'][current($data)] = current($data);;

I should be able to apply it and try it out over the weekend.

tmcw’s picture

tmcw commented

Initial commit: http://drupal.org/cvs?commit=361622

Still needs presets->behavior plugin dependencies.

tmcw’s picture

tmcw commented
StatusFileSize
new fix_multiple.patch2.77 KB

The initial commit breaks exports of multiple layers; I misknew the Features module implementation here. Here's a patch that should fix that behavior.

Will White’s picture

Will White commented
StatusFileSize
new 756692.patch1.29 KB

This patch removes the problematic code block as a stop gap solution.

tmcw’s picture

tmcw commented
Status: Needs work » Active

Okay, committed: http://drupal.org/cvs?commit=366128

zzolo’s picture

zzolo commented
Title: Ensure layer types and behaviors module dependency is handled correctly » Ensure layer types and behaviors module dependency is handled correctly in Features

Changing title for accuracy.

zzolo’s picture

zzolo commented
Issue tags: +beta blocker

tagged as beta blocker

zzolo’s picture

zzolo commented
Version: 6.x-2.x-dev » 7.x-2.x-dev
Priority: Normal » Minor
Issue tags: -beta blocker

Marking for 7.x and minor.

zzolo’s picture

zzolo commented
Component: Behaviors » OL API
Status: Active » Closed (won't fix)

Closing. Don't see this getting any attention in the near future.

  • tmcw committed 37a04d8 on 7.x-3.1.x 
    Committing patch by willwhite to temporarily resolve problems caused by...