This project integrate Unionpay into Drupal Commerce payment and checkout system.

Installation

cd sites/all/modules
git clone git://drupalcode.org/sandbox/jianhe/2164979.git commerce_unionpay

https://drupal.org/sandbox/jianhe/2164979

Comments

jian he’s picture

Title: [D7]commerce_unionpay » [D7] commerce_unionpay
mehul.shah’s picture

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

PA robot’s picture

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.

gobinathm’s picture

As per me the below 5 point are kind of important to move forward.

  1. you are using entity_metadata_wrapper & there is no dependency declared on entity
  2. As of now your module is supporting only Chinese Currency, may you can update this in your project page (or) module info decription
  3. Info file carries version number, normally this is discouraged as drupal.org package manager automatically does this.
  4. Parameters $order & $payment_method are unused in function commerce_unionpay_notify_validate, no sure if you want to remove them
  5. There are few unused variables in your module file as well, may be those can be removed for improved performance of this module on large sites

I consider above once as important one since they are improvement in your module. In-contrast the below once can be categories as nice to have, its you choice to change it (or) leave it as its today.

  • PAReview : There are issues reported related to coding standard related. These are not application blockers but its nice to clean them up, if you have time.
  • Your module is name as per info file is Unionpay but all your files are named commerce_unionpay, thou this is not a block it will be nice to same consistency.
  • README.txt : You are asking users to clone your repository for installation, i believe the preferred method is to use release version (or) dev snapshot. May be you would like to update this.
  • Not sure if you want to mention account information for testing, but i will leave this decision to you.
  • as of now you have hard-coded URL's in commerce_unionpay_server_url(), however you can provide an admin interface to manage these. In future if unionpay changes these URL's people would need to upgrade their module to get those changes, for which they will be looking for module release (or) they have to edit(hack) the module. I believe you would not want that.

Please manually review 3-4 Projects & have a review bonus tag added to your application. Project applications with review bonus always get priority
Also please go thru the application submission checklist which would be of great help.
Additionally you can also check Apply for permission to create full projectspage for clear instruction on how to have your project description updated.

Once you are done with necessary changes please change the status to Needs Review so that other contributors would start looking at this project.

jian he’s picture

Status: Active » Needs review

Hi gobinathm,

Very thanks for your review. I have just added the entity module dependency (I do not added it before because this module dependent on commerce, and commerce already added the entity dependency). And have fixed the README.

For the account number, yes this module already provided a admin page for setting it (See the commerce_unionpay_settings_form function).

jian he’s picture

Issue tags: +PAreview: review bonus
klausi’s picture

Issue tags: -PAreview: review bonus

Removing review bonus tag, you have not listed any reviews in the issue summary? Make sure to read the instructions again: https://drupal.org/node/1975228

heddn’s picture

Very close to RTBC.

  1. README doesn't list entity as required module
  2. Comments should all be on their own line and be full sentences. i.e. end with a period. See https://drupal.org/coding-standards/docs#inline
  3. commerce_unionpay_process_notify() $_POST['orderNumber'] might be empty. Check for !empty() ?
  4. commerce_unionpay_order_form() comment does not match function signature
  5. commerce_unionpay_server_url() @return doesn't match function signature ie. it could return NULL.
  6. Line 188, 189 duplicate lines 170,171
heddn’s picture

Status: Needs review » Needs work
PA robot’s picture

Status: Needs work » Closed (won't fix)

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.