Ahoy,

I've fixed a bug that prevented it from working with CVS HEAD, and changed the logic of nodeapi around a bit to do the checking in load and the theming in view. This allows me to customise the appearance far more in my theme in a less code and database intensive manner.

I've just noticed a few patches that may have hit the system since the version I downloaded, hence this patch may need review.

CommentFileSizeAuthor
#4 nodevote_improvements_unified.patch3.05 KBtelex4
nodevote_improvements.patch1.42 KBtelex4

Comments

kbahey’s picture

kbahey commented

Thanks for taking the time to provide a patch.

As for the HEAD compatibility, I will not do this change until 4.7 is branched off.

As for the rest of the changes, they sound reasonable.

If you submit a proper patch format "cvs diff -u", then I will apply them. Please do so against the most current version.

telex4’s picture

telex4 commented

Here it is in unified format, though I couldn't do a CVS diff because I don't have a repository checked out.

I also noticed that you'll need to filter out some regressions that my patch causes (a quick glance showed NODEVOTE_TYPE => NODE_TYPE). But I'm afraid I don't have time to go through and sort this out... use it if you want :)

kbahey’s picture

kbahey commented

You forget to attach it.

telex4’s picture

telex4 commented
StatusFileSize
new nodevote_improvements_unified.patch3.05 KB

Oh, urm, oops, oh well here goes again.

kbahey’s picture

kbahey commented

Hi

Before I commit this, I have a few questions:

1. Why did you use $object here?
+ $object['nodevote']->vote_display = 1;
+ $object['nodevote']->vote_do .= 1;

And not here?
+ if ($node->nodevote->vote_display) {
+ if ($node->nodevote->vote_do) {

2. Have you tested this?

telex4’s picture

telex4 commented

Ahoy,

On (1) there's no particular reason I can remember. If there's a more correct way then go ahead and change it :)

(2) - Yes, it's running as you see it on http://www.remixreading.org . See for example:
http://www.remixreading.org/taxonomy/term/49
http://www.remixreading.org/node/639

kbahey’s picture

kbahey commented
Status: Needs review » Fixed

Fixed implemented, except for the 4.7 way of listing nodes. This will be one when 4.7 is out.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)