Closed (won't fix)
Project:
Signup
Version:
5.x-2.x-dev
Component:
Code
Priority:
Critical
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
17 Jul 2007 at 04:59 UTC
Updated:
2 Jul 2008 at 14:35 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
mysocom commentedIt is very good idea to have this functionality.
cheers,
-mysocom
Comment #2
dwwSee the modules/signup/contrib folder on the DRUPAL-4-6 branch. There's already a module that does this integration. I just don't care about eCommerce at all, so I never upgraded it to 4.7.x or 5.x. Knock yourselves out. ;)
Probably easier to first port it to 4.7.x as the first patch, I'll review and commit it, and then start on porting to 5.x once the 4.7.x version lands...
Comment #3
pobster commentedPatch to 4.7 - upgrade to 5.x should be pretty simple, but I'm not going to do it as I don't need it... yet...
Pobster
Comment #4
dwwThanks for taking a stab at this. Visual inspection of the patch reveals 2 problems:
1) The variable changed from signup_ecommerce_nodeapi_$type to signup_ecommerce_$type but there's no DB upgrade path.
2) Code-style problems with string concatenations. E.g.:
should be:
(note, no space between the string literal and the dot/period). You've got that wrong in a few spots in the patch.
3) I'd prefer this:
To use the standard FAPI-style indentation:
Otherwise, looks good. I don't really have a way to test this, since I certainly don't want to setup all of ecommerce. ;) So, I'll be depending on some independent reviews and testing to know that this is really RTBC.
Thanks,
-Derek
Comment #5
pobster commentedMorning!
No worries, 2 + 3 are absolutely no problem and 1 would be in a separate .install file anyways and would be something like;
signup_ecommerce.install
I didn't bother with it before as meh, I'm lazy ;o) I know it's not the most elegant of code, but it works!
Thanks,
Pobster
Comment #6
pobster commentedOops, sorry forgot to change status.
Pobster
Comment #7
dwwsignup_ecommerce_update_1() needs work. ;)
1) You're "leaking" the old values. You should delete all of those when you're done to reduce junk in the {variables} table no one needs anymore.
2) variable_set() and variable_del() do other things besides manipulating the {variables} table. So, it's better to use those than to directly insert/delete.
Feel free to just attach a "signup_ecommerce.install.txt" file to this issue with your next draft, since that'll be easier for me to review and eventually commit than php blocks in your followups.
Thanks,
-Derek
Comment #8
pobster commentedAh wasn't aware of that with variables, no worries! Onto the next draft!
Pobster
Comment #9
pobster commentedErm *cough* sloppy workmanship, of course you don't need the $items any more as there's no db queries taking place. Sorry, use this instead.
Pobster
Comment #10
ZaphMann commentedjust tagging this thread - is there any way to follow a thread without doing this?? (ie get it into my recenet posts?
Comment #11
ZaphMann commentedjust tagging this thread - is there any way to follow a thread without doing this?? (ie get it into my recenet posts?
Comment #12
Anonymous (not verified) commented@ ZaphMann
No.
And I'm tracking as well.
Comment #13
dwwI cleaned up pobster's latest patch and .install file, fixed a few other problems I found when going through the signup_ecommerce code, and committed it all to CVS. I added the files on HEAD and branched for DRUPAL-4-7. So, the DRUPAL-4-7 branch now has the (presumably working -- I enabled the module and clicked around the settings a bit, but don't have time or interest in installing a full e-commerce test site for this) 4.7.x code.
Setting this back to active for whenever someone wants to port this to 5.x. Please role that patch against the code in HEAD.
If there are any bugs in the 4.7.x version, please open new issues about them and set the version accordingly.
Cheers,
-Derek
Comment #14
dwwp.s. If people test the version now in DRUPAL-4-7 branch and it's all happy, please create a task in the signup issue queue about a new official 4.7.x-1.2 release that includes signup_ecommerce. Thanks.
p.p.s. @pobster: please take a look at the CVS activity for details on what I did based on your patches:
http://drupal.org/cvs?commit=74672
http://drupal.org/cvs?commit=74674
http://drupal.org/cvs?commit=74675
http://drupal.org/cvs?commit=74676
http://drupal.org/cvs?commit=74677
Comment #15
pobster commentedOkay I don't have a fully functional Drupal 5.x install, so this really, really does need testing...
signup_ecommerce.info
And then use the patch on the 1.6 version of HEAD.
...I should point out that it doesn't actually appear to work correctly on my install (my install is borked though...) when the module creates the variables for some reason it creates eg. 'signup_ecommerce_story_story' and 'signup_ecommerce_workflow_story_story' - I've no idea why it produces the nodetype twice, but it could be down to my install (so could someone else test please?) I'm likely just doing something stupid, but I can't quite see where... Strange though, that if you issue a
print 'signup_ecommerce_'. $type;in the appropriate place, it will return the correct value which you'd expect? Meh I'm no programmer, I don't know... Other than that, it seems to work okay (once you've altered the variable).Pobster
Comment #16
pobster commentedI thought I'd change the status to 'code needs work' as I'd really like someone else to test it - I'm not kidding about my 5.x install being borked. I don't use 5.x and my install is a hack-ish mess, so I really need to know that it's not my install messing things up.
Please could someone test?!
Pobster
Comment #17
pobster commentedOkay, no worries consider the 5.x port abandoned due to lack of interest. No biggie for me as I don't use 5.x anyway. I've changed the status back to active as the previous effort was probably flawed anyway, I can't imagine that my borked install really *is* causing that double output, it's more likely something I'm not aware of (as I'm not familiar with 5.x).
Pobster
Comment #18
mysocom commentedI want to test your path to D5, but I don't have signup_ecommerce module.
Comment #19
mysocom commentedWhere can I get it ?
Comment #20
mysocom commentedI have Drupal 5.1, signup_ecommerce.module, signup_ecommerce_1.patch and signup_ecommerce.install_0.txt. I apply a patch and got error:
Glad if anybody could point out if I have configured anything wrongly.
Comment #21
savejonas commentedThis would really come in handy.
I'm just tracking the progress...
Comment #22
modctek commentedI'd really like to see this picked back up again. This would be extremely useful for our website.
Comment #23
pobster commentedThen test it and report your findings back here. That's all I asked for... I didn't ask for any coding help or anything taxing, just install the patch and let me know what happens?
Pobster
Comment #24
tizzo commentedI really need this functionality and am a novice coder. I want to help in any way that I can! I have applied the patch to the 1.6 version of signup_ecommerce.module and placed it in a folder with signup_ecommerce.intall and created a signup_ecommerce.info file exactly as instructed.
I did not get the problems that you had pobster, but I am unable to set the Signup E-commerce integration checkbox to enabled. I click the checkbox successfully, click save content type, and when I edit the content type again it is disabled. It is, of course, not working.
Comment #25
tizzo commentedTotally my bad. I had something messed up in my drupal install. As far as I can tell this patch is working perfectly! Thanks!
Comment #26
dww@indyfilm: FYI -- "Fixed" means I committed the changes to CVS. That obviously didn't happen yet. ;) Do you mean you took a HEAD checkout of signup_ecommerce, applied the patch from #15, tested everything thoroughly, and it all works? Did you review the code for potential security problems? Does the code adhere to the Drupal coding standards? Is it well documented and makes sense? If all of that is true, you could reply here, set the status to "patch (ready to be committed)" and say "#15 is RTBC because ...". Otherwise, your testing/review isn't very helpful, since I have no idea exactly what code you tested, if you tested it thoroughly, etc, etc.
Pobster seemed to imply that #15 is inherently flawed in comment #17, but then #23 implies it might work after all. I don't know what to make of this. To repeat: I personally don't care about this functionality at all, so I don't want to invest a bunch of time in this issue. I don't really want to think about it at all until there's a) a clear patch to review, and b) at least a few clear replies indicating that that patch has been tested and is working. Thanks.
Comment #27
pobster commentedI can confirm that for whatever reason the code does indeed create the variable incorrectly - as I mentioned before, it doubles the name eg. creates "signup_ecommerce_story_story" instead of "signup_ecommerce_story"... So it's not just my old borked install which was messing it up after all... The code in this area isn't particularly complex so I'm sure that can be sorted out easily enough, the problem really is that with the older version of ecommerce you could just manually alter the variable yourself and the patch would work... The latest version of ecommerce however... Nope... Doesn't work at all...
I'm afraid I've no time at all to sort this out, I'm working 14 hour days at the moment 5 days a week... Mind you I only made the patch before as someone on the forums requested it, I don't actually need this functionality myself... I'm sure I could find the time to do it if it were important to one of my own sites...
Pobster
Comment #28
pobster commentedIncidentally just to point out - the incorrect variable being created is why the checkbox never gets checked. That's why I mentioned it... If you change the variable to what it should be (in your variables table) then yes, the checkbox is checked and will work with older versions of ecommerce... No idea what version though, didn't bother to write it down.
Pobster
Comment #29
Krythis commentedFrom reading this thread looks like you are enabling products to sign up a user for an event, please correct me if I'm wrong. The functionality I'm looking for is slightly different in that I want to have the ability to enable an event to have a charge associated with it. This would allow the user to select an event and then be sent to the e-commerce check out after signing up, only if a product is assigned to the event. I've updated a copy of sign up to do this, but need to clean up the code and figure out how to create a patch before I can post it...Any tips on creating a patch in Windows?
Does this sound far off from what your trying to accomplish, or is there a benefit in doing the reverse by setting a product to be sign-up enabled?
Justin
Comment #30
Anonymous (not verified) commented@Krythis
Thats exactly what I am trying to do.
As for creating patches in Windows, the handbooks seem to have a section on that. http://drupal.org/node/60234
Hope to test out your patch soon!
txcrew
Comment #31
Krythis commentedDerek,
How do we handle a situation where there are two projects that are very similar but in the end will provide different functionality like this?
Comment #32
Krythis commentedFinally got cygwin to work, but in patching I found that I altered 1.120 and will need to update to 1.74.2.25 before I can prepare the patches.
Derek,
Why is the date for 1.74.2.25 prior to 1.120? I want to make sure I'm using the correct version, and not doing any unnecessary re-work on my part.
// $Id: signup.module,v 1.120 2007/10/16 19:39:30 dww Exp $
// $Id: signup.module,v v 2007/03/31 07:15:07 dww Exp $
Comment #33
Krythis commentedNot sure if Derek is following this thread. Can anyone lend some insight to my questions above? Even though my site has the change and is functioning I'd like to get this patch together ASAP as there appears to be many people interested in this feature.
Comment #34
dww@Krythis: as I said above, I'm not planning to follow this closely since I don't really care about this functionality. ;) However, to answer your questions:
re: #31: It sounds from #30 that these are converging on the same functionality, so I don't know what you mean about the conflict here...
re: #32: Because they're on different branches. 1.74.2.25 is on the DRUPAL-5 branch, whereas 1.120 is on HEAD. 1.74.2.25 is the revision that went into the official 5.x-1.0 release. There have been other changes to the DRUPAL-5 branch since then. See the CHANGELOG for details, or the CVS revision history. In your case, you want to take the latest code for contrib/signup_ecommerce from the HEAD branch, and port that to the D5 API, so the revision on the signup.module doesn't really matter.
hope that helps....
good luck,
-derek
Comment #35
Krythis commentedThe changes I made weren't to the signup_ecommerce contrib because it allowed a product to be and signup/event, instead I updated signup.module so that a signup could be a product.
Comment #36
dwwI just briefly skimmed the existing 4.7.x code of signup_ecommerce, and agree it's not ideal. However, there's no way I'll commit code directly to the signup.module to integrate in any way with ecommerce. So, sounds like this needs a fair bit of work. I can think of 3 possible approaches:
A) Port the existing signup_ecommerce functionality to D5 (shouldn't be hard at all). Once at least that much is working, modify it to work more like your changes to signup.module and allow a signup to be a product.
B) Refactor your changes to signup.module into a separate module, using signup_ecommerce as a rough guide, and then see if there's anything from the existing signup_ecommerce worth saving and adding to your thing.
C) Implement signups as CCK nodes with user reference and node reference fields (and whatever other field types you want as your signup form), complete with revisions, etc. Then, it should be trivial to be able to see those nodes as products via ecommerce. See http://groups.drupal.org/node/3430 for some of my prior thoughts on signup-as-CCK.
Comment #37
Krythis commentedSounds like what I've done basically, minus changing the name of the module. I'd love to learn how to pull out the changes I've made into a separate module and write it so that it can use signup, makes the most sense to me but is above my understanding of modules at this point and I don't really have the time to re-write everything right now. Is it normal for someone to take a module, make a couple tweaks, and change the name? Sounds redundant and against the standards IMO. Anyone interested in this project that would be willing to take a look at what I have and work with me to complete it in an 'standard' way? I have the functionality I need now, just looking to contribute to the Drupal project and help those who are looking for the same functionality.
Thanks,
Justin
Comment #38
dwwIs it normal for someone to take a module, make a couple tweaks, and change the name?
No, that's called "forking" the module, and not what I'm suggesting. There are already a lot of hooks and methods that signup.module exposes to let other modules do their business, which is, for example, how the existing signup_ecommerce.module works. I'm just talking about using those same hooks to implement the functionality you need, instead of duplicating all of the code in signup.module itself.
While I appreciate your desire to contribute, and respect that you have neither the time nor the understanding to do it the "standard" way, I'm sorry to report that I don't have time to help you through the details, and a forked copy of signup that tightly integrates it with ecommerce isn't something I can commit to the official version.
Since you're not actually using the existing signup_ecommerce functionality at all, and not porting it to D5 (which is what this issue is really about), perhaps you should just create a new issue in the signup issue queue, post exactly the code you have now directly to that, and say roughly: "here's what i'm using and it works for me. it can't be committed in its current form, but if someone wants to use this as a starting point to do it right, knock yourself out". you can post back to this issue with a link to your new one, and if people who care about ecommerce integration like your approach, they might be inspired to do what it takes to turn it into something that can really be committed. if not, no harm done.
sound good?
Comment #39
Krythis commentedSounds like that's exactly what i need to do, thanks for your time and guidance on this!
Justin
Comment #40
tizzo commented@dww Re:26 I'm sorry, I am new to the coding side of drupal and I am just now really making the transition from user to developer. (My first module should be ready for contrib this week :) !) Anyway, it now seems very obvious that fixed means that the patch has been committed. Sorry I was so unhelpful but hopefully this patch makes up for it.
I have created the patch using a CVS checkout I did just a moment ago. I have tested it and as far as I can tell the code is 100% working. Pobster's code just needed a little tweak in the implementation of form_alter and a corresponding change in the _signup_ecommerce_is_autosignup section where the setting is called.
Please take a look and let me know what you think or what I need to change! I am sorry that I did not reply earlier I have just been caught up with too many deadlines.
Comment #41
tizzo commentedI forgot to change the status. Hopefully I got it right this time :)
Comment #42
tizzo commentedAlso how do we go about creating new files for contrib modules? Should they go into the patches? I am attaching the .info file that I created. It is the same as the one placed in pobster's post above but adds store as a requirement as well.
Sorry for so many posts...I'm done now.
Comment #43
tizzo commentedI have further updated the module. It previously checked to see if signups were enabled by default for a given content type and whether signup ecommerce integration was enabled to determine whether or not to sign the user up. This created some counter intuitive behavious and made it impossible to have a type where you wanted some products to be events that could be auto-signup enabled and some products to just be products.
Previously if signups were enabled by default and signup_ecommerce integration was enabled but signups for the node in question were disabled the product would have signups enabled upon purchase and the user would be signed up.
I wanted to be able to control this on a per-node basis, so the module now ignores whether signups are enabled by default and just checks to see if signups are enabled for this node, if this node is a product, and if the content type to which this node belongs has signup ecommerce integration enabled. If it does, then the user is signed up when they complete their purchase.
This patch is for the most recent version of signup_ecommerce.module in CVS (downloaded with a "cvs -z6 -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib checkout contributions/modules/signup" command at the unix terminal) and not for the product of either my or pobster's earlier patches.
In my testing this module seems very stable. Though, the signup after payment completion seems to signup at checkout (and always did as far as I can tell).
Comment #44
GreenLED commented"signup_ecommerce_3.patch 6.66 KB"
Doesn't that freak anybody out?
Add some extra text or something.
:)
» Respectfully, GreenLED
»
Comment #45
tizzo commenteddww: Have you taken a look at this code? I have been getting requests from people asking me to send them my working version. I think there is a fair bit of interest.
Do you think it would make more sense for this to be its own module or part of signup?
I do not have a CVS account as I have not had anything meaningful to contribute to this point but would be more than happy to maintaint his module
Comment #46
tizzo commenteddww: Have you taken a look at this code? I have been getting requests from people asking me to send them my working version. I think there is a fair bit of interest.
Do you think it would make more sense for this to be its own module or part of signup?
I do not have a CVS account as I have not had anything meaningful to contribute to this point but would be more than happy to maintain his module
Comment #47
modctek commentedIs there another way to access the signup_ecommerce files you used to build this patch aside from checking them out of the CVS? I haven't the foggiest clue how to get them from CVS, nor do I have access to a Unix terminal, but I would dearly love to test this on our server. If it works for our company, I might be able to persuade them to donate some hard cash, as this would be a lynch pin in our website operations, and it would save me a ton of time, manually cross-walking purchases to signups in a spreadsheet.
Comment #48
socialnicheguru commentedsubscribing. I am drupal 5.7.
Can anyone tell me how true this graph actually is?
http://www.drupalecommerce.org/ec_solutions
I need to be able to integrate the ecommerce system with
signup/registration for events
signup/download of content
location
userpoints
ads
use cases:
Quick workflow/checkout for signup content like events.
I also need to be able to create a quick workflow/checkout for event signup:
ie. logged in users who click signup will be taken to the pay popup or page, pay, have their status updated on our site and in the database.
We will also need a manual way to delete and refund.
Roles system to sell memberships:
We are also looking for a system that integrates into the roles mechanism in drupal (admin->user management-> roles.
We have numerous roles on our site already defined. Account Types (bronze, silver, gold) encompass different roles. I would like to be able to sell these account types as defined by the account types module.
Ecommerce solution to sell content at different rates based on role:
anonymous users would pay full price for an event or piece of content
bronze members would be a little less for the same content
gold the lease
so i guess I am looking for price sensativity based on account type/roles that a person has
Give members who have a certain number of points discount membership or subscription. Allow them to share with their friends.
Also, I want it to work with creation of nodes like OG groups or being able to setup a store for any group
Thanks Chris
Comment #49
tizzo commentedThat chart is woefully out of date. eCommerce is a good module but in my opinion the current release of Übercart far surpasses the current release of eCommerce.
It is very easy to get this functionality with Übercart and workflow-ng. I recently migrated one of my signup + ecommerce production sites to signup + workflow-ng. I think it is a better way to go! If you are interested in how to do this with workflow-ng I'll write up some docs on it right away.
That said, if you still want to go with eCommerce this module has been spun into it's own project here: http://www.drupal.org/project/signup_ecommerce
It has a known issue with some non-paypal payment gateways where it signs the user up before they complete the payment process. I'll be fixing that as soon as I get the time.
Comment #50
pelicani commentedAnother option that is in development is the Signup Pay module. http://drupal.org/project/signup_pay
This is a great option if the only monetary transaction is for signups.
Comment #51
modctek commentedI'd definitely be keen to see documentation of Ubercart + signup + ng-workflow. I started looking into this exact route, but my newb drupal programming skills were no match for this particular project, and my eyes started to cross while reading the ng-workflow documentation.
I'd agree that Ubercart appears to have a more professional-looking solution than ecommerce, but at the moment, I do have a working website implementing ecommerce, signups, signup-status (w/certificates) and signup_ecommerce. I'm loathe to upend a working solution, however inelegant it may be at the moment.
Comment #52
dwwThanks, tizzo for taking over maintenance of this code. I don't use it, and am happy not to spend any more time on it.
Since tizzo moved the code over to http://drupal.org/project/signup_ecommerce I'm therefore no longer dealing with signup_ecommerce at all. I just removed those files from the main signup/contrib/signup_ecommerce directory, and left a note in the README.txt file pointing to the new project. I also added a little list of signup-related modules to the signup project page, so that folks are more likely to find these other add-ons if they want them.
Since signup_ecommerce claims to be 5.x-compatible, this issue is hereby over. Phew. ;)
Comment #53
tizzo commentedIn comment #49 I promised docs on signup + ubercart + workflow_ng docs "right away". Well "right away" obviously didn't happen, but otherwise I have kept my word.
This doc can be found here: http://drupal.org/node/277721
Sorry for the delay.