Okay a very quick one this. In reply to this thread a very very quick plugin to only allow a person to comment once to an entry.

Usage V1.0.5

{exp:one_voice mode="url_title"} or {exp:one_voice mode="entry_id"}

{if already_spoken}
<p>Sorry you have already posted a comment and so are not allowed to post another!</p>
{if:else}
{exp:comment:form preview="{my_template_group}/comment_preview"}
<p>Standard comment form code goes in here</p>
{/exp:comment:form}
{/if}

{/exp:one_voice}

Required Parameters
mode=“url_title”
mode=“entry_id”
url_title or entry_id must (at the moment) be the last segment of the URL for this to work properly.

Hopefully should work okay. Let me know how it goes.

Best wishes,

Mark

Oh by the way you can also do this if you like :

in place of the above conditional in the first post and if you set this to your Super Admin group then Super Admins will always be allowed to post as many comments as they like.

Just thought that might help a bit too!!

Best wishes,

Mark

Mark, this plugin has some pretty big problems.  You’re using text straight from a parameter in a query without escaping it, are accessing an object property that might not be set, leading to PHP errors, are SELECTing * when you don’t use any fields, and you’re using ‘true’ as a variable name for a conditional.  Lastly your usage shows that you can send member_id as a parameter, but that’s not the case.  I highly suggest you read the developer guidelines, all three sections, which will be of tremendous assistance.

Hi Derek,

Thanks for the input. I did think last night that my conditional name was a bit silly so I am now changing this to {if already_spoken}, will that be better?

Regarding SELECT *, what is wrong with doing that as all I want to do with the plugin is check if there is a row for that user and that entry_id and if so don’t allow them to then comment. Is there a better way of doing this then?

Regarding the member_id=”“ parameter that was a mistook as it was originally in there and being used until I realised that I could just use the $SESS class instead to get what I needed. This has now been fixed.

The other two things you mention are the text parameter being used in a query. I am pretty sure I know what you mean by that as I guess that someone could paste anything into {segment_3} which could then lead to major problems. How would I go around this then or how do I sanitize (is that the correct word in this case) the parameter as a url_title could be anything really?

Is there a better way that I could go about making this plugin. I’d really like to learn the best way but just need some pointers if that’s okay? The developer documentation sometimes goes a little over my head and my coding skills aren’t all that good so any help you can give here would be greatly appreciated, thanks.

The last thing you mentioned was “accessing an object property that might not be set”. I must show my ignorance here as I don’t really know what that means? If you could possibly elaborate a little so that I can look this up or do whatever is needed then I will do. It’s not my intention to make plugins or what not that could be dangerous to use on sites so any help would be greatly appreciated.

Thanks in advance.

Best wishes,

Mark

Hi Mark

Just a quick note on the database thing: in all your custom db queries you should use

function for security reasons. Also if you only want to check if a row exists, instead of SELECT * selecting all fields (more resources intensive operation), you could also select only one field (as you’re only checking against an existing row) or even do a join and then use the DB functions to check the number of rows. Is that what was meant Derek?

Mark, read the developer guidelines, which include dos and don’ts code samples, and then return with questions that you need clarification on.  Just about everything I mentioned is already covered in those, and you’ll benefit from the other information as well.  The exception is this:

The last thing you mentioned was “accessing an object property that might not be set”. I must show my ignorance here as I don’t really know what that means?

Put this in a PHP enabled template, and don’t focus on the query, it’s just an example of a query that we know will return no results:

@peschehimself - yes.

http://expressionengine.com/docs/development/guidelines/security.html#sql_injection_prevention

And on the query, selecting any fields at all is unnecessary since he isn’t using any of them.

Then of course you wouldn’t examine num_rows, as that will always return a result, an integer equal to the number of records returned in $query->row[‘count’].

Hiya,

Thanks for both chipping in on this. Derek I have read all the developer guidelines but at times they just go over my head. I have read everything you have both said and I am very grateful for the help on this as I would like to make this plugin (as simple as it is) work correctly and safely if I can.

I have attached a new version to the top post so perhaps you could let me know if I am now on the right lines?
I think I have addressed most issues that were there in the older version?

Thanks for looking at this for me as I really do want to get better at all of this.

Best wishes,

Mark

Mark Bowen - 05 July 2008 04:34 PM

Derek I have read all the developer guidelines but at times they just go over my head.

I feel you - I can imagine that for a less experienced coder those rules and guidelines can be pretty overwhelming, nevertheless it should not stop you from learning as you’re doing right now Mark.

This might still throw an error when no entry with that url_title in the weblog with weblog_id is found. You should also probably escape all variables passed to the DB query. Other than that, it looks good.

Hiya,

Thanks for the reply, this is very much appreciated. Thanks for the idea there. I hadn’t thought about if say someone places in the wrong weblog_id in the parameter. I was just taking it that they would do this right.

I have just uploaded V1.0.3 which hopefully now should be checking that if there are no results in the query that checks for an entry_id then it throws out the conditional error so that the form isn’t shown.

Hope I did this right?

Once again thanks for the help on this. I am trying my best to understand all of this, honest.

Best wishes,

Mark

Hi Mark

The problem is, if no row is found in your query (e.g. if someone sets the wrong url_title or weblog_id parameter), this line

will throw an error, as there’s no ‘entry_id’ position in the row array. Instead you should use

or similar, as num_rows will be there in the $query object in any case and with this you actually check whether the query did find any rows in the first place.

Regards,
Peter.

Ah right,

Actually I did originally have :

instead but I guess that was wrong as well. Will change it to what you have shown above. Thanks for the help on this.

Best wishes,

Mark

New version now uploaded to the top post.

Thanks again for the help on this. Hopefully now everything is accounted for?

Best wishes,

Mark

Hi Mark

I don’t see any changes, in the code it still says:

Is the 1.0.3 version the latest?

peschehimself - 06 July 2008 03:19 AM

Hi Mark

I don’t see any changes, in the code it still says:

if ($query->row['entry_id'] != 1)

Is the 1.0.3 version the latest?

That was the change

I used to have :

Then I changed it to :

when you mentioned it. I probably changed it all a little too quickly

Thanks for all the help on this, really appreciated.

Best wishes,

Mark

Back to this post Mark, did you try the code sample I indicated in a template?  Make sure you have error reporting enabled in your environment.  Testing is important as well, surely this latest version is not behaving as you’d expect it to, unless you are testing with the Getting Started entry that comes installed with EE.

Ah ok, but still, this does not account for the error it might throw, because if there are no rows in the query result

this will throw an error as there’s not ‘entry_id’ in the row array, whereas

will always exist, num_rows as the property of the $query object will exist even if no rows were selected. Do you see the difference? In my example, I’m not accessing any array field of the $query->row property.

Hi Guys,

Thanks for the help on this but I have to admit that now I am really really confused

Derek Jones - 06 July 2008 06:40 AM

Back to this post Mark, did you try the code sample I indicated in a template?  Make sure you have error reporting enabled in your environment.  Testing is important as well, surely this latest version is not behaving as you’d expect it to, unless you are testing with the Getting Started entry that comes installed with EE.

Hi Derek,

I tried that code, I guess that I was supposed to surround it with php tags first though? By error reporting do you mean the :

Admin->System Preferences->Output and Debugging Preferences->Debug Preference

or something else perhaps?

I tried placing the code you showed in a template with PHP enabled with surrounding PHP tags and visited the template but just got a blank page with no source to it. Is that what I should expect to see? If so then I am completely dumb as I don’t quite get what it all means I’m afraid.

As to the version that I have on my system it is working fine with any entry and any weblog so not too sure why you would think it would only work with the Getting Started entry which I always delete upon installing ExpressionEngine anyway. Any more information as to where I am going wrong would be great though although at the moment I am seeing this all working the way I expect it to unless I really am WAY too tired and completely missing the point?

peschehimself - 06 July 2008 07:14 AM

Ah ok, but still, this does not account for the error it might throw, because if there are no rows in the query result

if ($query->row['entry_id'] != 1)

this will throw an error as there’s not ‘entry_id’ in the row array, whereas

if ($query->num_rows != 1)

will always exist, num_rows as the property of the $query object will exist even if no rows were selected. Do you see the difference? In my example, I’m not accessing any array field of the $query->row property.

I have to completely throw my hands in the air now and say that I’m afraid I didn’t understand this part, sorry

I have uploaded the ‘test’ version which I have on my system which to me seems to be working fine in the way that I want it to which is that if a person has already posted a comment to the entry then they aren’t allowed to post again and the other way being that if they haven’t posted a comment yet then they are allowed to.

If possible could you explain to me in simpleton terms (as that is what I must be ) what it is that I am still doing wrong and why it isn’t working as I see it on my system as indeed working.

Thanks for the help on this. I read through all the developer guidelines and I am trying to apply everything there to my code as far as I can but still it looks like I am obviously not quite getting it all

Best wishes,

Mark

Mark, I’m trying to guide you there in way that will lead to a lasting impression of the knowledge gained, instead of just giving you the direct code you need.  It will be much to your benefit if you first discover why what you’re doing is wrong before moving on.

See this KB article about blank pages, and enabling PHP errors.  And yes, the code will need PHP tags around it, and reference the global $DB object just as you are doing in your plugin.  A cut and paste for your template would be: