This module records the node access history. Unlike {history} table in core which only stores the last viewing history of a node for each user. It stores all viewing history. It's useful for counting the hits of nodes in a certain period rather than a simple total count in the {node_counter} table of core. We can also find access history in {accesslog}, but we need to extract node ID in URL and it's much slower.

Here is the sandbox page of the project: http://drupal.org/sandbox/hgneng/1468518

Here is the git command to checkout the code:

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/hgneng/1468518.git node_view_history

The module is for Drupal 7.

PAReview.sh result: http://ventral.org/pareview/httpgitdrupalorgsandboxhgneng1468518git

Thanks for review!

Cameron

CommentFileSizeAuthor
#6 drupal_issue.JPG174.03 KBpgogy
#1 drupalcs-result.txt4.58 KBtargoo

Comments

targoo’s picture

targoo commented
StatusFileSize
new drupalcs-result.txt4.58 KB

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.
Review of the master branch:

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. Get a review bonus and we will come back to your application sooner.

targoo’s picture

targoo commented
Status: Needs review » Needs work

Manual review :

1) node_view_history.info
Remove version = "7.x-1.0"

I will test your module soon.

hgneng’s picture

hgneng commented
Status: Needs work » Needs review

Thanks for review.

>Remove version = "7.x-1.0"
done.

switched to 7.x-1.x branch.

Also fixed most Drupal Code Sniffer issues except line ending issue (Is it a must to change \r\n to \n?).

patrickd’s picture

patrickd commented

This probably means that your files are not saved unix-encoded. Your editor probably allows you to change this by saving ("save as") the file and choosing unix format.

as there are currently many applications in queue we need more reviewers,
so think about getting a review bonus and we will come back to your application sooner.

hgneng’s picture

hgneng commented

Netbeans seems have no way to change the line ending. I tidied up newlines with dos2unix.

I will think about getting a review bonus. Thanks for reminding.

pgogy’s picture

pgogy commented
StatusFileSize
new drupal_issue.JPG174.03 KB

Hello

  1. in admin/reports/node-view-history some of the labels for the form are in Chinese? (see attached file)
  2. package = "Access Log" - The .info guide suggests some standard packages
  3. in admin/reports/node-view-history could the timestamp be altered to different styles
pgogy’s picture

pgogy commented

Happy to test more once I can see the labels.

jygastaud’s picture

jygastaud commented

If you're working on netbeans, you can add this plugin which will allow you to convert line endings.

hgneng’s picture

hgneng commented

>If you're working on netbeans, you can add this plugin which will allow you to convert line endings.
Thanks for the tips!

hgneng’s picture

hgneng commented

Hi pgogy,

Thanks for the review!

>1. in admin/reports/node-view-history some of the labels for the form are in Chinese? (see attached file)
It seems to be a bug when exporting a view as Features. It will translate strings to locale setting automatically. I've fixed it manually.

>2. package = "Access Log" - The .info guide suggests some standard packages
Removed package option.

>3. in admin/reports/node-view-history could the timestamp be altered to different styles
Do you mean that you want a selection menu to switch timestamp style among short/medium/long format?

pgogy’s picture

pgogy commented

Hi

Re 3 - I'm english, so the American date style can be confusing - an option to set a preference would be nice - or display dates with the nam e of the month?

Hope this helps

jygastaud’s picture

jygastaud commented

Hi,

Manual review:

.module
Comments at line 110 and 127 should end with a . , ? or ! as each other comments.
Please remove unused code as your "Implementation of hook_user_cancel()."

.views_default.inc looks to have a wired encoding format. Please check it.

hgneng’s picture

hgneng commented

>American date style can be confusing
got it. I've changed timestamp to long style.

>Comments at line 110 and 127 should end with a . , ? or ! as each other comments.
fixed.

>Please remove unused code as your "Implementation of hook_user_cancel()."
fixed.

>.views_default.inc looks to have a wired encoding format. Please check it.
I've changed those Chinese comments to English.

c-logemann’s picture

c-logemann
he/him
Primary language German
Location Frankfurt/M, Germany
commented
Assigned: Unassigned » c-logemann

Machine Review is nearly clean:

There are still files other than README.txt in the master branch, make sure to remove them. See also step 5 in http://drupal.org/node/1127732
Review of the 7.x-1.x branch:

Maybe add the following link to "Issue summary".
http://ventral.org/pareview/httpgitdrupalorgsandboxhgneng1468518git
and change git link to a guest version:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/hgneng/1468518.git node_view_history

Assigned issue to myself to show that I am working on manual review.

c-logemann’s picture

c-logemann
he/him
Primary language German
Location Frankfurt/M, Germany
commented
Assigned: c-logemann » Unassigned
Status: Needs review » Needs work

Module duplication?

http://drupal.org/node/894256#anchor-goals
What is the difference betwenn this module and "user_access"?
http://drupal.org/project/user_access
Why not merging the projects? You can do the upgrade to Drupal 7 of user_access.

Manual review

I have successfully installed the module in a fresh drupal 7.12 only with views. It seems that it's recognizing node views as it should. But on testing of your default view page the following error appears:

Notice: Undefined index: timestamp in views_handler_filter->accept_exposed_input() (line 547 of /Applications/MAMP/htdocs/d7review/sites/all/modules/views/handlers/views_handler_filter.inc).
Notice: Undefined index: uid_1 in views_handler_filter->accept_exposed_input() (line 547 of /Applications/MAMP/htdocs/d7review/sites/all/modules/views/handlers/views_handler_filter.inc).
Notice: Undefined property: views_plugin_query_default::$fields in views_plugin_query_default->query() (line 1183 of /Applications/MAMP/htdocs/d7review/sites/all/modules/views/plugins/views_plugin_query_default.inc).
Warning: Invalid argument supplied for foreach() in views_plugin_query_default->compile_fields() (line 1101 of /Applications/MAMP/htdocs/d7review/sites/all/modules/views/plugins/views_plugin_query_default.inc).
Notice: Undefined property: views_plugin_query_default::$fields in views_plugin_query_default->query() (line 1183 of /Applications/MAMP/htdocs/d7review/sites/all/modules/views/plugins/views_plugin_query_default.inc).
Warning: Invalid argument supplied for foreach() in views_plugin_query_default->query() (line 1185 of /Applications/MAMP/htdocs/d7review/sites/all/modules/views/plugins/views_plugin_query_default.inc).
Warning: Invalid argument supplied for foreach() in views_plugin_query_default->compile_fields() (line 1101 of /Applications/MAMP/htdocs/d7review/sites/all/modules/views/plugins/views_plugin_query_default.inc).

Additionally: If you are providing a page url like "admin/reports/node-view-history" you can also provide a "Normal menu entry" in the "management"-menu to show a link on "admin/reports".

c-logemann’s picture

c-logemann
he/him
Primary language German
Location Frankfurt/M, Germany
commented
Issue summary: View changes

switched to 7.x-1.x branch

hgneng’s picture

hgneng commented

> Why not merging the projects? You can do the upgrade to Drupal 7 of user_access.
You are probably right. I will take a look at user_access. Thank you very much!!

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
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.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Issue summary: View changes

change git clone command and add link to PAReview result