This module uses Facebook URL Linter API to send Drupal published content links to Facebook to make sure that Facebook have always the most recent version of your content that uses for Share link dialog on Facebook timeline.

Main reason why I code this module is that I have a problem on one news publishing portal with sharing a link on Facebook when Facebook doesn't fetch current information (title, description, image) from site but some older one from its cache.

Here is some documentation about the problem and the service that I use in this module Open Graph Protocol (section Editing Meta Tags).

Project page: Facebook URL Linter
Drupal version: 7.x
GIT repository: http://git.drupal.org/sandbox/tomasbarej/1840572.git

git clone --recursive --branch 7.x-1.x http://git.drupal.org/sandbox/tomasbarej/1840572.git fb_url_linter
cd fb_url_linter

Reviews of other projects:
http://drupal.org/node/1870348#comment-6863694
http://drupal.org/node/1863790#comment-6863792
http://drupal.org/node/1808004#comment-6863936

Comments

cubeinspire’s picture

Hi tomasbarej !

Automatic review

No misspelling reported.

Manual review

1. Maybe you can add a tag $query->addTag('node_access'); to the fb_url_linter_cron() and _fb_url_linter_write_record($node) queries so other modules can hook and modify them.

I don't see any blocker or other common problem on this module, entered data by user is correctly parsed.
If this PA had more feedback from other reviewers I would set it as RTBC.
Very clean work for a brand new project application !

tomasbarej’s picture

Thank you logicdesign for so quick review of my project :)

According to you suggestion about tagging the queries I add a tag $query->addTag('node_access'); to query on hook_cron execution where I select the nodes from db table. But I'm not sure if I understand what you mean about adding this tag into function _fb_url_linter_write_record(). Make this tag sense on that INSERT/UPDATE queries on module own table?

facine’s picture

Status:Needs review» Needs work

Hello tomasbarej!

My manual review:

Maybe you can call directly to settings form in hook_menu function with drupal_get_form
http://drupalcode.org/sandbox/tomasbarej/1840572.git/blob/refs/heads/7.x...

<?php
  $items
['admin/config/search/facebook-url-linter'] = array(
   
'title' => 'Facebook URL sending',
   
'description' => 'Configure settings for Facebook URL Linter module.',
   
'access arguments' => array('administer facebook url linter'),
   
'page callback' =>'drupal_get_form',
   
'page arguments' => array('fb_url_linter_configuration_form'),
   
'file' => 'fb_url_linter.pages.inc',
  );
?>

Also remove fb_url_linter_configuration_form_validate function using a core validation as:

<?php
   
'#element_validate' => array('element_validate_number'),
?>

Use in config form this for add default buttons and save values.

<?php
 
return system_settings_form($form);
?>

And rename fb_url_linter.pages.inc to fb_url_linter.admin.inc

Regards!

tomasbarej’s picture

Status:Needs work» Needs review

Hey facine!

Thank you for your feedback definitely make sense and simplify the code.

I've implemented all the points you've suggest and put changes back to the repo.

Again many thanks for useful feedback.

Cyberschorsch’s picture

Status:Needs review» Reviewed & tested by the community

Hey tomas!

I just reviewed your project and found some small things I would like to point to you.

1) In fb_url_linter.admin.inc Line 46 there is a small typo (Fitler => Filter)

2) In fb_url_linter.module
function fb_url_linter_send_node
You may want to get the path of the node with entity_uri instead of hardcoding "/node..". You already have the entity object when calling the send node function.
Also, maybe change the fb request url to a variable in case the url changes or the user wants to use a different url (i.e. sandbox).

This is nothing which blocks your request, though. Marking as RTBC.

tomasbarej’s picture

Thank you Cyberschorsch for your feedback and RTBC.

@ 1.) I correct that typo.

@ 2.) Ok so for building node url I used entity_uri. And URL for the endpoint of Facebook Linter I put into variable. At this point I don't this that interface for changing this variable is necessary.

klausi’s picture

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

tomasbarej’s picture

Adding PAReview tag.

klausi’s picture

Status:Reviewed & tested by the community» Fixed

Review of the 7.x-1.x branch:

  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
    FILE: /home/klausi/pareview_temp/fb_url_linter.module
    --------------------------------------------------------------------------------
    FOUND 1 ERROR(S) AFFECTING 1 LINE(S)
    --------------------------------------------------------------------------------
    133 | ERROR | Doc comment for var $nid does not match actual variable name
         |       | $node at position 1
    --------------------------------------------------------------------------------

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. You have to get a review bonus to get a review from me.

But otherwise looks good to me, so ...

Thanks for your contribution, tomasbarej!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

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.

Thanks to the dedicated reviewer(s) as well.

tomasbarej’s picture

Thank you klausi!

Automatically closed -- issue fixed for 2 weeks with no activity.

Anonymous’s picture

Issue summary:View changes

Add links to other projects reviews that I've made.