Should uc_recurring(_order) list uc_order / uc_cart as dependencies? [#907256]

I'm reading through the code of uc_recurring_order.module, and as far as I can see this module doesn't do just about anything without either the uc_order module (display the settings page) and the uc_cart module (display the recurring options). Looks like uc_cart requires uc_order, so that's probably the place to start.

So, should we add that to the list of dependencies for uc_recurring_order? Or move up the tree to uc_recurring itself? Or..? Basically, right now, it's possible to enable these modules without them doing basically anything, which is really confusing.

Comment	File	Size	Author
#1	uc_recurring-dependencies.patch	1.55 KB	univate

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

univate CreditAttribution: univate commented 10 September 2010 at 07:51

Status:

Active

» Needs review

File	Size
uc_recurring-dependencies.patch	1.55 KB

Yes dependencies don't appear to be correct, if you are using uc_recurring you will basically need all the core ubercart modules for it to be useful, it doesn't depend on anything else.

The following patch should take care of all the dependencies, its not completely accurate as to the true dependencies, but by listing things this way hopefully it helps somewhat make each of the modules functionality clearer?. uc_recurring is now not designed to do anything itself it requires either uc_recurring_order or uc_recurring_product to be enabled to be useful, which is documented in the README.

Comment #2

webchick

she/they

English

Vancouver 🇨🇦

CreditAttribution: webchick commented 10 September 2010 at 08:16

Makes sense. I was going by hook_menu() in uc_recurring.module which adds entries under "Payment" and "Orders" so thought those were hard dependencies.

These were the modules I had to enable to get this to actually work for http://drupal.org/handbook/modules/uc_recurring:

    * Other package
          o Token (inferred from all the rest; no need to list as dependency)
    * Ubercart - core package
          o Cart (YES. definitely a dependency)
          o Conditional Actions (inferred from cart)
          o Order (inferred from cart)
          o Product (inferred from cart)
          o Store (inferred from cart)
    * Ubercart - core (optional) package
          o Payment (should this be a dependency, too? I guess not since the project page lists not requiring payment as a feature)
    * Ubercart - payment package
          o Credit Card (no; optional)
          o Test Gateway (no; optional)

So I think we could get by with just listing uc_cart. But either way, this patch is definitely an improvement to helping people figure this out. Not sure I have enough confidence in my grokking of Ubercart internals to mark RTBC.

Comment #3

univate CreditAttribution: univate commented 10 September 2010 at 12:16

Status:

Needs review

» Fixed

Yeah actually I tend to agree, so just adding uc_cart as dependency in uc_recurring and other modules depend on uc_recurring - nice and simple.

payment - is definitely not required and it has been useful to test this module without worrying about having any payment stuff in the way - hopeful its obvious enough that if you want to accept payments you will need that module.