[D7] Rdbanners

There are many information missing in your project application. you should follow up below articles to create a application in correct format. For example, project repo path and sandbox URL etc. are missing.

https://www.drupal.org/node/1587704
https://www.drupal.org/node/1659588
https://www.drupal.org/node/1127732

You should also run a automated review at http://pareview.sh/

Thanks,
Mukesh

Git clone command for the sandbox is missing in the issue summary, please add it.

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,

Thanks for the contribution.

Please find some notices that i found which could be improved

Manual Review

Individual user account
Yes: Follows the guidelines for individual user accounts.

No duplication
Yes

Master Branch
Yes: Follows the guidelines for master branch.
Licensing
Yes: Follows the licensing requirements
3rd party code
Yes: Follows the guidelines for 3rd party code.
README.txt/README.md
Yes
Code long/complex enough for review
Yes: Follows the guidelines for project length and complexity.
Secure code
No. If "no", list security issues identified.

1) rdbanners_add_banner_form_submit input is not sanitized before saving
2) In function rdbanners_add_banner_images_form_submit its better to apply filter_xss before saving.
3) rdbanners_settings_form_submit input is not sanitized before saving
Coding style & Drupal API usage
4) rdbanners_edit_banner_form_submit input is not sanitized before saving
1) Remove the white space before 'rdbanners_list'
$query = db_select(' rdbanners_list', 'rdbl');

2) Not sure if you really need this variable $variables['count']

3) Missing hook_requirements to check if bxslider is present

4) Please add a check to make sure library is present before this code
drupal_add_css($lib_path . '/jquery.bxslider.css');
drupal_add_js($lib_path . '/jquery.bxslider.js');

5) Wrong way of adding js and css to block use attached as below to avoid issues when block caching is enabled
$block['content'] = array(
'#markup' => mymodule_testblock_content(),
'#attached' => array(
'css' => array(
drupal_get_path('module', 'mymodule') . '/css/mymodule.css',
),
'js' => array(
drupal_get_path('module', 'mymodule') . '/js/mymodule.js',
),
),
);

6) $rdbanners_options = array(); is not necessary.

7) use https://api.drupal.org/api/drupal/includes!theme.inc/function/theme_image/7 instead of img tag $file = 'uri) . '">';

8) Always a bad practice to call $image = file_load($fid[$i]); and $src = image_style_url($image_style[$i], $image->uri); from templates, please use preprocess functions.

Prajwal

Corrected Sandbox URL and Git clone command.

@ram0108, thankyou for your contribution.

Automated Review

Best practice issues identified by pareview.sh / drupalcs / coder. None.

Manual Review

Individual user account

Yes: Follows the guidelines for individual user accounts.

No duplication

(*) May Be: Does not cause module duplication and fragmentation. As I googled there are lot of similar module exist but no module gives feature of putting multiple sliders on same or different - 2 pages. But there is already a Bx Slider module that uses same library, but did not provide this extra feature. Can you follow the steps in the link and confirm that the functionality contained in this module should stay separate from Bx Slider?

Also It would be good to extend your project page little bit more, you can take a reference from https://www.drupal.org/project/nivo_slider that almost does the same thing.

Master Branch

Yes: Follows the guidelines for master branch.

Licensing

Yes: Follows the licensing requirements

3rd party code

Yes: Follows the guidelines for 3rd party code.

README.txt/README.md

Yes: Follows the guidelines for in-project documentation and the README Template.

Code long/complex enough for review

Yes: Follows the guidelines for project length and complexity.

Secure code

No. If "no", list security issues identified.

1. (*)rdbanners_add_banner_images_form: this is vulnerable to XSS exploits. If I enter <script>alert('XSS');</script> in this form then I will get a nasty javascript popup when I visit the view page (admin/structure/rdbanners) and assigned page. At both places I get this popup. You need to sanitize user provided text before printing, make sure to read https://www.drupal.org/node/28984 again.
   
   
   
   
   
   
   
   

Coding style & Drupal API usage

1. (*) rdbanners_requirements(): In following code wrong placeholder name used in anchor tag. !bxslider and !curl_url, need to be fixed.
   'description' => $t("jquery.bxslider not found. <a href='!bxslider'>bxslider</a> library.", array('!curl_url' => 'http://bxslider.com')),
2. Initially when this module is enable, according to Readme there is link available for Add Banner at Administration > structure but no link is there. It needs to be fixed, currently there is no way to visit this page.
3. (+) Why are you adding css stylesheets[all][] = css/jquery.bxslider.css through .info file, even you are adding the same in .module file in correct manner.
4. (+) Why are you creating separate template (rdbanner_image.tpl.php), just to render the image, better to use either theme_image or theme_image_style.
5. rdbanners_add_banner_form_submit(): Please add your comment for this function drupal_flush_all_caches();, why it is required in your module.
6. (+) rdbanners_add_banner_images_form_submit(): Again drupal_flush_all_caches(); is there, also you are inserting user provided input directly to database that need to be fixed. Also check the same for another form too.
7. (+) rdbanners_view_banner_images(): You are providing only delete link there but if user want to edit existing banner then there is no way to do this.
   
   
   
   
   

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.

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

Thanks again for your hard work!

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.