[D8] LinkedIn Profile

Multiple Applications

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

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

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

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.

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 leanderl,

I have found couple of issues in your project. Please find those issues in "reviewed-module.txt" file attached.

Thanks,
Vipul

Hi leanderl,

i think that you shouldn't load scripts like this in your twig template : <script src="//platform.linkedin.com/in.js" type="text/javascript"></script> you can check how to do it in a better way with libraries or the #attached property here : https://www.drupal.org/theme-guide/8/assets

Git errors:

Git default branch is not set, see the documentation on setting a default branch.
Review of the 8.x-1.x branch (commit 29e5b73):

Coder Sniffer has found some issues with your code (please check the Drupal coding standards). See attachment.
DrupalPractice has found some issues with your code, but could be false positives.
FILE: ...al-7-pareview/pareview_temp/src/Plugin/Block/LinkedinProfileBlock.php
---------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------
57 | WARNING | #description values usually have to run through t() for
| | translation
---------------------------------------------------------------------------

FILE: /var/www/drupal-7-pareview/pareview_temp/linkedin_profile.install
---------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
---------------------------------------------------------------------------
14 | WARNING | Unused variable
| | $must_have_some_php_to_pass_coding_standard.
---------------------------------------------------------------------------

Time: 37ms; Memory: 4.25Mb

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.
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.

FILE: ...ar/www/drupal-7-pareview/pareview_temp/linkedin_profile.libraries.yml
---------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------
5 | ERROR | [x] Expected 1 newline at end of file; 0 found
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------

FILE: ...al-7-pareview/pareview_temp/src/Plugin/Block/LinkedinProfileBlock.php
---------------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
---------------------------------------------------------------------------
2 | ERROR | [x] Namespaced classes, interfaces and traits should not
| | begin with a file doc comment
---------------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
---------------------------------------------------------------------------

Time: 96ms; Memory: 6.75Mb

Source: http://pareview.sh/ - PAReview.sh online service

Automated Review
There are some minor errors & warnings the automated test reports, see http://pareview.sh/pareview/httpgitdrupalorgsandboxleanderlindahl2687063git

Manual Review :

Installed the module, and followed all the steps mentioned in the help. But LinkedIn Profile block with inline as a choice is empty as shown in the screen shot below:

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 / No: Does not follow] the guidelines for 3rd party assets/code.

README.txt/README.md

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

Seems a little odd having such short lines in the last paragraph and longer lines above, but this isn't a massive thing.

Code long/complex enough for review

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

I think this just passes..it's pretty short, but there's not a lot that can be done about that due to the nature of the module.

Secure code

[Yes: Meets the security requirements.]

Looks good to me.

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. <?php
   /**
    * @file
    * Contains \Drupal\linkedin_profile\Plugin\Block\LinkedInProfileBlock.
    */
   

   Namespaced classes, interfaces and traits should not begin with a file doc comment

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 are also a couple of other things I noticed; please see the following:

1. I can replicate the issue found by panshulk in #19. I used my own LinkedIn profile URL as found here: https://www.linkedin.com/in/matt-chapman-92225123. I've attached a screenshot of my configuration.

2. I think 1 may be due to the fact you're including in.js twice. Once in the library definition, and once in the template. There's an error message in the console stating that "duplicate in.js loaded, and parameters will be ignored". This is followed by a bunch of LinkedIn errors. I've attached a screenshot.

3. I'd want to shorten the help text variable. 500 characters long is pretty wide. Perhaps you could split it up as a multiline variable assignment across multiple lines?

4. In linkedin_profile.module you've got the following in your hook_theme implementation:

        'profile_type' => 'Profile type from module file',
        'profile_url' => 'Profile URL from module file',
        'display_mode' => 'Display mode from module file',
        'profile_name' => 'Profile name from module file',

Are these actually the default values for these variables? Seems like they should be empty or NULL

5. I don't think the theme hook name has to be block--linkedin-profile. It's a little confusing as it's not actually a block template..it's the template for the content inside the block. I'd be tempted to rename it to just linkedin-profile.

6. As an aside/personal preference..the if/else blocks in LinkedinProfileBlock::build look a bit overwhelming. It might be more readable if you were to break them up with a little whitespace, or move them to ternary operators on the render array.

7. You don't need the title key in your hook_theme implementation

8. In your info.yml file you've got it listed as being part of the Custom package. Seems like this shouldn't be the case as when released the module will no longer be "Custom".

9. Do you need the '#description' => t('description from block.php'), on line 57 in LinkedinProfileBlock.php?

10. Should line 93 in LinkedinProfileBlock.php read "block" instead of "plugin"?

@leanderl Thanks for the quick heads up. The Settings are as follows :

@leanderl Thank you for looking into it. :)
I suppose this problem resides within LinkedIn's Member profile URL:s , because i tried working out with a different URL and there seems to be no problem with displaying the profile.

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.