Profile visits

Welcome!

You could make your project page look more structures by some html tags ;)
Also have a look at how to make your project page follow tips for a great project page.

Does it really make sense to put this module into the Profile visits package ?

Great to see that you've already cleaned up the code with drupalcs :)

We do really need more hands in the application queue and highly recommend to get a review bonus so we will(/can) come back to your application sooner.

regards

At first glance, the code looks good! (And its a little longer than your Invite Link module...though not by much).

Here are some suggestions:

1. Maybe add a menu tab to the users profile that has the profile visit history on it. This would serve two fold:
   • It would remove the dependancy on Views, and
   • Provide defaultfunctionality out of the box.
2. Consider making the visit expiration admin configurable.
3. Add another table to store total visits:
   • This would keep a running history for the life of the account, of how many visits the profile has gotten. That way you wouldnt lose the history when the visits expire.
4. What about a block that shows the total number of visits (see previous comment), and the number of new visits (Think the "Your Posts" block on your d.o dashboard). -- It would be a pretty small block though.

Instead of waiting to add functionality until the module is approved, add it during the process. I just had my first project application approved a week or so ago. And if you look at the code from the first commit to the code when it was approved, it went through a number of evolutions. Adding any one of these items would help demonstrate your ability to write well formed code.

I agree that they all can be done via the views module, but thats not the point. The point of this is process is to see whether or not you can write well formed code. By adding some of those features yourself (instead of relying on the views module) you accomplish 3 things:

1. You remove a dependency. Yes views is a popular module and is installed on most project, but still, it relies on views for something that could easily be accomplished by something like this:
   

   $visits = db_query('SELECT * FROM {user_visits} ...')->fetchAll; 
   foreach ($visits as $vist) {
   $rows[] = array(
     $visit-> uid,
     $visit ->  visited_at,
   );
   }
   
   $table = array(
     'headers => array(t('header1'), t('header2')),
     'rows' => $rows,
   );
   $output = theme('table', $table);
   return $output;
   

   Obviously this is just an example, but you could probably provide some default page showing a table of visits in about 20-30 lines of code.

2. Out of the box features. Provide a tab on the users profile by default, then let users override it with a more detailed view.
3. You provide more code for reviewers to look at and better get a sense of your coding abilities.

That's not up to me. Again its not just on quantity, but quality. Show the reviewers that you understand how to write code for drupal.

My suggestion is this:

While waiting for a more in depth review, why not start adding some of the new features. It will probably help get you approved faster.

Hi zpieslak

Ran this through ventral
There are some minor errors it found
http://ventral.org/pareview/httpgitdrupalorgsandboxzpieslak1489652git

don't use $GLOBALS - use global $user here
Also try to make this function more readable. Really tough to find out what it does.
Drupal coding standards recommend line length at 80 http://drupal.org/coding-standards#linelength

Also wrap the stuff after the || in parentheses as well :)

 function profile_visits_access($uid_host) {
  return ((($GLOBALS['user']->uid == $uid_host) && user_access('access own visits')) || user_access('administer users')) && $uid_host > 0;
 }

other than that, code looks clean.

The only suggestion I have is to make the module a bit more user friendly. Add some functionality as mentioned above, maybe a theme, maybe rules integration.

But I think one that will hit best - is a block that's not necessarily views based. Just so people can see what the module does out of the box.

Cool!

Very interesting that the user module does this... Looked around a bit more and seems that the functionality is the same. I just usually don't see it being done in that way. Cool So just let those two go then :)

Cool that your module provides views out-of-the-box, even tho i would probably implement them using drupal cores api.

Couldn't find any issue on manual review and module works great on my localhost.

Wether or not this project is big enought to grant project promotion permissions, i will leave for someone else to evaluate.

Is the functionality between this module and
https://drupal.org/project/user_visits
https://drupal.org/project/user_visits_adv
so different that it is necessary to make a new module out of it? (Apart from the point that this module is for d7)

I'm not sure how much sense the current implementation makes regarding caching ?
As drupal has an internal page cache that won't activate the callbacks your overriding by hook_menu() and as many sites are using proxy caches like varnish (which will also not trigger your callback when a cached page is visited) - wouldn't it make more sense to use javascript here (by including it within the profile page) to call a custom callback?

I don't know how other drupal-intern statistic modules handle this (I always tend to use external tools like piwik with custom plugins), but have you thought about that case ?

Sorry for the delay, but you have not listed any reviews of other project applications in your issue summary as strongly recommended in the application documentation.

Regarding the caching: The page cache only works for anonymous users, so you should be fine.

manual review:

1. profile_visits.module line 102: doc blocks comments should end in "*/" only, not "**/".
2. profile_visits_cron(): I think the time span should be configurable.
3. profile_visits_install(): why do you need to change your module's weight? If you want to make sure that any of your hooks is run before/after another hook you should use hook_module_implements_alter().

But that are just minor issues, so ...

Thanks for your contribution, zpieslak! Welcome to the community of project contributors on drupal.org.

I've granted you the git vetted user role which will let you promote this to a full project and also create new projects as either sandbox or "full" projects depending on which you feel is best.

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

As you continue to work on your module, keep in mind: Commit messages - providing history and credit and Release naming conventions.

Thanks to the dedicated reviewer(s) as well.