mods invoking [displayname]_paymentapi() rather than [module]_paymentapi()

Good Post! That sums up a lot of what I've said in some of those post exceedingly well.

I'd like to make 2 more small arguement for said alternative solution. If 2 methods where to use the same display name there could be issues. Granted, this is somewhat contrived. Why are you going to confuse your customer with 2 "Checkout with Credit Card" payment options? But still, its something that breaks the current model.
The second I think I put in one of the above listed posts but I'll rehash it. The function listed above is expensive in comparison. Iterate through all the modules(which on a large site might be quite a few) or just have the variable available to you.

I'll work on a patch for the alternatice solution tomorrow so people can review the changes needed! Lets put this bug to bed!

oh and one other thing, module_invoke checks if the function_exists so we might as well use:

function payment_get_modulename(&$display_name) {
  foreach (module_list() as $module_name) {
    if ($display_name == module_invoke($module_name, 'paymentapi', NULL, 'display name'))
      return $module_name;
    }
  }
  return $name;
}

or at least use module_hook instead of function_exists for clarity's sake

I've got no problem tackling the patch.

There are quite a few references in the store.module to payment_method. I think this is where your suggestion to leave that in the transaction variable might be key. That's is also where its going to be hardest to track whether any changes are going to need to be made I believe.

The reason is that we store the payment_method in the ec_transaction table in the database. If somewhere that's being used to call the api that someone is going to get broken because it's going to be a display name now. However if we decide to hack it and store the payment_module in that field, we're going to run into trouble with outputing history if a payment type module is removed.

So, the decisions we need to make are, are we going to need to call the api later? and if so well need to store the payment_mdoule so do we need to store the payment display name or do we want another solution to the module removal like falling back to the modulename? Also in consideration for these decisions: Is the 4.7 update process a valid reason to change our table layout?

But, other then that I think I've got it working pretty well right now though.

OK I think this is a pretty comprehensive patch of all the modules not in the contrib directory to implement the use of payment_module over payment_method. I can make a patch for the contrib modules later but we'll see if this sticks first.

I also to the liberty of fixing up one of the function I found along the way. The fix is both on a minor page and only a cleanup but its in the patch. Here's the summary.
payment_admin_overview has the following code in it:

  foreach (payment_get_methods() as $name) {
    if (function_exists($name. "_paymentapi")) {
      $settings = (function_exists($name. "_settings")) ? "[". l(t("settings"), "admin/settings/$name") ."]" : "";
      $help = $name. "_help";
      $desc = (function_exists($help)) ? $help("admin/modules#description") : "";
      $output .= "<p><strong>$name</strong> $settings<br />$desc</p>";
    }
  }

But, payment_get_methods only returns modules that implement the paymentapi hook. so that if isn't needed. Also it would seem better practice to use module_hook and module_invoke rather than calling the actual function exists functions. The functionality is the same but we end up with the following code:

  foreach (payment_get_methods() as $name) {
    $settings = module_hook($name,'settings') ? "[". l(t("settings"), "admin/settings/$name") ."]" : "";
    $desc = module_invoke($name,'help','admin/modules#description');
    $output .= "<p><strong>$name</strong> $settings<br />$desc</p>";
  }

Like I said, exceedingly minor but better code IMHO.

And one more piece of reference material for everyone. This is the output of grep -n payment_method */* */*/* after the above patch is applied.
This might be useful for finding issues and for contrib people that want to make the update to the stuff in the patch if/when it goes through.

And since I'm forgetful, here's the mentioned file

Yeah your probably right. Trying not to mow over some of the development. Here's the patch with out the function change.

I have fixed this in a much simpler method. This was actually a bug that I introduced when I updated the payment.module which meant that $txn->payment_method was the display name instead of the module name.

I have made a 1 line fix and the payment_method will hold the payment module.

I have removed that now.