Closed (won't fix)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
26 May 2013 at 14:58 UTC
Updated:
11 Oct 2013 at 10:24 UTC
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
Comment #1
kordaris commentedComment #2
kordaris commentedComment #3
kordaris commentedComment #4
brice_gato commentedHello @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
Comment #5
kordaris commentedThanks for the suggestions brice_gato!
I think I fixed the errors above. ventral.org was very useful.
Comment #6
kordaris commentedhello,
are there any more suggestions/corrections about this module?
what is the next step?
Thanks.
Comment #7
PA robot commentedWe 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.
Comment #8
rolando.caldas commentedHello 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
This functions is not necessary. You can do it in your hook_menu and remove stalker_admin_config():
.install
In hook_unistall use the variable_del() to remove the module's variables:
stalker_threshold, stalker_identity and stalker_narcissism
Comment #9
kordaris commentedThanks for the review rolando.caldas!
I believe I fixed your suggestions. Please let me know about any additional corrections.
Comment #10
alberto56 commentedI 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.
Comment #11
rolando.caldas commentedHello.
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:
I understand that, in this case, it is better to follow the recommendation of a person who can enable publishing projects.
Comment #12
PA robot commentedClosing 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.
Comment #12.0
PA robot commentedgit repo added.