Add {mollom}.form_id and store originating form_id for session values

Initial stab. My hope is to get this ready for the new release tomorrow already.

We're starting to make further use of the session data we are getting from Mollom. Everything in the module is tied to a form_id in the meantime, except for the session data stored in {mollom}.

Right now, in various places, we are loading the locally stored data, so as to perform an action on it. However, the process of getting from that {mollom} data back to the originating form_id is a painful job that requires lengthy loops and lots of code lines. We need to get back to the form_id, to be able to load the registered Mollom form information, to e.g. figure out whether there is a 'report access callback' to invoke, or a registered 'delete form' to integrate with.

More specifically, #903770: Integrate with Mollom moderation will only get a Mollom session hash from mollom.com, so that's the only information we have. With the session hash, we can load our locally stored data, but as of now, that data only contains an entity type and entity ID. Thus, to be able to report content to Mollom, or to display or execute the delete (confirmation) form, we have to invoke mollom_form_list() to collect all registered forms, and for each registered form, compare the form information with the data of the session, in order to find the corresponding form information that belongs to the session. So only after taking this long code path of trying to find the originating form information, we can actually perform the desired action.

With a {mollom}.form_id, it gets as simple as this:

  $data = mollom_session_load($session_id);
  $mollom_form = mollom_form_load($data->form_id);

And this is an example for the painful ride we currently have:

function mollom_report_access($entity, $id) {
  // Retrieve information about all protectable forms. We use the first valid
  // definition, because we assume that multiple form definitions just denote
  // variations of the same entity (e.g. node content types).
  foreach (mollom_form_list() as $form_id => $info) {
    if (!isset($info['entity']) || $info['entity'] != $entity) {
      continue;
    }
    // If there is a 'report access callback', invoke it.
    if (isset($info['report access callback']) && function_exists($info['report access callback'])) {
      $function = $info['report access callback'];
      return $function($entity, $id);
    }
    // Otherwise, if there is a 'report access' list of permissions, iterate
    // over them.
    if (isset($info['report access'])) {
      foreach ($info['report access'] as $permission) {
        if (user_access($permission)) {
          return TRUE;
        }
      }
    }
  }
  // If we end up here, then the current user is not permitted to report this
  // content.
  return FALSE;
}

To facilitate testing, let's do this for HEAD first.

OK, I think this is ready to fly.

Sorry, working on the other issue revealed that a style $GLOBALS condition remained in mollom_data_save().

Plus, mollom_data_save() should allow to use re-save existing data.

Ran into one more bug in trying to re-save existing $data. Actually, it would be a good idea to finally change the {mollom}.session column into {mollom}.session_id... if you'd be on board with that.

Thanks for reviewing! I agree that the casting is poor design. Only used it, because I didn't have time to analyze whether $data should be an array or an object. If we enforce it for mollom_data_save(), then all other mollom_data_* functions and calling code should follow. As of now, I can only guess that an array makes more sense, since the data is derived from XML-RPC, resp., $form_state, which are both arrays. However, I need to double-check all existing code to make sure it's the right decision.

So I've analyzed our current and also upcoming usage of $data, and also took into account best practices for data types in Drupal core, and figured that $data should be an object, and we also want to explicitly require the $entity, $id, and $form_id properties to be set on $data, instead of overloading those individually via function arguments.

I'm going to resolve the @todo of renaming the two columns 'session' and 'did' now, which should ideally be part of this patch.

All of this is an API change, but I don't think (and actually hope) that no one is using those functions.

I actually expected a giant patch now, but the size of the attached one basically proves that the module provides really good API functions.

Forgot to also update the 'session' table index.

Cumulative backport to D6. Note that I've committed two small follow-up fixes for the existing update path in HEAD (typos).

Fixed stale code in mollom_data_save().

oh my, sorry - forgot to revert disabled test class methods.

Fixed lots of hiccups + learned something new about the D6 version: We only happen to store Mollom session date for nodes and comments in D6, because we implement dedicated hook_nodeapi() and hook_comment() hooks.

More bullet-proof. Ran tests manually.

Thanks for reviewing! Committed to D6.

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.