[D8] Animate slider block

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

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.

You need to perform some manual reviews of other projects in order to gain other reviewers' attention.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

There's another sandbox-project which is using same library - Animate Slider. Also there are fiew similar modules like Views FractionSlider but they are using different libraries so this one may work as a standalone module.

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

Yes: Follows the guidelines for in-project documentation and/or the README Template. But the documentation should be more complete. For example it is unclear to me what configuration should I make in order to add few slides to the slider (the example http://drup8demovdjm2cyo7u.devcloud.acquia-sites.com/ shows four slides). Currently I was able to get this work with a single slide in the block. Another issue is that the folder structure for the libraries provided in the README (and in the module itself) is different from the structure that these libraries have when you download them from GitHub. As a result you are getting confused with the errors in js-console because module can't find the required files. Also - found some typos in the README file - misspelled English words.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

Yes: Meets the security requirements. Although t() and check_markup() functions are not being used in some places where they could be used - please review your code again.

Coding style & Drupal API usage

1. (*) CodeSniffer found one error in 4 files: End of line character is invalid; expected "\n" but found "\r\n"
2. (*) What is the aim of the 'uuid' value in yml files inside config/install folder? That probably shouldn't be there. See screenshot Selection_535.png
3. I found that the widget accepts some settings like 'show', 'hide', 'delayShow'. Maybe it would be reasonable to make these configurable (e.g. in block settings)?
4. I would add some comments into methods of the AnimateSliderBlock() class for better understanding of what each block of code does.
5. I think hook_help() would be helpful here (to give users better understanding on configuration).

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.

Manual Review

1. Basic application checks

   1.1 Ensure your application contains a repository and project page link.

   [Yes: Follows]

   1.2 Ensure your project is not duplication.

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

2. Basic repository checks

   2.1 Ensure the repository actually contains code.

   [Yes]

3. Security Review

   3.1 Ensure the project does not contain any security issues.

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

4. Licensing checks

   4.1 Ensure the repository does not contain a ‘LICENSE.txt’ file.

   [Yes: Follows] the licensing requirements.

   4.2 Ensure the repository does not contain any 3rd party (non-GPL) code.

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

5. Documentation checks

   5.1 Ensure the project page contains detailed information.
   5.2 Ensure the repository contains a detailed README.txt.

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

   5.3 Ensure the code contains a well-balanced amount of inline-comments.

   [Yes].

6. Coding standards and style

   6.1 Run an automated review and ensure there are no major issues.

   [Yes: Automated review is passed..]

7. API and best practices Review

   7.1 Ensure you are using Drupals API correctly.

   [Yes: Used Drupal API correctly.]

@ashwin.shaharkar: look like you forgot to change the status. Is this now RTBC after your review or are there application blockers left and this should be "needs work"?

Review of the 8.x-1.x branch (commit 66c6a0f):

• DrupalPractice has found some issues with your code, but could be false positives.
  

  
  FILE: /home/klausi/pareview_temp/src/Plugin/Block/animateSliderBlock.php
  --------------------------------------------------------------------------
  FOUND 0 ERRORS AND 5 WARNINGS AFFECTING 4 LINES
  --------------------------------------------------------------------------
    38 | WARNING | \Drupal calls should be avoided in classes, use
       |         | dependency injection instead
    48 | WARNING | t() calls should be avoided in classes, use dependency
       |         | injection and $this->t() instead
    67 | WARNING | \Drupal calls should be avoided in classes, use
       |         | dependency injection instead
   104 | WARNING | \Drupal calls should be avoided in classes, use
       |         | dependency injection instead
   104 | WARNING | t() calls should be avoided in classes, use dependency
       |         | injection and $this->t() instead
  --------------------------------------------------------------------------
  
  Time: 31ms; Memory: 4Mb
  

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

manual review:

1. animateSliderBlock.php: File name must be the same as class name, i.e. AnimateSliderBlock.php.
2. animateSliderBlock.php: @file doc blocks are not needed in namespaced classes, see https://www.drupal.org/coding-standards/docs#file
3. Term::loadMultiple(): do not call global loading functions in classes, use dependency injection instead. See https://www.drupal.org/node/2133171 . Same for ImageStyle::load().
4. AnimateSliderBlock::build(): what the rand() calls here? Can't you just use an increasing counter so that you don't get duplicates? Please add a comment.
5. animate-slider-block.html.twig: all user facing text such as "Read more" must run through the translation system. See https://www.drupal.org/developing/api/8/localization

Have to run now, will continue later.

Yes, I used the command line version of Coder which is slightly more up to date than pareview.sh

The block is vulnerable to XSS exploits. If I enter <script>alert('XSS');</script> as body field with a restricted text format on a slider item then the block will display a nasty javascript popup. You need to respect text formats and you must not use "| raw" in the twig template. Make sure to read https://www.drupal.org/node/28984 again. Instead you should use a render array in your block class with #type => 'processed_text' where you specify the content and the configured text format. See http://drupal.stackexchange.com/a/139107/5356 . And please don't remove the security tag, we keep that for statistics and to show examples of security problems.

Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Hello @bapi_22

Can you please use the code HTML tag to make the code more readable?

You can find an interesting introduction to this aspect of Drupal at https://www.drupal.org/node/2133171.

Hello @bapi_22

Code Review are as follows:

1. First of all your Block file name(/src/Plugin/Block/animateSliderBlock.php) is started with lowercase, It should be 'AnimateSliderBlock.php' which will match with your class name.

2. in 'animateSliderBlock.php' line.no 38 => instead of using\Drupal::entityQuery('taxonomy_term');, you can use the class based like shown in here
3. in 'animateSliderBlock.php' line.no 41 => instead of usingTerm::loadMultiple($tids); ,you can entityTypeManager for it. In line.no.67 , you used entityManager for getting node storage and then load nodes. same same step you have to use in order to load terms, use class based method for entityManager as shown in here
4. and use EntityTypeManager, as EntityManager is going to deprecate

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.

Automated Review

There are many issues found by pareview.sh

https://pareview.sh/node/795

Please solve them before proceeding further

This review uses the Project Application Review Template.

@vivek.addweb,
Automated reviews are not application blocker. Is there any other blocker that you found or it's RTBC?

// Naveen

Automated Review

There are many issues found by pareview.sh

https://pareview.sh/node/795

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

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

It was complex for me to find why the next/prev arrows ('controls' fonts) are not displaying.

Code long/complex enough for review

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

Secure code

[Yes: Meets the security requirements.]

This review uses the Project Application Review Template.

Please do not assign ticket yourself. See the workflow https://www.drupal.org/node/532400

1: Module is not working for me . Please see the attached screenshot.
2: Dont use \Drupal::l in

$node_link = \Drupal::l($this->t('Read more'), $node_uri);

@deprecated in Drupal 8.0.0 and will be removed before Drupal 9.0.0.
   *   Use \Drupal\Core\Link instead.
   *   Example:
   *   @code
   *     $link = Link::fromTextAndUrl($text, $url);
   *   @endcode

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.

Manual review
Individual user account
Yes: Follows 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.
README.txt/README.md
Yes: Follows 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.

1. Please resolve all errors .
2. Line 32, 62, 97 AnimateSliderBlock.php use dependency injection instead \Drupal.
3. Line 99 and lin 111 AnimateSliderBlock.php use $this->t() for 'Read more' and 'Animate slider'.

Thank you for your contribution!

After 2017 March 7 everyone can promote a project to a full project.  A full project has a short project name and a drupal.org/project URL.  It can also have releases (like alpha1 or 1.0).  Edit your sandbox project, and then choose the 'Promote' tab.

https://www.drupal.org/docs/8/understanding-drupal-version-numbers/drupa...
https://www.drupal.org/docs/8/choosing-a-drupal-version/what-do-version-...
https://www.drupal.org/docs/8/understanding-drupal-version-numbers/what-...
https://www.drupal.org/docs/8/choosing-a-drupal-version/release-stable-v...

If you'd like to opt into security coverage, please ensure your module is ready for a full release, and then set this issue back to 'needs review'

Immense apologies for how long it took to get to this review completed.

I am closing it for lack of replies. Please re-open it, if you still want to opt into security coverage.

I apologize for the long delay before an appropriate response.