[D7]Bootstrap Banners Widget

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

Fixed the git clone URL in the issue summary for non-maintainer users.

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 kunal.kagalkar,
I had a quick look at your module and done my bit of review :)

Manual Code Review:
1. Please change git clone command to
git clone --branch 7.x.1.0 https://git.drupal.org/sandbox/kunal.kagalkar/2758311.git bootstrap_banners_widget
2. Use project page template for the project page.
3. Add hook_help in module.
4. Use proper commit messages see the documentation.
5. Use Readme.txt template for the module. Currently README.txt file is blank.
6. File block comment is not proper, it should provide summary of your module.
/**
* @file
* This file is defines the form and themes.
*/

Moving back to N/W due to above issues.

Automated Review

You have a lot of errors there my friend you need to fix .. try to get them fixed:
http://pareview.sh/pareview/httpsgitdrupalorgsandboxkunalkagalkar2758311git

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

No: Does not follow the guidelines for 3rd party assets/code.
Why did you include bootstrap components (js and css) in your project? That's not good. For example, if my template already contains the files from bootstrap, then the same files are being included twice.

README.txt/README.md

No: Does not follow the guidelines for in-project documentation and/or the README Template.
At least Introduction, Requirements and Installation sections are mandatory.

Code long/complex enough for review

No: Does not follow the guidelines for project length and complexity.
Remains 130 lines of php code if we exclude bootstrap components and files containing the html structure. Formally, the module meets the requirements.

Secure code

Yes: Meets the security requirements.

Coding style & Drupal API usage

1. (*) In the template files should not be used the logic using php. (Example in file carousel.tpl.php)
2. (*) Use functions drupal_add_js and drupal_add_css in preprocess functions, not in template files.
3. (*) Use function t(). For example here: $block->title = 'Bootstrap Banners Widget';
4. Add more comments. For example in function bootstrap_banners_widget_form there is some kind of magic

This review uses the Project Application Review Template.

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

You have a lot of errors there you need to fix :http://pareview.sh/pareview/httpsgitdrupalorgsandboxkunalkagalkar2758311git

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.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity. Module all over contains the around 160 - 180 lines of php code.

Secure code

Yes: Meets the security requirements

This review uses the Project Application Review Template.

Please don't RTBC your own applications, see the workflow: https://www.drupal.org/node/532400

Hi kunal.kagalkar,

I did a manual review and installed this module in my localhost. Please see my comments below.

1. Pass all user facing text through t(). Found hard-coded texts in template files. Found below in carousel.tpl.php
<p><a class="btn btn-primary btn-lg" href="<?php echo $node->link; ?>" role="button">Learn more</a></p>. In many places noticed the same. Fix in all places.
2. If giving JavaScript inside panel widget body field, it is executing while displaying in front-end. eg: give this value inside widget content body "<script>alert('XSS');</script>" it will give an alert while displaying slideshow. There is no Text format filter available for body field. It seems like an XSS vulnerability and currently an application blocker.
3. Also if I put "http://google.com">
   alert('XSS');

   " in link field getting alert "XSS". Please sanitize the values before displaying to front-end user.

4. Move your JavaScript code in to separate file instead of writing inside template file. Found in form.tpl.php. Also drupal javascript behaviours in your code. Refer Managing JavaScript in Drupal 7.
5. As your module is depends on bootstrap library you can add dependency on Bootstrap Library module instead of Bootstrap theme.
6. Found bootstrap_banners_widget.JPG in repository. If not using remove it from repo.

Thanks,
ARUN AK

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.