In my original patch, I added an additional field for tracking who a regcode was sent to. This is important because:

1. Admin creates 500 codes
2. Admin sends out 100 to a first batch of respondents to a survey
3. 20 of those people redeem it in the first week
4. Admin sends out another 100 codes
5. 80 of those people get codes which were already sent to the first batch.

This is a problem. That's why I included a "sent" column.

I suggest it gets added back or the feature is useless as it can't be relied upon. We should also make a way to mark an ind. code as "sent"

Comments

aidanlis’s picture

aidanlis commented
Category: bug » feature
Priority: Normal » Minor

Hi again,

Sorry for not responding to this earlier. The reason why it didn't get included is the way you implemented it wasn't extensible, e.g. I don't want to have any mailing code in the core module.

Unfortunately this makes it quite hard to do stuff like you suggest, but I think it's still possible - unfortunately I don't have time at the moment.

I'm moving this to a feature request, if anyone wants to take a stab I can guide them.

aidanlis’s picture

aidanlis commented

I've done 80% of this, the missing step is the filtering ... The current API doesn't allow me to filter on rows that aren't in the database table.

Some sort of hook which alters the database query might work ... or altering the table via each extension module, I'm not sure which is cleaner.

JacobSingh’s picture

JacobSingh commented

Great! Would be happy to not have a patched version.
I honestly feel like the *best* approach is views integration.

2nd to that, just add it into the main module, with an option to shut if off if you think that's important. I see it as a 90% use case, and no need to be separate.

Anyway, your choice but shouldn't add much bloat and makes for a more compelling core product IMO.

Best,
Jacob

aidanlis’s picture

aidanlis commented

It's definitely not a 90%, but the idea is to make the core module flexible enough that this is not only possible but easy.

What do you mean by views integration?

JacobSingh’s picture

JacobSingh commented

Well, if you wanted to, you could list regcodes in a view, but then, a views dependency (yet another). Perhaps you have a roadmap of all the amazing felxible thing such a module might do, but I don't know what they are. I'm in the camp of adding rich functionality to the core and then adding a modular structure when required, not to let it get int he way of having a complete product. The problem we're facing is that you send out a bunch of regcodes but then how do you send another batch? You can't because you might send the same one to two people, basically making the email functionality worthless. IF nothing else, a field for "sent" on a regcode (which could be set manually) is a must have IMO. Ideal to have the email sent to as I put in the version I sent to you, but if not, at least that.

-J

aidanlis’s picture

aidanlis commented

I don't see how listing regcodes in a view would be useful though. You can do this yourself with the tablewizzard or data module, but for what purpose? Adding new fields is a CCK thing, not a Views thing.

The smallcore / bigcore is a personal preference thing, but I'm a smallcore person and only a handful of people are using the regcode_mailer module, so there's no argument (other than making it work) for putting the mailer functionality in core.

I'll keep experimenting with a clean way of adding the regcode mailing functionality in ... it would be super easy with D7 because I could pass the query object to be altered ... maybe I can achieve the same thing with some string manipulation.

JacobSingh’s picture

JacobSingh commented

Hi Aidanlis,

Views 2 isn't just restricted to CCK. The listing interface you've built could be handled by views if you knew how to write a views handler to read from the regcode table. Then, a module adding additional fields can be done in an abstract way. The way it is currently built, trying to have fields outside the main table in separate modules will be quite complicated.

I don't think you should do this integration unless you know how, and even then it's just too much abstraction IMO since I don't see it being extended much or used in many places.

Funny this smallcore buzzword... means nothing in particular, yet used constantly by different people to mean different things. It's not like a plugin architecture is unique to Drupal. It's a well used design pattern and usually a wise idea, but not the only pattern in the book. Anyway, this module isn't core, it is contrib, it will likely be replaced by something else at some point, or it will mutate, etc. I think it's important that it rocks first and is super flexible and architechted 2nd.

In this case, given how the the module is currently architechted, it is a "bigcore" because it is hard to extend (as we are both finding). I don't think this is a bad thing, because I think it is pretty limited in scope, but entirely up to you. Just sad to see code I spent time on and stabalized wasted...

I've also written a function to enable people to enter codes after they have already registered to get additional roles, etc but I can't contrib it now because I'm operating off of a fork since you threw out my original patch and decided to include only a portion of it. sad, for what? Smallcore?

I see a couple things that will happen with this module:

1. Add some ability to mark a regcode as sent, integrate with messaging module to allow admins to send codes via twitter, sms, etc as well as mail. Will absolutely required a "sent" field to filter on.

2. A way for users to use codes post registration - integrate with rules (done).

3. A way for users to be able to get so many "invitations" with their registration (like google wave, gmail, etc) and send them to their friends (uses part 1).

I don't know what else we are expecting here, but all of those cases depend on sending codes and tracking that it has been sent.

Hope that changed your mind, but it doesn't sound like it will.

Best,
Jacob

aidanlis’s picture

aidanlis commented

Hi Jacob,

All of your points convince me that adding a single sent column isn't the way to go! Using your example, having a module which provides twitter integration, I might want to send codes via email and twitter ... should I then add two sent columns to the core? I want to track what email address the codes are sent to, and what twitter account ... should I add columns for these to core too? Of course not ... finding a way to achieve this via modules is the best way forward.

A way for users to use codes post registration is already provided via the regcode_voucher module.

I'll have a look at the views integration, I like the idea of being able to add additional fields / filter on fields with a layer of abstraction ... the way I'm doing it now isn't going to work. I've committed what I've done so far, it's pretty close but the query_alter is crappy.

JacobSingh’s picture

JacobSingh commented

k, I'll take a look.

I'm a big fan of iteration. You can just add a "sent" bool field now. In fact, that's all that is required probably. You could have the actual details stored separately / access separately if you have multiple senders. The (small)core module - heheh- also would need this. If I send them manually, how do I know that I've sent them?

Anyway, good luck with figuring out views integration.

Please also consider in the future that if someone dedicates time to a module you maintain, they are adding a feature which they require and are submitting it not only so that others can use the functionality, but so that they can stay in line with the module. If I just wanted to fork, I wouldn't have spent time documenting anything, or cleaning up the original code base which had huge duplication all over the place.

Because of this, if you take part of it, and just throw out the rest because it doesn't agree with some larger architectural sensibility, I don't benefit at all from my contribution. It no longer meets my needs, and I am running a fork, in which case I probably wouldn't have made any effort in the first place. This doesn't feel good, as time is precious (as I'm sure you are aware). Next time, try voicing concerns and working on a solution there and then OR committing it because it rocks and figuring out a "smallcore" solution later.

Best,
Jacob

aidanlis’s picture

aidanlis commented
Status: Active » Fixed

I've just committed the latest version to CVS on the 2.x branch. A development release should show up in the next 12 hours.

This new version has full views integration, and tagging via the taxonomy module. This means you can tag codes as sent_via_email or sent_via_sms and filter on them, which should solve your problem.

It's a complete rewrite of both the API and admin pages. This means there's heaps of bugs, so it's definitely not recommended for production.

I'm marking this as fixed, so keep an eye on the 2.x changelog.

JacobSingh’s picture

JacobSingh commented

Sweet! This is exciting, I'll have to take a look when I get time... (yeah right, time) :)

Best,
J

aidanlis’s picture

aidanlis commented

I've just committed a finished version of the code mailer submodule, everything appears to be working as expected!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.