This module adds a simple block that displays the "last modification" date of the site.
(It shows the date when any node was saved or modified the last time. This is displayed globally, and not per node!)

I needed this feature and could not find any module that achieves this. There are only code fragments at DrupalAnswers and spread somewhere in the internet that try to solve this, sometimes with much DB polling.
Example done wrong

So I implemented this feature, and I want to contribute it as it seems to be needed by more people than myself...

Git clone instructions:
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/droetker/2467991.git last_modification_date
cd last_modification_date

This is my first Drupal project contribution, so no other project applications yet.

Comments

Manjit.Singh’s picture

Manjit.Singh
Primary language Hindi
Location Chandigarh
CreditAttribution: Manjit.Singh at Srijan | A Material+ Company commented
Status: Needs review » Needs work

@nerdoc Thanks for the contribution :)

Found some coding standard issues in last_node_modification.module and last_node_modification.install files. Please resolve these.
http://pareview.sh/pareview/httpgitdrupalorgsandboxdroetker2467991git

Also Please add review bonus also.

AshwiniPatil’s picture

AshwiniPatil CreditAttribution: AshwiniPatil commented

Good Work.

Follow the coding standard.
@file block missing in last_node_modification.install

reinier-V’s picture

reinier-V CreditAttribution: reinier-V commented

Hi nerdoc,

Seems like a nice tiny feature for some people!
On the other hand, this is possible as well by createing a view block with a contextual filter (this node), and printing some node data.

One major issue imo:
You say the block shows last mod date of any node (I'm reading it as 'with respect to the current requested node').
But since you use hook_node_insert to save the mod-date into a (global) drupal variable, and use it in the block content, you actually always show the date of the last modified node. (So the block will show the same thing, on each page etc).

So this should work different, or maybe it's wise to change the description of your module a bit.

Coding comments:

  • No proper doc comments on install file (top of file, description of file content / functions)
  • Line 45, commented code should be removed
  • Line 58, 67, 74: Indents
  • _last_node_modification_content(): '@return' should be in proper format, like '@return string'. See https://www.drupal.org/coding-standards/docs#functions
  • last_node_modification_node_update() / last_node_modification_node_insert() : Comments should be inline (not above function) for hook implementations

Regards,

Reinier

nerdoc’s picture

nerdoc CreditAttribution: nerdoc commented

Eh - yes, maybe this description is a bit misleading. The "last update per node" would be already done by Drupal itself. each node has included this info in the theme AFAIK any you can switch this off and on in the theme settings.

What this module should do is exactly: Show when the site (globally) was updated the last time (except config settings). I'll change the description text.
Your other issues:

  • @file comment in install file: done
  • remove commented debug code in line 45: done
  • indents: done
  • proper @return string: done
Unclear(ed) issues
nerdoc’s picture

nerdoc CreditAttribution: nerdoc commented
Issue summary: View changes
nerdoc’s picture

nerdoc CreditAttribution: nerdoc commented

Oh, @reinier-V: Yes, this could be done with a view. But IMHO it is heavier than this solution. With this module you don't even need views installed.
And, as I need it on many (similar) multisites, I would have needed to create the view in each single (multi)site again, or create a feature (which is a module as well, and even more overkill).

me-great’s picture

me-great CreditAttribution: me-great commented

Please change git instructions to

git clone --branch 7.x-1.x http://git.drupal.org/sandbox/droetker/2467991.git last_modification_date
cd last_modification_date

Also you can optimize your code by using hook_node_presave instead of hook_node_insert and hook_node_update with equal code.

Regarding uninstall hook, put there variable_del function for other variables that were used in your module

nerdoc’s picture

nerdoc CreditAttribution: nerdoc commented
Issue summary: View changes
nerdoc’s picture

nerdoc CreditAttribution: nerdoc commented

@Oleksiy Shevchuk - thanks - didn't check the received patch closely enough to delete all set variables again. _presave is also a good idea, implemented thanks.
All PAReview errors ditched.

ajayNimbolkar’s picture

ajayNimbolkar CreditAttribution: ajayNimbolkar commented

Review of the 7.x-1.x branch (commit e749f7a):

No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

Manual Review

Individual user account

[Yes: Follows] the guidelines for individual user accounts.

No duplication

[Yes: Does not cause] module duplication and/or fragmentation.

Master Branch

[Yes: Follows] the guidelines for master branch.

Licensing

[Yes: Follows / No: Does not follow] the licensing requirements.

3rd party assets/code

[Yes: Follows ] the guidelines for 3rd party assets/code.

README.txt/README.md

[Yes: Follows] the guidelines for in-project documentation and/or the README Template.

Code long/complex enough for review

[Yes: Follows] the guidelines for project length and complexity.

Secure code

[Yes: Meets the security requirements.]

Coding style & Drupal API usage

[List of identified issues in no particular order. Use (*) and (+) to indicate an issue importance. Replace the text below by the issues themselves:

  1. (*)variable_get('last_node_modification', NULL) in line no 105,No need to explicity assigned value "NULL" because last_node_modification not having option like "None of this".
  2. (*) $date = $last_update_date === NULL ? t('No recent updates.') :
    format_date($last_update_date, variable_get('last_node_modification_date_format', 'medium')), line no 108
    as per the above comment the condition change as follow
    $date = !empty($last_update_date) ? format_date($last_update_date, variable_get('last_node_modification_date_format')) : t('No recent updates.');
nerdoc’s picture

nerdoc CreditAttribution: nerdoc as a volunteer commented

Ah, thanks. As you can see, PHP is not my native language ;-)
I'll check that.

nerdoc’s picture

nerdoc CreditAttribution: nerdoc as a volunteer commented

@ajayNimbolkar: ad 1.: done.
ad 2: I find it more readable to not use the !empty double false condition, so I switched it to

$date = empty($last_update_date) ? t('No recent updates.') :
    format_date($last_update_date, variable_get('last_node_modification_date_format', 'medium'));

And I think the "medium" default should stay. If a node was saved, but there was any manipulation of the database and the last_node_modification_date_format variable is not present, at least the default 'medium' format is used. Unlikely, but does not harm.

PA robot’s picture

PA robot CreditAttribution: PA robot commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. If you are still working on this application, you should fix all known problems and then set the status to "Needs review". (See also the project application workflow).

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