UC Luottokunta provides a Finnish Luottokunta Credit Card payment method for Ubercart. This module also provides two Luottokunta-based gateways: Luottokunta Redirect and Luottokunta XML. Support for UC Recurring Payments and Subscriptions is provided using the eInvoicing -service, which uses the XML-gateway for payments without authorization.

There is one similar module, which provides only basic integration with luottokunta: http://drupal.org/project/uc_finnish_payments
Project maintainer states in its readme that the Luottokunta integration is untested. Also it does not fully support the XML API or the Recurring payments. I tried to first make a submodule of the uc_finnish_payments -module but the approach needed to be more specific to work well (IMO).

Link: http://drupal.org/sandbox/afox/1146792

Comments

13rac1’s picture

13rac1 commented
Status: Needs review » Needs work

This module duplicates (some) functionality. It should be integrated with or patched into the http://drupal.org/project/uc_finnish_payments project. According to the project page, the maintainers are looking for co-maintainers.

afox’s picture

afox commented

I agree that this duplicates some functionality but here's my take on why it should exist as its own module:

1) Finnish payments module is a generalization of Finnish payments. However, Luottokunta (card payment service provider) provides many services that are not available on any or all banks, like recurring payments, XML API, different testing requirements, etc. Thus it requires a lot more exceptions from the Luottoukunta-part compared to the bank-payment functionalities. Only the functionalities that the UC Finnish payments provide are common to all.
2) To maintain a submodule inside a project that is bigger than the project itself is a bit overkill.
3) Luottokunta is not a bank. It's a card payment service provider. If a site uses payments it most likely uses one of the three options:
a) One-contract solutions, like Suomen Verkkomaksut (Finnish Payments), which is being developed at http://drupal.org/sandbox/oakulm/1119134 or checkout.fi (http://drupal.org/project/uc_checkoutfi)
b) Separate contract with banks (UC Finnish Payments)
and/or
c) Separate contract with Luottokunta (the only card payment service provider in Finland).

For users, Credit card and bank payments are a logical difference. It should also be differentiated in the module-end. Luottokunta should logically be compared with the likes of authorize.net, etc. instead of banks.

In my opinion a dedicated module for Luottokunta should be a more reasonable choice.

afox’s picture

afox commented
Status: Needs work » Needs review
jordojuice’s picture

jordojuice commented
Component: new project application » module
Issue tags: +PAReview: Ubercart, +pdx-code-review

Sorry about the long wait! Hopefully we can interest an Ubercart familiar reviewer for this. Tagging appropriately.

As for module duplication, I think you might have made a good argument for maintaining the project separately, but it may depend on how other members or the git administrators feel about this.

jordojuice’s picture

jordojuice commented
Status: Needs review » Needs work

Some notes:

You should add a README.txt file detailing installation and setup of the module.

I think I'm against the practice of deleting variables in a query in hook_uninstall(). Fir one, I think you need to clear the cache. Secondly, using LIKE runs the risk of deleting variables set by modules with a namespace beginning with the same name in the future. Most modules I see use variable_del() just fine here.

Includes should be including conditionally if possible.

Documentation needs to be cleaned up. Be sure to run the module through Coder module on minor (most) and consult the coding standards (http://drupal.org/coding-standards). Comments should begin with a space and capital letter and end with a period in sentence format. Function doc blocks need to be directly above the function or it will not be associated with the function.

All functions must be documented, preferably with @param or @return and similar references for functions that are part of the module's API. See documentation and commenting standards at the previous link.

There are empty lines at the end of files (there should be one), some random spaces in documentation (like before ending periods), and @file references should have description on the next line. So those are the things I was referring to with cleaning up documentation. They are certainly not show stoppers, but are tasks that should be completed for consistency (though certain documentation standards must be followed properly).

Once these issues are corrected and committed, please set this issue back to "Needs review"

oakulm’s picture

oakulm commented

This project and this http://drupal.org/sandbox/oakulm/1119134 are two separate methods of payment. I don't see any reason to combine these to projects.

3) Luottokunta is not a bank. It's a card payment service provider. If a site uses payments it most likely uses one of the three options:
a) One-contract solutions, like Suomen Verkkomaksut (Finnish Payments), which is being developed at http://drupal.org/sandbox/oakulm/1119134 or checkout.fi (http://drupal.org/project/uc_checkoutfi)
b) Separate contract with banks (UC Finnish Payments)
and/or
c) Separate contract with Luottokunta (the only card payment service provider in Finland)

It's like comparing this project to Paypal.

misc’s picture

misc commented

The applicant has been contacted to ask if the application is abandoned.

afox’s picture

afox commented

@MiSc This project application is not abandoned. I had actually forgotten that I needed to fix the issues mentioned by @jordojuice. I think was more waiting for an Ubercart-familiar reviewer to take a look at the application. I have been working on this module for our site locally, so it has been under development. I'm planning to clean up some site-specific things and fix the issues mentioned in this one and also in my other modules during the next few weeks so I'll get all of them released.

@oakulm there were no plans in combining the modules. I merely compared the different use cases in Finnish payment solutions.

misc’s picture

misc commented

@afox has again been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

afox’s picture

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

I was Git vetted through another project. This project is still under active development, but the application can be considered abandoned for now. I will create a new project later if I ever get the chance to rework this.