[D7] Video Embed AOL

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.

Getting some errors, see http://pareview.sh/pareview/httpgitdrupalorgsandboxbogdan19882176241git

Hello dstorozhuk,

Please add git clone command :
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/Bogdan1988/2176241.git video_embed_aol

I suggest using require_once DRUPAL_ROOT rather than module_load_include() video_embed_aol.module - line 10 :
require_once DRUPAL_ROOT . '/' . drupal_get_path('module', 'video_embed_aol') . '/video_embed_aol.video_embed_field.inc';

In the creation of a new field I obtain an error :
Undefined index: use_id in video_embed_aol_form_field_ui_field_edit_form_alter() - (line 35)

For video_embed_aol.js, add semicolon lines 15, 28, 32, just for perfect.

Other than that, I think it looks okay.

A few issues:

Web service providers have a tendency to, ahem, change things. It might be a good idea to make

define('VIDEO_EMBED_AOL_API_SINGLE_VIDEO_ENDPOINT', 'http://api.5min.com/video');

a setting that can be changed by the user.

I'm not sure that

$cached = & drupal_static(__FUNCTION__ . $video_id, '');

is equivalent to

$cached = &drupal_static(__FUNCTION__ . $video_id, '');

In

$parsed = @parse_url($url);

What error are you trying to suppress? Is it necessary?

In video_embed_aol_video_embed_handler_info
your array keys should use '', not "".

When you attach behaviors in your JavaScript, you should ensure that you are only attaching to new items, by using context.

I see no reason why module_load_include is a bad idea here. It provides consistency and is The Drupal Way ™

Hello,

- for module_load_include, sorry for I thought that it was necessary to use it for include of another module.

- for issue with "Undefined index: use_id"

1. I installed a new drupal
2. I installed modules Chaos Tools, Entity API, Video Embed Field and Video Embed AOL
3. Clear all caches
4. Then I go structure -> content types -> manage fields (article) -> Add new field (Field type text) -> save -> save field settings

You may use module_load_include to load any include files, whether of this module, or of other modules, though if it is another module, it pays to verify the module is installed first.

@dstorozhuk

I think all of that makes sense.

You are using a variable called "$posible_video_id" in multiple places - nitpicky, but its spelled wrong - might want to change it to "$possible_video_id" for readability.

Line 110: this entire function could be simplified by simply returning FALSE after all the if statements - you don't need to have 3 "return FALSE" statements. The outer "if" statement can also be removed as "if (isset($parsed['path']))" performs the same check as "if ($parsed)"

Line 190: You might consider a regular expression to validate the URL rather than exploding the string and checking the parts. It also looks like you are only checking the "video" part of the URL - don't you want to check the "on.aol.com" part as well? Right now it looks like I could submit "http://www.youtube.com/video/testvideo" and have the URL accepted.

Automated review
a lot of errors. Please see
http://pareview.sh/pareview/httpgitdrupalorgsandboxbogdan19882176241git

Manual view
$ git clone --branch 7.x-1.x http://git.drupal.org/sandbox/Bogdan1988/2176241.git video_embed_aol
Cloning into 'video_embed_aol'...
fatal: repository 'http://git.drupal.org/sandbox/Bogdan1988/2176241.git/' not found

Looks like this was already promoted to a full project: https://www.drupal.org/project/video_embed_aol