In the current data model for Subscriptions, there is no way to tell when a subscription was added. A timestamp can be useful for a lot of use cases. In my case, I am building an administrative interface for a client who has to report site performance indicators to his boss. One of the indicators is the amount of new subscriptions in a given period.

Other use cases could include:
- Recently popular subscription content
- Recent subscribers to a node
- Recent subscribers to a user

This type of functionality should not have to be included by Subscriptions, but it is currently not possible to build it without a Subscription timestamp.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

idebr’s picture

Issue summary: View changes
idebr’s picture

Attached patch adds a column 'created' to the {subscriptions} table. The timestamp is added in subscriptions_write.

Status: Needs review » Needs work

The last submitted patch, 2: subscription-created_timestamp-2378985-1.patch, failed testing.

idebr’s picture

Status: Needs work » Needs review
FileSize
3.19 KB

The patch in #2 breaks update.php when there are currently more than 10 subscriptions in your site. Attached patch splits the db_add_field and db_update parts in two different update hooks, so db_add_field is only called once.

idebr’s picture

salvis’s picture

I'm a bit on the fence here. Adding a column is easy with hook_schema_alter(), and we could just add a new hook in subscriptions_write_subscriptions() to let you add additional values. This would open up a lot more opportunities than just hard-coding an additional field.

(
Additional comments on your patch:

Why are you selecting and updating in a batch, rather than just updating the entire table in one swoop? Are you concerned that db_update() might time out? Have you ever seen this happen when you update an entire column?

If you take only 10 records at a time then this is going to take an eternity if the table is large. You could use db_update() with range(), but not with 10 but 10000 records at a time...
)

idebr’s picture

I understand your hesitation to add a new column to a module that has done very well without for a long time. In this case, I believe this opens up a lot of opportunities for site builders as well as developers to add functionality 'out of the box' as mentioned in the possible use cases in #0.

Adding a column is easy with hook_schema_alter(), and we could just add a new hook in subscriptions_write_subscriptions() to let you add additional values. This would open up a lot more opportunities than just hard-coding an additional field.

Yes, that would have solved this particular use case as well. But why not have both? :). If you believe this should be a custom addition and not something that belong in Subscriptions itself, that would be fine with me. You are the maintainer after all.

Why are you selecting and updating in a batch, rather than just updating the entire table in one swoop? Are you concerned that db_update() might time out? Have you ever seen this happen when you update an entire column?

If you take only 10 records at a time then this is going to take an eternity if the table is large. You could use db_update() with range(), but not with 10 but 10000 records at a time...

In my particular case, the update does not take very long as the {subscriptions} table only contains a few hundred rows. Other sites may have a larger user base and different use cases. Potentially large updates are often written in batch format, since a complete update is more important than performance. Of course I'm willing to work on the particulates of the patch, if you are agree this column should be added in the first place :)

vuil’s picture

Status: Needs review » Closed (works as designed)

I close the issue as Closed (works as designed). If someone really needs it, please feel free to re-open the issue and update its information (at all).

idebr’s picture

Status: Closed (works as designed) » Needs review

I reopen the issue as Needs review. Please feel free to let me know if you need any additional information.

salvis’s picture

Status: Needs review » Needs work

The patch needs a re-roll.