[D7] Pushbullet

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.

Hi bahuma20,

Upon Manual Review,

1 - Two different menu paths for same form.

/**
 * Implements hook_menu().
 */
function pushbullet_menu() {
  $items = array();
  $items['admin/config/services/pushbullet'] = array(
    'title' => 'Pushbullet',
    'description' => 'Pushing messages',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('pushbullet_form'),
    'access arguments' => array('administer site configuration'),
    'type' => MENU_NORMAL_ITEM,
  );
  $items['admin/config/services/pushbullet/push'] = array(
    'title' => 'Push',
    'description' => 'Pushing messages',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('pushbullet_form'),
    'access arguments' => array('administer site configuration'),
    'type' => MENU_DEFAULT_LOCAL_TASK,
  );

To make the first menu as default menu, you can just use title and type.

$items['admin/config/services/pushbullet/push'] = array(
    'title' => 'Push',
    'type' => MENU_DEFAULT_LOCAL_TASK,
  );

2 - Configuration form code is written in the .module file.

$items['admin/config/services/pushbullet/settings'] = array(
    'title' => 'Pushbullet settings',
    'description' => 'Configuration for the pushbullet module',
    'page callback' => 'drupal_get_form',
    'page arguments' => array('pushbullet_config_form'),

but the Administer Configuration forms should be in the seperate .inc file for each of the forms(file name should form_name.admin.inc).

3 - Variables that are created in this module which are not deleted while uninstalling the module.

/**
 * Page callback: Pushbullet settings.
 *
 * @see pushbullet_menu()
 */
function pushbullet_config_form($form, &$form_state) {
  $form['pushbullet_api_key'] = array(
    '#type' => 'textfield',
    '#title' => t('API-Key'),
    '#default_value' => variable_get('pushbullet_api_key'),

Variables can be deleted using variable_del() on the hook_uninstall() in the .install file.

Thanks
Purushothaman C

Hi bahuma20,

Your code looks very clean and well-organized, and the inclusion of a rules action makes this a potentially useful module for admins to get quick alerts, provided they want to use pushbullet. Seems to me like use cases are pretty limited, though, until you're able to implement other forms of messages. With only text available, I'm not sure I see how this is an improvement on rules module's baked in email event.

Only other thing I see is that you should remove the .gitignore file before promoting the project.

In response to kvnm, this could be useful in 3rd world countries where data-signals are hard to come by, and text messages are used far more often.

I did come across an error when running this locally on my machine:
An error occurred while pushing your messages: "cURL Error: SSL certificate problem: self signed certificate in certificate chain"

I imagine this is a restriction of me not having an SSL certificate with my local site. Is there a way around this? Just curious.

Code looks pretty clean. I saw in pushbullet.module, pushbullet_push function there is a switch statement with a case. Is there a reason for this?

try {
    $p = new PushbulletPushBullet(variable_get('pushbullet_api_key'));
    switch ($type) {
      case "note":
        $p->pushNote(NULL, $parameters['title'], $parameters['message']);
        break;
    }
    drupal_set_message(t('A message was pushed to your devices'), 'status');
  }

Also, minor typo stuff, but check out the issues here.

Keep up the good work. If this gets approved my company might find some use for it.

Fix the pareview.sh issues: http://pareview.sh/pareview/httpgitdrupalorgsandboxbahuma202237035git

Manual review:
The form which allows you to send push messages should be available to other user roles too. This is only functionality I found missing in this module.

Make the changes and I guess your module is good to go. Also review other projects to get review bonus. Nice work!

Pushbullet.class.inc is non-GPL code, It is copied wholesale from https://github.com/ivkos/PushBullet-for-PHP/blob/master/PushBullet.class.... Simply modifying it is insufficient to pass muster (in my opinion).

Read https://www.drupal.org/node/1001544

In pushbullet_rules.module, on line 34: use single quotes when possible.

In config_form.admin.inc, you are using html in t(), which you shouldn't. Pass an l() with a !placeholder

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.

Hello bahuma20 and thank you for your contribution,
it is a good idea to integrate pushbullet inside a drupal installation and with rules this module can be very helpful, please check the following:

Automated Review
Pareview is still giving some error and warning, please check:
http://pareview.sh/pareview/httpgitdrupalorgsandboxbahuma202237035git

Individual user account
OK! You can read https://www.drupal.org/node/272587

No duplication
OK!

Master Branch
OK! you can check: https://www.drupal.org/node/1127732

Licensing
OK! check: https://www.drupal.org/licensing/faq

3rd party code
OK! third-party issues have been addressed, see: https://www.drupal.org/node/422996

README.txt/README.md
I also think it’s OK! See: https://www.drupal.org/node/2181737

Code long/complex enough for review
OK! https://groups.drupal.org/node/195848

Secure Code
I see no security issues. OK! https://www.drupal.org/writing-secure-code.

Style of Code
Code is clean and well documented, previous issues have been resolved.

Project Description
Please complete the git checkout link to for making the checkout easier.
e.g., git clone --branch 7.x-1.x http://git.drupal.org/sandbox/bahuma20/2237035.git pushbullet

Nice work - thank you for your contribution! Integration with Pushbullet seems like a great idea.

Here is an idea for you to consider: how about you'd allow any user to add their own Pusbutton Access Token and allow to build Rules upon for example user's own nodes commenting? This way with Rules you could add Pushbullet notices or even replace email notification if users would have their onw Pushbullet accounts.

Automated Review

No issues found.

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 the licensing requirements.

3rd party assets/code

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

README.txt/README.md

No: Does not follow the guidelines for in-project documentation and/or the README Template. See also Coding style & Drupal API usage -section.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements. / No: List of security issues identified.]

Coding style & Drupal API usage

1. (*) Pushbullet rules prints a message to the user of sent message - regareless of the event and wether current user has permission to push messages. This also applies to Rules-based message pushing. For example:
   1. Add "View user profiles" -permission to anonymous user
   2. create Rule to push a message when someone visits user profile page
   3. go to a user profile page as anonymous user
   4. => Visiting user should definately not see notification about pushed message.
2. (+) Requirement to download and install Pushbullet library is not stated on project page. Please update both the project page and README.txt (add INSTALLATION -section) to include detailed instructions for users how to download and install required 3rd party library.
3. (+) README Template is missing information of 3rd party library requirement and requirements that are dependencies of Rules (ie. Entity). Or perhaps you should add RECOMMENDED MODULES -section and put Rules (& it's dependencies) there?
4. (+) Consider checking Pushbullet Access token before allowing submitting Note / Link pushing forms. Currently submitting form without Access token leads non-selfdescriptive (for a non-technical user)error An error occurred while pushing your messages: "HTTP Error 401". You could also add a post-installation message telling user to add Access token in admin/config/services/pushbullet/settings. Maybe clarify technical messages, there are only a few of them after all.
5. Please add direct link to repository to you project application: http://cgit.drupalcode.org/sandbox-bahuma20-2237035
6. Project page guidelines: Please include a link to a Community Documentation for the project at Drupal.org (or, if there is no community documentation, a link to the README-file in the repository HEAD).
7. README.txt MAINTAINERS-section should contain maintainer -info, move from INTRODUCTION-section
8. Consider adding hook_requirements() implementation as it would be helpful in checking presence of required library (and also prints this information on admin/reports/status -page).

The starred items (*) are fairly big issues and warrant going back to Needs Work. Items marked with a plus sign (+) are important and should be addressed before a stable project release. The rest of the comments in the code walkthrough are recommendations.

If added, please don't remove the security tag, we keep that for statistics and to show examples of security problems.

This review uses the Project Application Review Template.