Show Referrals in Profile Review

[ZiNgA BuRgA]

This "review" was requested by leefish.

A few small things:

• PHP Code:

  $showrefer_template['member_profile_showrefer'] ='<tr> <td class=\"trow1\" valign=\"top\"><strong>{\$lang->referrals} ({$memprofile[\'referrals\']})</strong></td> <td class=\"trow1\">{$showrefer_referrals}</td> </tr>'; $showrefer_template['member_profile_showrefer_avatar'] ='<img src={$useravatar[\'image\']} {$useravatar[\'width_height\']} style="margin-right:5px;max-width:20px;height:auto;"/>';

  ---

  Be careful of over-escaping.  \$ in a single quoted string will stay like that in the actual template!

• PHP Code:

  1 2 3 4 5 6 7 8 9

  function showrefer_uninstall() { global $db; $db->delete_query("templates", "`title` = 'member_profile_showrefer'"); $db->delete_query("templates", "`title` = 'member_profile_showrefer_avatar'"); rebuild_settings(); }

  ---

  rebuild_settings is unnecessary as no settings have been changed.

• PHP Code:

  $referrer = htmlspecialchars($user['referrer']); //Fetch Referrer uid $query = $db->simple_select("users", "uid,username" , "username = '".$db->escape_string($referrer)."'");

  ---

  PHP Code:

  $new_user = htmlspecialchars($user_info['username']);

  ---

  Though not really detrimental*, htmlspecialchars is unnecessary here.
  * Usernames cannot contain HTML special characters except for the double-quote, but the MyBB parser unescapes it automatically, so largely a moot point

• PHP Code:

  $pmsubject = "New member referred by you."; $pm_message = "Thanks for referring me. Check out my profile ";

  ---

  The plugin makes use of language files, but these are hard coded...

• There's no limit to the number of users displayed in the referrers list.  Unlikely to be many, I presume, but may be worth consideration
  

[leefish]

Thank you for the review. I did not know about the usernames being already escaped; I was a bit paranoid about the username maybe being tampered with.

I will go fix those things. I must say,I was thinking that maybe there should be a setting where the last 6 referrals should show on profile with a link to show more as with a lot of referrals it gets crowded pretty fast.

[ZiNgA BuRgA]

In general, you only need to do htmlspecialchars for data to be displayed on a webpage.  Variables that are sent into the database need to be escaped with escape_string instead of htmlspecialchars.

[Sama34]

I would like to suggest some things.

• You should consider loading the language file only when necessary.
• Instead of:
  
  PHP Code:

  $referrer = (int)$mybb->input['uid']; $query = $db->simple_select("users", "uid,username,usergroup,displaygroup,avatar,avatardimensions,referrer,referrals" , "referrer = '$referrer'");

  ---

  
  You could just do:
  

  PHP Code:

  $referrer = (int)$memprofile['uid']; $query = $db->simple_select("users", "uid,username,usergroup,displaygroup,avatar,avatardimensions,referrer,referrals" , "referrer = '$referrer'");

  ---

  
  Which is more reliable.

• Instead of:
  
  PHP Code:

  1 2 3 4 5 6

  $referrer = htmlspecialchars($user['referrer']); //Fetch Referrer uid $query = $db->simple_select("users", "uid,username" , "username = '".$db->escape_string($referrer)."'"); $refers = $db->fetch_array($query);

  ---

  
  You could just do:
  

  PHP Code:

  //Fetch Referrer uid $query = $db->simple_select("users", "uid,username" , "uid = '".(int)$GLOBALS['userhandler']->data['referrer_uid']."'"); $refers = $db->fetch_array($query);

  ---

  
  Ideally and if using 1.8:
  

  PHP Code:

  //Fetch Referrer uid $refers = get_user_by_username($user['referrer']/*, array('fields' => array('username'))*/); // You don't really make use of the username.

  ---

  
  Even better you should be rather be hooking at datahandler_user_insert in whether 1.6 or 1.8

• Instead of:
  
  PHP Code:

  $newblink = '[url='.$mybb->settings['bburl'].'/member.php?action=profile&uid='.$new_uid.']'.$new_user.'[/url]';

  ---

  
  You should be using:
  

  PHP Code:

  $newblink = '[url='.$mybb->settings['bburl'].'/'.get_profile_link($new_uid).']'.$new_user.'[/url]';

  ---

• Ideally you should be using send_pm() instead in 1.8.
  

Most of those could not make any difference in your board where, IIRC, yo do not use SEF urls and users with small boards will probably not even notice/care for the others  but since you are releasing this to the public I felt just like sharing my thoughts.

[leefish]

Thank you Sama; I will make the change to the profilelink (as I have to do that anyway regarding the language strings) but is the rest making it more performant or is it just your preference?

[Sama34]

First point kinda yep. You will probably waste more time moving the line that somebody else caring about its position.

In my second point _(int)$mybb->input['uid']_ may not be set if for example users visit:
http://mybbhacks.zingaburga.com/member.p...http://mybbhacks.zingaburga.com/member.php?acti

Third point is about the correct code and hook to use to make your plugin more friendly (using the correct hook on case, somehow, an user is created outside the registration page and a referrer is set, for example) and use less code.

Fourth point will probably not make a difference if you use Google SEO but core SEFs are.. different:
http://community.mybb.com/member.php?act...http://community.mybb.com/member.php?action=profile

Pretty sure somebody will complain eventually for this.

Lastly, send_pm() is just about using less code and it also helps you if you want to send the message in the user's language instead of the board default.

I wouldn't just call them "preference" but "experience" I have obtained from writing a bunch of free and private plugins, simple and extensive ones equally.

[leefish]

On the send_pm Looking at the code, my code is a duplicate of it, so I agree its wasted code - a pull request in github might be nice. The memprofile tip is something I will use as I did notice that happening.

I have several forums; I think I tried it on an SEO'd  forum and the link in the pm worked... but hey, it's a free plugin, it wont harm the board (ie its not likely to get exploited) , and if someone doesn't like it no doubt there are more like it. You might even have one.