replace omniture.inc system with hook_omniture_variabless and document it

Excellent! I was going to start an issue about this sometime today. I am working on a companion module that handles ubercart markup with s.events and s.products for the scAdd, scCheckout and purchase events. This is exactly what I need for that.!

Greg & Steve,
I'm standing by to test this out. I have verified that the current omniture.module is collecting data.

Clint

I have a module for Ubercart that will add omniture script to the cart pages. If the page is not a cart page it returns having done nothing. That project currently uses the hook footer to do it's work so on cart pages we now get two instances of the omniture javascript, one from that module and one from this one. I have put that project on hold while we make hook_omniture_variables function.

Where I am a less experienced PHP programmer I am not sure how to proceed. I assume we want to pass the array that is currently populated by the omniture_variables.inc file into the hook via the module_invoke_all call, correct?

I am wonder what is the customary relationship between modules like these in the drupal community? I assume that other modules might spring up over time to markup zendcart or oscommerce for instance?

What's next?

so by adding a new line at 111 I have my upbercate module contributing to this modules omniture markup:

$omniture_inc_vars = module_invoke_all('omniture_variables', $main);

I didn't want to submit this as a patch without some reassurance first. When I create the diff/patch file for this, all the other changes between my .module and the branch are included. This includes the stuff from other issues that have not been committed to the code yet. Is it generally expected that patches will contain just the change as opposed to all the diffs from non-committed patches? I assume not. Should I edit my patch file to include just this one change? or wait till you ask me for a patch?

How do we handle the case where two modules called by module_involke_all('omniture_variables',$main); want to change or add info to the same array element?

I added a module_invoke_all that gathers eVars from other modules like my ubercart module. It is crude because it appends eVars to the end of the eVar list without consideration for duplication. I wanted to post this so that the structure is available and in hopes of encouraging some others who need this module to lend a hand.

The attached patch includes only the applicable code to implement this feature and is not cumulative of other changes not accepted into the core module.

I'll review and report back today...

The Omniture hook is working for uc_sitecatalyst module. Steve we need to get that module up on Drupal.org! I've installed both modules, applied the posted patches and have testing everything I can think of.

There are a few issues with the uc_sitecatalyst module that I've posted (not sure if its the hook or the uc_sitecatalyst module), Steve any insights? Issues posted here: http://www.ubercart.org/comment/5076/Steve-Looks-really-good

Specifically for this module I have a few suggestions/questions:

1) As far as I can determine s.pageName is not being populated. Check my page naming feature request for further details. Having a simple, logical and "automatic" page naming system is critical for a Drupal site. Even implementing $title if else statement would work really well. Or allow the content creator to specify on the "node/add" page what the s.pageName would be.

2) Adding the ability to define "s.events =" in the admin interface, very similarly to how you have crafted the taxonomy terms. For example, if a user could choose that "Submit Comment = event4" or "Submit Rating = event5".

3) admin/logs/status show the module as "Not Configured" (see attachment).

4) I cannot for the life of me figure out how to add code to the "Advanced: Javascript Code" section of /admin/settings/omniture. Greg can you provide an example? I imagine this is the method you intend for adding custom events, etc.

5) We also need to capture search data, search term and # of search results.

6) Error pages: "Page Not Found" or "Not Authorized" need to populate s.pageType.

That's all for now.

You do amazing work.

Clint

Greg,
I will definitely get these comments separated out to individual issues...

Look for it by the end of today, tomorrow at the latest.

Clint

Steve,
Thanks for busting out all my comments into separate issues. I was swamped at work the last few days.

Clint

Changed spelling in title.. it was bugging me ;)

Here's a revised patch that takes this concept far enough to meet the needs of http://drupal.org/node/317830 ... attached is a sample module that implements the hook ... patch was rolled from HEAD

[UPDATE] - attached a new patch that removed the calls to omniture.inc

Here's a little documentation. Modules implementing the hook return an array with the optional keys 'header' (string), 'variables' (an array of key value pairs for output in the js), and footer (string). You can declare any one or none of them. Header and footer have defaults that provide the previous JavaScript. The vars array is added to the output after the value of the omniture_codesnippet which is set by the admin interface if anything is set.

If nothing is defined in the admin interface and no module implements the hook then nothing is written between the header and the footer of the script. If necessary I can modify this pretty quick to include the default empty variables the module currently writes ... just let me know which way to go with it.

I gave this a quick test and really like where this is going. Nice work. I am going to test this a bit more later tonight and reply with anything that I notice.

Committed to head. http://drupal.org/cvs?commit=144813

Greg,
I think that we should abandon the 5.x version, unless you want to maintain it. I think that with D7 being so close, it is hardly worth maintaining the D5 version if nobody is really using it.. especially because it is in a usable state. I really don't mind porting the hook change over to D5 and trying to keep the branches in sync, but I think it is more important, at this point, to add new features to the D6 branch.

Secondly, I was meaning to create the dev version last night with the new hook system, but was distracted. I will take care of this asap.

Third, I guess I misunderstood the package. After reading http://drupal.org/node/101009, I will remove the package declaration when I create the -dev.

Thanks for the comments.

Package declaration removed: http://drupal.org/cvs?commit=144966
dev created: http://drupal.org/node/318264 (note, the release node is currently unpublished until the module has been packaged.)

I may have some time to do some documentation on this, and provide a set of example code, but it probably would not be till later on next week, Its a good offer is there any urgency to it? Unless ultimateboy has some bandwidth to do this earlier

cytefx, I highly doubt I'll have time to put together decent docs in the near future, so if you have the cycles, it's all yours.