[D7] LinkedIn post to group

There are some errors reported by automated review tools, did you already check them? See http://pareview.sh/pareview/httpgitdrupalorgsandboxle722365553git

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.

Automated Review

Please fix best practice issues identified by pareview.sh http://pareview.sh/pareview/httpgitdrupalorgsandboxle722365553git

Manual Review

Individual user account

[Yes: Follows the guidelines for individual user accounts.

Please fix issue title following the standard e.g [D7] Project name

No duplication

[No: Causes] 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.
No instructions on how to use or Install the module

Code long/complex enough for review

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

Secure code

[Yes: Meets the security requirements. Not sure I am not an expert.]

Coding style & Drupal API usage

Check http://pareview.sh/pareview/httpgitdrupalorgsandboxle722365553git

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.

There was a long process of getting an API account with LinkedIn, so this will not be a full review but just some thoughts that might help you :) Some other person that has access to the API will have to do the full test.

Automatic testing:

http://pareview.sh/pareview/httpgitdrupalorgsandboxle722365553git

There are still some formatting issues. No deal breaker what-so-ever, but it should be fixed.

Other thoughts

1. The block should not be cached globally (row 42)? Since you have different permission based on global $user I'm guessing it should be https://api.drupal.org/api/drupal/includes%21common.inc/constant/DRUPAL_...
2. The select queries you do - is there some reason that you use db_query instead of db_select? This will invalidate other modules for using hook_query_*? (Even though if I don't see any use cases for that)
3. linkedin_post_to_group_form_validate() doesn't do anything? Do you need it?
4. You should maybe add an uninstall hook in your install file that removes the variable you set - linkedin_post_to_group_allowed_extensions?
5. You could set a configure path to your admin in your info file - check here: https://www.drupal.org/node/542202
6. This one is maybe overkill - but in a worst case scenario if I follow your code correctly - a site owner could install the LinkedIn module, add the Oauth module/parameter, let a user connect, then remove the Oauth, then you will be stuck with an notice status message on all pages with the block - you could instead do the same validation that is done in linkedin_init() in the if()-statement for the block_view hook (row 64).

All the points above are definitely not any show stoppers. I wont change the status - I set out to fully try out your module, but it's better someone who has an actual LinkedIn API account does that. At least I hope it's some kind of useful feedback on the code which looks good.

Cheers

le72,

So far this module looks good! Congrats.

1.

$url = 'http://api.linkedin.com/v1/people/~/group-memberships:(group:(id,name),membership-state)?count=20';
$url = 'https://api.linkedin.com/v1/groups/' . $group_id . '/posts';

Think to insert these urls on a admin field, or on a variable, or on a constant, is better than keep this hardcoded.

2.
Personally, I don't liked the content of this function
_linkedin_post_to_group_post_to_group
This function are doing a lot of things, like, access database, make a post, etc. Try to isolate these components on other functions to better reuse and readability.

Automated tests:

################################ Coder Sniffer #################################

FILE: /home/jribeiro/Projects/git/linkedin_post_to_group/README.txt
--------------------------------------------------------------------------------
FOUND 0 ERROR(S) AND 2 WARNING(S) AFFECTING 2 LINE(S)
--------------------------------------------------------------------------------
 38 | WARNING | Line exceeds 80 characters; contains 89 characters
 41 | WARNING | Line exceeds 80 characters; contains 99 characters
--------------------------------------------------------------------------------

Thanks for your contributions.I would suggestion you to take a review bonus to speed up the process. Please help reviewing and put yourself on the high priority list, then we will take a look at your project right away :-)

Automated Review

http://pareview.sh/ -- has coder finding . Please work on those.
Please make sure that characters not exceeds the above 80 characters

Manual Review
README.txt
Please make sure that characters not exceeds the above 80 characters
linked_in_post_group.module
$form = array();

// User profile settings.
$form['linkedin_post_to_group'] = array(...........................

Please remove the extra blank spaces in the code

Automated Review

http://pareview.sh/ -- has coder finding . Please work on those.
Please make sure that characters not exceeds the above 80 characters

Manual Review
README.txt
Please make sure that characters not exceeds the above 80 characters
linked_in_post_group.module
$form = array();

// User profile settings.
$form['linkedin_post_to_group'] = array(...........................

Please remove the extra blank spaces in the code

Those minor formatting issue are surely not application blockers, anything else that you found or should this be RTBC instead?

Hi,

Great job.!!
It would be great if you can get a clean cheat from the "http://pareview.sh/pareview/httpgitdrupalorgsandboxle722365553git", as it still now is showing certain errors. :)

Hi le72, after reviewing your module I have the following suggestions:

1. Add a hook_help section with the necessary help content to make it easy for developers and site maintainers to better understand your module. Make sure your README.txt file is in sync with the hook_help content and that there are no ambiguities.

2. Can't see where module variables are being set! Consider creating functionality for a user to set the variables.

3. The module is not clearing all the module variables via hook_uninstall. Make sure all the variables used in the module are specified in the hook_uninstall function.

Hope that helps.

Multiple Applications

It appears that there have been multiple project applications opened under your username:

Project 1: https://www.drupal.org/node/2736097

Project 2: https://www.drupal.org/node/2365671

As successful completion of the project application process results in the applicant being granted the 'Create Full Projects' permission, there is no need to take multiple applications through the process. Once the first application has been successfully approved, then the applicant can promote other projects without review. Because of this, posting multiple applications is not necessary, and results in additional workload for reviewers ... which in turn results in longer wait times for everyone in the queue. With this in mind, your secondary applications have been marked as 'closed(duplicate)', with only one application left open (chosen at random).

If you prefer that we proceed through this review process with a different application than the one which was left open, then feel free to close the 'open' application as a duplicate, and re-open one of the project applications which had been closed.

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