Updated for D6

WorldFallz - March 15, 2008 - 01:05
Project:Premium
Version:HEAD
Component:Code
Category:support request
Priority:critical
Assigned:Unassigned
Status:closed
Description

I needed this module for D6 so I took a shot at updating it. It seems to be working fine. I ran it through coder and cleaned it up.

Coder is giving me the following message on lines 45 and 73 and I'm not sure what, if anything, needs to be done:

In SQL strings, Use db_query() placeholders in place of variables. This is a protential source of SQL injection attacks when the variable can come from user data.

So far everything looks good....

AttachmentSize
premium-6.x-1.x-dev.tar_.gz8.48 KB

#1

WorldFallz - March 15, 2008 - 01:26

I forgot to say that it's not in patch form... I wasn't sure how to do that for a whole module, so I just attached the whole archive.

#2

moshe weitzman - May 13, 2008 - 22:08

there is a new $node->build_mode that this module can probably take advantage of in D6. A straight port might be less than ideal.

#3

moshe weitzman - May 30, 2008 - 01:18
Status:needs review» reviewed & tested by the community

Actually, this patch is just fine. I tested it and code reviewed it and it is committable IMO. Sure more could be done, but that would go beyond a straight port. Lets leave those for other issues.

I noticed a little cruft at the end of premium.info which can be easily deleted before commit. It is harmless though.

#4

moshe weitzman - May 30, 2008 - 01:20

That coder warning refers to the query where we inject the current timestamp. The current timestamp is always an integer and isn't user-provided content so the coder message is spurious in this case.

#5

Chill35 - July 5, 2008 - 14:16

I am keeping an eye on this.

Thank you for the port, WorldFallz.

Any improvement can be made into an issue after that's committed.

#6

jerdavis - July 9, 2008 - 22:38
Status:reviewed & tested by the community» needs review

In order to make things easier for a Drupal 6 upgrade, I've created a CVS diff patch based on the current Premium CVS HEAD branch. I've done some extensive testing on this end, and everything seems good. This was against Drupal 6.2.

Creating a patch from a CVS diff is really highly preferable to a re-write and full tarball of all module files. It makes it much easier for the project maintainer, and really isn't very difficult to do. There's some great information available here on Drupal.org at http://drupal.org/patch/create and http://drupal.org/node/120511.

Thanks!

Jer

AttachmentSize
premium_upgrade_d6.patch 10.98 KB

#7

salvis - August 31, 2008 - 14:12
Status:needs review» needs work

Congratulations for getting your seat, Jer!

With the commits at the beginning of August, this is obsolete.

Can you give an indication of when a D6 version might become available?

#8

webdevbyjoss - September 9, 2008 - 13:01

Hello, I'm interested in Premium module to be available for D6 so I can contribute some code, testing, etc for this module.

As I understand I can get an HEAD revision in order to get the latest Drupal 6 module code. Can you please assist?

#9

jeremdow - September 27, 2008 - 21:04

Hi, I rerolled this patch (actually I used Deadwood, just left a couple things to clean up on the installer and menu hook) - tested on Drupal 6.4 and running on my site - everything seems fine.

Thanks for a great module!

Jere

AttachmentSize
premium_upgrade_d6.install.patch 1.59 KB
premium_upgrade_d6.module.patch 2.84 KB

#10

jeremdow - October 4, 2008 - 22:36
Status:needs work» needs review

Anyone want to test this - Jer, Moshe?

I'd really like to see a supported D6 port of this module - this seems to be a good way to get that right away, as Moshe said.

Jere

#11

salvis - October 7, 2008 - 04:26
Status:needs review» needs work

Thank you for picking this up — a few comments:

  1. You dropped the primary key.
  2. In
    function premium_nodeapi(&$node, $op, $teaser) {
       $node->premium = _premium_node($node);
    -  $node->premium_access = _premium_access($node, $teaser);
    +  $node->premium_access = _premium_access($node, $teaser, $account);

    where does your $account come from?
  3. In
    -    if (is_bool($access = $function($user, $node))) {
    +    if (is_bool($function($account, $node))) {
           return $access;

    you stopped setting $access but you're still returning it.

#12

jeremdow - October 11, 2008 - 23:04
Status:needs work» needs review

Thanks - new patches attached - on your points -

1. Added the primary key back to 'nid' in the schema.

2. You're right, no need to pass $account through these functions at all - reverted to the original.

3. Not sure how this got in there - I'm not using any other modules hooking _premium_access - but reverted to the original - this is a short patch now.

Jere

AttachmentSize
premium.install.6x.patch 1.61 KB
premium.module.6x.patch 1.7 KB

#13

salvis - October 16, 2008 - 19:34
Status:needs review» needs work

It does look too short now, indeed.

If you go from

function _premium_node_operations_premium($nids, $premium = 0) {

to

function _premium_node_operations_premium(&$form_state, $nids, $premium = 0) {

... — where is the caller of that function? How does the caller know that he's suddenly supposed to pass an additional first argument?

Did you run this through Coder to check for other porting issues?

#14

webdevbyjoss - October 17, 2008 - 07:31

Thanks!

#15

chrysonline - October 30, 2008 - 20:53

hi,

it's difficult for me to patch. (i used patch software for windows like patch, tortoise, eclipse.. with no result).

Can you give the lastest version of Premium module for Drupal 6.x in attachment (i use D6.6) ?

Thanks!

#16

adamcarsonb - November 6, 2008 - 05:24

I would also like t try the 6.x rev version. Care to post a tarball? I'll let you know how it worked for me this week.

#17

xrundel - November 25, 2008 - 20:19

Hello all.

Great module. Sorry for the question, but could you please tell me if this module ready to install to 6.x?

#18

adam_b - December 1, 2008 - 13:48

subscribe

#19

djbdjb - December 3, 2008 - 11:10

subscribing

#20

PepeMty - December 30, 2008 - 05:05

subscribing

#21

drupalhizmetleri - January 5, 2009 - 19:05

subscribe

#22

kjay - January 8, 2009 - 16:24

subscribe

#23

celtictigger - January 13, 2009 - 13:12

Has this been committed yet as a D6 module? It's not listed on drupal.org/project/premium. I really need to have it for a project I'm working on and (for a variety of reasons) patching the D5 module isn't an option for me.

I'll go with the current dev tarball unless there is a better option.

#24

jerdavis - January 13, 2009 - 19:21
Status:needs work» fixed

HEAD updated to D6 and branch created - snapshot should be up soon!

#25

celtictigger - January 13, 2009 - 22:41

Once it is up I'll install on my Dev site and do a quick test.

#26

System Message - January 27, 2009 - 22:50
Status:fixed» closed

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

#27

JStarcher - February 27, 2009 - 21:48

Any updates? Should I use the unsupported from the HEAD?

#28

majnoona - March 23, 2009 - 13:07

subscribing

#29

espirates - August 30, 2009 - 02:09

subscribing

 
 

Drupal is a registered trademark of Dries Buytaert.