Commerce ibis module

Welcome maris to the community,

Firstly, you are working on a Master branch, you should be using a version specific branch.
More about this here - http://drupal.org/node/1127732

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 :-)

Hi Maris,

the PHP code looks good but there are a few Drupal specific things which could be improved.

1. Getting the commerce_order

At the moment you are doing

    $order_id = arg(3);
    $wrapper = entity_metadata_wrapper('commerce_order', $order_id);

which works but isn't very portable. Commerce module gets the order like

    global $user;
    $order = commerce_cart_order_load($user->uid));

which can be used in any function.

2. Use of user input to get order details

This is a possible security issue and really needs to be fixed.
Eg stuff like

    $amount = $_REQUEST['total'];

You need to use the Form API to submit your forms.
At the moment you are doing

    $form['#action'] = base_path() . 'cart/ibis/make/dms';

and defining the 'cart/ibis/make/dms' route in hook_menu().
What you need to do is instead

    $form['submit'][] = 'commerce_ibis_make_dms';

and then make the commerce_ibis_make_dms($form, &$form_state) callback available without having to load the separate file.
Convention is to name the submit function FORM_CALLBACK_NAME_submit, so in this case you would rename commerce_ibis_make_dms to commerce_ibis_make_dms_form_submit.
If you need to do validation you can do

    $form['validate'][] = 'commerce_ibis_make_dms_form_validate';

and commerce_ibis_make_dms_form_validate will return a form_error() if validation fails.

Form API reference
http://api.drupal.org/api/drupal/developer!topics!forms_api_reference.ht...

Couple of articles on why we need to use the Form API to protect against CSRF attacks.
http://pixeljets.com/blog/csrf-avoid-security-holes-your-drupal-forms
http://crackingdrupal.com/blog/greggles/protecting-your-drupal-module-ag...

3. Setting correct status for watchdog and drupal_set_message

You've got few error checks which is great to see, but you should set the correct type and status for the watchdog and message respectively.
Eg

    watchdog('ibis', 'DMS payment make failed: !result', array('!result' => $resp), WATCHDOG_ERROR);
    drupal_set_message(check_plain(t('DMS payment make failed: !result', array('!result' => $resp))), 'error');

http://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/watch...
http://api.drupal.org/api/drupal/includes%21bootstrap.inc/function/drupa...

4. Debug code present in module

I think this debug code should probably be removed

  // Use localhost IP address to make testing procedure for blakclist IP.
  // $ip = '192.168.1.2'.

You could leave a comment in saying how to debug if you want, but the commented out code line means that every dev who looks at this is going to have to work out what it's for and why it's been commented out. Makes reviewing the code a little bit harder.

5. Use of drupal_goto

You should avoid using this function if at all possible

http://xkcd.com/292/

If you use the Form API as suggested in (3) you can set $form['#redirect] to set where the user will be redirected to after the form submit callback has completed. There is a bunch of stuff that Drupal does after the callback has completed that won't happen if you issue an immediate redirect with drupal_goto().

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.

Forgot to add git clone link and drupal version.