A new 7.x-1.0 release for Stalker module.

Name of the module: Stalker.
http://drupal.org/project/stalker

Description: Lists the number of times a user's profile has been viewed by other users. Handy for spotting stalkers and secret admirers.
This module was created as an example module for use in Lullabot's Drupal Module Development tutorial DVD and video download.

Sandbox project: http://drupal.org/sandbox/kordaris/2004546

Git repo: git clone --recursive http://git.drupal.org/sandbox/kordaris/2004546.git stalker

Intended Drupal core version: 7.x

Comments

kordaris’s picture

Title: Stalker 7.x » [D7] Stalker
kordaris’s picture

Assigned: kordaris » Unassigned
kordaris’s picture

Status: Active » Needs review
brice_gato’s picture

Status: Needs review » Needs work

Hello @kordaris,
* The README.txt file not provided!
* I believe you have committed a lot of errors in the appellations of your hooks and your functions :
LINE 124 of stalker_7.module : Implementation of hook_menu() should be stalker_7_menu() instead of stalker_menu
LINE 98 of stalker_7.module : implementation of hook_theme() should stalker_7_theme instead should stalker_theme
...
* Some thing in stalker_7.pages.inc file
* Revise your ending file too. You should transform all of your files into unix style terminators.

As you make your corrections take a look on http://ventral.org/pareview/httpgitdrupalorgsandboxkordaris2004546git

kordaris’s picture

Thanks for the suggestions brice_gato!

I think I fixed the errors above. ventral.org was very useful.

kordaris’s picture

Status: Needs work » Needs review

hello,

are there any more suggestions/corrections about this module?

what is the next step?

Thanks.

PA robot’s picture

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

rolando.caldas’s picture

Status: Needs review » Needs work

Hello kordaris!

my manual review:

.module line 62:

* Implements hook_user().

You implements the hook_user_view so it must be

* Implements hook_user_view().

.module lines 165s

/**
 * Page callback function for admin/settings/stalker.
 */
function stalker_admin_config() {
  return drupal_get_form('stalker_settings_form');
}

This functions is not necessary. You can do it in your hook_menu and remove stalker_admin_config():

  $items['admin/config/user-interface/stalker'] = array(
    'title' => 'Stalker Settings',
    'description' => 'Configuration options for the stalker module',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('stalker_settings_form'),
    'access arguments' => array('administer site configuration'),
  );

.install

In hook_unistall use the variable_del() to remove the module's variables:

stalker_threshold, stalker_identity and stalker_narcissism

kordaris’s picture

Status: Needs work » Needs review

Thanks for the review rolando.caldas!

I believe I fixed your suggestions. Please let me know about any additional corrections.

alberto56’s picture

Status: Needs review » Needs work

I have installed and manually tested this module. Here are a few notes:

(1) On the page user/%user/stalkers, if the threshold has not yet been reached, one sees a blank page. Some text here might be a good idea, for example "Nobody has visited this user's profile 40 times or more".

(2) Logic and interface are mixed in the following case: the function stalker_user_page() returns a _themed_ version of the top stalkers, but, say I am writing a third-party module which needs to retrieve the number of stalkers, using this function is clunky because it returns markup and not structured data. Compare this to the function stalker_user_view() which, in my opinion is done right: the presentation (stalker_user_view()) is separate from the logic (stalker_get_count()), so if I want to retrieve only the unthemed data, I can call stalker_get_count(). In the same way, I would expect stalker_user_page() to call another function, for example stalker_get_stalkers(), whose role it is to return structured data, which in turn would be themed by stalker_user_page().

(3) The logic behind "Allow narcissism" is not clear. "It says Show how many times a user views their own profile". But: If you "Allow narcissism", your visits to your own profile are counted; then, if turn off "Allow narcissism", your visits before turning it off are _still_ counted, that is, the site still "show[s] how many times a user views their own profile" before "Allow narcissism" was turned off. Therefore, instead of "Show how many times a user views their own profile", a more exact phrasing might be something like: "Add a user's visit to their own profile to the total view count".

Cheers,

Albert.

rolando.caldas’s picture

Hello.

I have to correct one of the points of my review. Sorry. In the review process of a module of mine told me that instead utilizase module_load_include required, so I adopted that point as a correction to make.

My module has passed the review process, but the person who enabled the publication of the project (cweagans) commented as follows on the subject:

For this bit:

 389 /**
 390  * include the block functions file
 391  */
 392 require_once realpath(__DIR__) . '/pay_with_a_tweet.blocks.inc';
 393 
 394 /**
 395  * include the tokens functions file
 396  */
 397 require_once realpath(__DIR__) . '/pay_with_a_tweet.tokens.inc';

1) To load module include files, you should use module_load_include; and
2) If you're including this unconditionally, there's no reason to split them out into another file. It is, in fact, a bit slower, since you're adding a couple of disk reads/file open operations into the mix. You could probably just copy the contents of those files into your .module file and you'd be fine.

I understand that, in this case, it is better to follow the recommendation of a person who can enable publishing projects.

PA robot’s picture

Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

I'm a robot and this is an automated message from Project Applications Scraper.

PA robot’s picture

Issue summary: View changes

git repo added.