Features
This module provides a Drupal Commerce payment method to embed the payment services provided by LiqPay.
It efficiently integrates payments from various sources such as:
- credit cards;
- cash via self-service terminals (offline payments);
- email receipts;
- privat24 banking or liqpay accounts.
Screenshots
Settings page (payment rule config)
LiqPay offsite payment page
Service description
LiqPay is payment getaway from PrivatBank.
PrivatBank is the largest commercial bank in Ukraine with official offices in some EU countries (Ukraine, Latvia, Georgia, Portugal, Italia, Cyprus).
Other or similar projects overview
There are no full projects for such integrations at drupal.org.
There are few sandbox projects but no one works correctly (used old broken API or did not supported for years).
Project page
Link: https://www.drupal.org/project/commerce_liqpay
Code repository: http://cgit.drupalcode.org/commerce_liqpay
git clone command
git clone --branch 7.x-1.x https://git.drupal.org/project/commerce_liqpay.git
cd commerce_liqpay
Reviews of other projects:
Comments
Comment #2
PA robot CreditAttribution: PA robot commentedFixed the git clone URL in the issue summary for non-maintainer users.
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.
Comment #3
sourabhutani CreditAttribution: sourabhutani as a volunteer and at Kellton Tech Solutions Ltd commentedAutomated Review
There is some coding standard errors to fix : http://pareview.sh/pareview/httpgitdrupalorgsandboxbohart2680989git
Individual user account
Yes
Duplication
No
Master Branch
Yes. Branch 7.x-1.x
Licensing
Yes. Follow
3rd party assets/code
Yes follow. Do not contains any 3dr party.
Code long/complex enough for review
Yes. Well documented. Extending views sort, fields and argument.
Secure code
None security issue found. Looks good.
Coding style & Drupal API usage
After a manuel Review, everything seems to be ok. Perhaps, just some recommandations :
-README.txt is missing.That should be in you module.
-On line 143 in commerce_liqpay.module file, Only string literals should be passed to t() .
Module works fine, without errors.
Comment #4
bohartHi there, @sourabhutani.
Thanks for your time and feedback.
README file added.
This is a warning but not an error. A lot of contrib modules pass to t() string variables from admin UI instead of string literals.IMHO, that's not an issue, is not it?
Module has been update to use commerce_i18n_string() for translation instead of t().
pareview.sh for this sandbox did not find any errors.
Comment #5
sanjayk CreditAttribution: sanjayk commentedManual Review
1) On line 143 in commerce_liqpay.module file, Only string literals should be passed to t()
2) On line 29 in commerce_liqpay.module file, Only string literals should be passed to t()
3) On line 172 in commerce_liqpay.module file, Only string literals should be passed to t()
4) On line 120 in commerce_liqpay.commerce.inc file, Only string literals should be passed to t()
Comment #6
bohartAll issues around manual and automated reviews are fixed.
Comment #7
anpolimusI've tested this module locally and approve that defined functionality is working properly.
Comment #8
podarokHey there
I've finished looking through the code of module and found it looks nice.
I see no additional issues that could be the blockers for promoting this sandbox to a full project
Let's do it
Comment #9
niko- CreditAttribution: niko- as a volunteer commentedHey @bohat
It seems that this module not handle multiple payment rules - please check as example for you
http://cgit.drupalcode.org/commerce_interkassa/tree/commerce_interkassa.... and http://cgit.drupalcode.org/commerce_interkassa/tree/commerce_interkassa....
All other looks good
Comment #10
niko- CreditAttribution: niko- as a volunteer commentedComment #11
niko- CreditAttribution: niko- as a volunteer commentedAlso you should check if server_url really have call on any payment status change. If not, you should handle "fallback" for this status
Please check as example is_interaction parameter http://cgit.drupalcode.org/commerce_interkassa/tree/commerce_interkassa....
Comment #12
PA robot CreditAttribution: PA robot commentedClosing 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.
Comment #13
bohartHi @niko-,
Thanks for the review.
I got a need to reuse the module,
and re-check everything here.
1) Multiple payments methods are tested. All works as expected. There is no need to have load placeholder in hook menu, implement custom load method for our instance and so on. This module (as well as Commerce core) always use method full machine name (like
liqpay|rules_you_custom_rule_name
).2) Additionally, for better support of multiple rules, was added an ability to set different descriptions for every payment rule: #2924116: Multiple payment rules
3) We have all needed fallbacks ( for sandbox requests, response signature validate, amount/order/payment re-checks, etc). See `commerce_liqpay.pages.inc` for more details.
Thanks!
Comment #14
max-kuzomko CreditAttribution: max-kuzomko as a volunteer commentedHi all,
I did a review: checked Drupal code styles (looks code) and did some basing testing.
Found one warning, for which I have created a patch:
https://www.drupal.org/project/commerce_liqpay/issues/2931219#comment-12...
The remaining things to check:
1. Test the payment process itself.
2. Fix documentation for functions (parameters are not documented well).
Comment #15
MatroskeenHello,
@max-kuzomko, thanks for the review.
Your patch was committed! The issue is fixed.
Regarding this point,
It was fixed in this issue: #2932126-3: Document parameters for custom functions
Comment #16
MatroskeenComment #17
MatroskeenAdded references to the manual reviews and added review bonus tag.
Comment #18
Mario SteinitzSome minor code style and documentation issues left.
Review of the 7.x-1.x branch (commit 64441a6):
hook_help()
. See https://www.drupal.org/docs/develop/documenting-your-project/module-docu... .This automated report was generated with PAReview.sh, your friendly project application review script.
Comment #19
MatroskeenMinor code style and documentation issues were fixed here: #2947881: PAReview.sh minor issues .
Probably, some automated tests will be added later. I believe they shouldn't block this module to go.
Moved back to "Needs review".
Comment #20
abrammChecked code, no complains. RTBC.
Comment #21
bohartCan't find a rule how to use priorities in `security advisory coverage applications` project. Hopefully, 2 years for the issue should be good enough to update the priority.
Comment #22
andypostDoc is https://www.drupal.org/node/475848
According it
- needs stable release (rc1 now)
- needs test coverage
Comment #23
apadernoThose are the documentation for which versions are checked from the Drupal Security team. Applying to be able to opt in the security coverage doesn't have those requirements, but its own requirements, which say:
Comment #24
apadernoThank you for your contribution!
I am going to update your account so you can opt into security advisory coverage now.
These are some recommended readings to help with excellent maintainership:
You can find more contributors chatting on the IRC #drupal-contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.
I thank all the dedicated reviewers as well.