[D7] WebEx Aggregate

Hi Josh,

Your repo on your sandbox project page is empty. It will be great if you can clone the repo from your git to drupalcode.org too so that it is available on http://drupalcode.org/sandbox/joshhoegen/1866336.git
Then you can also use the automated code review at http://ventral.org/
Also, specify a version specific branch.

The "README.txt" file is more than sufficient hence no need for "README.md".

Also the .module file has
/*
*
*
/ type comments. Not in line with drupal standards. Please correct according to http://drupal.org/node/1354#general.

Also the @file directive is missing in the module as well as in the includes folder http://drupal.org/node/1354#files

On line 66 of module try using use "db_select" rather than "db_query"

Please specify comments for functions like 'function webex_event_list_block_view($delta = '')' and 'function webex_event_list_theme()' in line with http://drupal.org/node/1354#functions

The .info file has an incomplete

5 project status url =

Please remove the redundant files like .gitignore and .gitattributes from the repo as well.

Manual Review :

1) Please add README.TXT file in your module that tells the project description & other useful information.

Because of a number of coding standard errors. Please rectify and I'll be happy to review in detail again.

Automated review

1. Quite a few errors, see http://ventral.org/pareview/httpgitdrupalorgsandboxjoshhoegen1866336git-...

Manual review

1. You have a master branch in your Git repo of the project which is confusing. By the Drupal community standards you should only have branches for the actual module versions, e.g. 7.x-1.x (which you have) or 6.x-1.x.
2. Translation support t() missing in a few implementations in webex_aggregate.module, e.g. webex_aggregate_menu(). It's also partly missing in webex_aggregate_form.inc, webex_aggregate_list.inc, block--webex_aggregate_title.tpl.php, block--webex_aggregate_date.tpl.php.
3. Translation support is also missing from the Javascript implementations in webex_aggregate.admin.js and webex_aggregate.js.
4. console.log('colorbox'); found on webex_aggregate.js ln 34, this is likely to break Javascript on some browsers.
5. webex_aggregate.js ln 34 has the dependency on Shadowbox object, how's that dependency resolved? Libraries API is not deployed, no additional code is given. Please document or resolve the dependency.
6. README Usage pt 2 incorrect. The correct path is not /admin/webex_event_list/list but /admin/config/webex_aggregate/list and the link says "Add New WebEx Aggregate". It could be that I'm doing something wrong, but README is clearly not in sync at this point and is confusing, please adjust. Please also consider bringing your README in sync with a project page.
7. Clicking on "Add New WebEx Aggregate" link ends up on 404 if Clean URLs are disabled.
8. $webex_info->webex_json in webex_aggregate_output() is not sanitized, see Handle text in a secure fashion. Because the module in its current form is vulnerable, I'm adding a security tag. Please don't remove it, it is used for tracking purposes.

The webex_aggregrate.install() file should implement hook_uninstall() where you would delete the variables which have been set in the module.

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Automated review:
- See http://ventral.org/pareview/httpgitdrupalorgsandboxjoshhoegen1866336git again (only two minor things)

Manual review:

.gitignore:
- Line 1: $ cat .gitignore?

webex_aggregate.install:
- Line 123: hook_uninstall shouldn't return anything

webex_aggregate.module:
- Line 80,91: Remove unused commented code.
- Line 84: Unused variable $k
- Line 99: $deltas not defined. Init with $delta = array().
- Line 161-165: module_load_include should only be called inside a function when these files are needed. Or are included by the menu_hook. If you need them anytime, use files[] in the info file - like for classes.

block--webex_aggregate_date.tpl.php
- Line 39: Use l() ;)

block--webex_aggregate_title.tpl.php
- Line 45: Use l() - What is the target of this link?

webex_aggregate_form.inc:
- Line 22: Unused variable $debug.
- Line 33: There is a function drupal_jeson_decode() which should be used.
- Line 205 to 218: Values submitted by the form are stored in $form_states['values'], therefore this block can be deleted. With form_state_values_clean you can remove added data by drupal.

webex_aggregate_list.inc:
- Line 24,31,32: Use t()
- Line 27-35: You could also theme('table', ...)
- Admin form - Why not using checkboxes?
- Line 70: Use $form['xxx'] => array('#markup' => ...) instead of $form['#suffix']. #suffix is not intended for this is.

webex_api_handler.inc:
- Line 136, 338, 339: Unused variables.
- Line 269: This seems wrong for me (dirname($_SERVER['SCRIPT_FILENAME'])). What do you want to achieve here?

webex_db_handler.inc:
- Line 27, 63, 94: The new db_select should be used here.
- Line 58: Documentation and function signature are not in sync.
- Line 168, 204, 237, 256, 259: Unused variable.

webex_output.inc:
- Line 27: Unused variable.
- Line 69: Where the function bundle_output is coming from?

WebexAPI.class.php:
- seems good to me.

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

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

Hi Josh, did you ever do any more work on this? I'm interested in helping.

Just fixing priority.

and title.

Still some issues on http://pareview.sh/pareview/httpgitdrupalorgsandboxjoshhoegen1866336git . Please set this to "needs review" once you are ready. See also https://drupal.org/node/532400

updated malformed anchor tag

No need to make me a maintainer - you should be the one to fix the problems, since this is your project application.

Please set this back to needs review once the pointed out problems are fixed.

Closing due to lack of activity. Feel free to reopen if you are still working on this application (see also the project application workflow).

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