OFBiz
  1. OFBiz
  2. OFBIZ-3180

SagePay payment gateway integrated

    Details

    • Type: New Feature New Feature
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: Trunk
    • Component/s: accounting
    • Labels:
      None

      Description

      SagePay payment gateway integrated. All the gateway parameters are configurable using the Payment gateway configuration, also the transaction types that are available in sagepay are configurable.

      1. httpclient-4.0.jar
        283 kB
        Abdullah Shaikh
      2. httpcore-4.0.1.jar
        169 kB
        Abdullah Shaikh
      3. jcip-annotations-1.0.jar
        2 kB
        Abdullah Shaikh
      4. OFBIZ-3180_SagePay payment gateway.patch
        99 kB
        Abdullah Shaikh

        Activity

        Show
        Hans Bakker added a comment - more info at: http://blogsbyabdullah.blogspot.com/2009/11/sagepay-payment-gateway-configuration.html
        Hide
        Abdullah Shaikh added a comment -

        oops sorry for the confusion .. yes it's working fine .. thanks

        Show
        Abdullah Shaikh added a comment - oops sorry for the confusion .. yes it's working fine .. thanks
        Hide
        Jacques Le Roux added a comment -

        A patch ? What for ?

        Show
        Jacques Le Roux added a comment - A patch ? What for ?
        Hide
        Abdullah Shaikh added a comment -

        Fine. I will provide a patch for this.

        Show
        Abdullah Shaikh added a comment - Fine. I will provide a patch for this.
        Hide
        Jacques Le Roux added a comment -

        Hi Abdullah,

        At r882337, I had to change mode to sagePayMode because MODE is a SQL reserved word. Could you please check it still works well ?

        Thanks

        Show
        Jacques Le Roux added a comment - Hi Abdullah, At r882337, I had to change mode to sagePayMode because MODE is a SQL reserved word. Could you please check it still works well ? Thanks
        Hide
        Abdullah Shaikh added a comment -

        Thanks Jacques, thank you very much

        Show
        Abdullah Shaikh added a comment - Thanks Jacques, thank you very much
        Hide
        Jacques Le Roux added a comment -

        Thanks Abdullah,

        Your (slightly modified) patch is in trunk at r882103

        Some small details I changed (else your code was very well done)

        • We son't need to put such comment in labels files (else we will finish with plenty) : <!-- labels for SagePay payment gateway -->
        • you forgot the white spaces between if and (
        • when possible, prefer enhanced for loops on iterators
        • in general please follow our Coding+Conventions. Most of the time I use Ctrl+Shft+F in Eclipse for that (only on code snippets and with a larger line than default - 150 instead of 80- else it breaks all) but I kept yours in the snippet beginning by
          if (vendorTxCode != null) { parameters.put("VendorTxCode", vendorTxCode); }
          

          because it was much more easier to read

        • I remove this code snippet (vars never read)
          String avsCv2 = (String) paymentResult.get("avsCv2");
          String addressResult = (String) paymentResult.get("addressResult");
          String postCodeResult = (String) paymentResult.get("postCodeResult");
          String cv2Result = (String) paymentResult.get("cv2Result");
          
        • as you I always use authorised (because it's how it's written ion French) but it's actually authorized (thanks to Eclipse automatic language check)
        Show
        Jacques Le Roux added a comment - Thanks Abdullah, Your (slightly modified) patch is in trunk at r882103 Some small details I changed (else your code was very well done) We son't need to put such comment in labels files (else we will finish with plenty) : <!-- labels for SagePay payment gateway --> you forgot the white spaces between if and ( when possible, prefer enhanced for loops on iterators in general please follow our Coding+Conventions . Most of the time I use Ctrl+Shft+F in Eclipse for that (only on code snippets and with a larger line than default - 150 instead of 80- else it breaks all) but I kept yours in the snippet beginning by if (vendorTxCode != null ) { parameters.put( "VendorTxCode" , vendorTxCode); } because it was much more easier to read I remove this code snippet (vars never read) String avsCv2 = ( String ) paymentResult.get( "avsCv2" ); String addressResult = ( String ) paymentResult.get( "addressResult" ); String postCodeResult = ( String ) paymentResult.get( "postCodeResult" ); String cv2Result = ( String ) paymentResult.get( "cv2Result" ); as you I always use authorised (because it's how it's written ion French) but it's actually authorized (thanks to Eclipse automatic language check)
        Hide
        Jacques Le Roux added a comment -

        Cool thanks

        Show
        Jacques Le Roux added a comment - Cool thanks
        Hide
        Abdullah Shaikh added a comment -

        I will keep that in mind, but I don't know how these lines came, as the other patches that I submitted weren't having them.

        Regarding the jar thing, before I could get your message I already raised that issue here https://issues.apache.org/jira/browse/OFBIZ-3224, we already had a discussion on this on ML long back, and you suggested to open a jira, so when again the same thing got back, I just opened the jira.

        Show
        Abdullah Shaikh added a comment - I will keep that in mind, but I don't know how these lines came, as the other patches that I submitted weren't having them. Regarding the jar thing, before I could get your message I already raised that issue here https://issues.apache.org/jira/browse/OFBIZ-3224 , we already had a discussion on this on ML long back, and you suggested to open a jira, so when again the same thing got back, I just opened the jira.
        Hide
        Jacques Le Roux added a comment -

        Of course don't update this one

        Show
        Jacques Le Roux added a comment - Of course don't update this one
        Show
        Jacques Le Roux added a comment - A detail I forgot, please when you use Eclipse internal command don't use finish and rather select project to avoid the 2 1st lines in the patch
        Hide
        Jacques Le Roux added a comment -

        Thanks Abdullah,

        Don't worry I will handle that.
        We have also to fill some places with these informations.
        If interested have a look at the commit I will do "soon"
        We also update this document http://docs.ofbiz.org/display/OFBADMIN/Libraries+Included+in+OFBiz

        Show
        Jacques Le Roux added a comment - Thanks Abdullah, Don't worry I will handle that. We have also to fill some places with these informations. If interested have a look at the commit I will do "soon" We also update this document http://docs.ofbiz.org/display/OFBADMIN/Libraries+Included+in+OFBiz
        Hide
        Abdullah Shaikh added a comment -

        Hi Jacques,

        The http*.jar are Release 4.0. Yes I just saw there are 4.0 betas there, shouldn't we remove the betas with the release version ? Should I raise a separate jira for this ?

        Thanks

        Show
        Abdullah Shaikh added a comment - Hi Jacques, The http*.jar are Release 4.0. Yes I just saw there are 4.0 betas there, shouldn't we remove the betas with the release version ? Should I raise a separate jira for this ? Thanks
        Hide
        Jacques Le Roux added a comment -

        Hi Abdullah,

        I had a new look. Do you know which are the http*.jar versions ? We have already 4.0-betas in OFBIz.. I suppose it's 4.0 stable for client as it has been announced the 2009-08-13 but for core ?

        Thanks

        Show
        Jacques Le Roux added a comment - Hi Abdullah, I had a new look. Do you know which are the http*.jar versions ? We have already 4.0-betas in OFBIz.. I suppose it's 4.0 stable for client as it has been announced the 2009-08-13 but for core ? Thanks
        Hide
        Abdullah Shaikh added a comment -

        Thanks Jacques for the review, apart from the space between if and (, I also have some more things, like providing a drop down for transaction types during payment gateway configuration, in mind, and also will look for more improvements, will submit the patch for them once this code gets in.

        Show
        Abdullah Shaikh added a comment - Thanks Jacques for the review, apart from the space between if and (, I also have some more things, like providing a drop down for transaction types during payment gateway configuration, in mind, and also will look for more improvements, will submit the patch for them once this code gets in.
        Hide
        Jacques Le Roux added a comment -

        I'd just put a white space between if and (

        Show
        Jacques Le Roux added a comment - I'd just put a white space between if and (
        Hide
        Jacques Le Roux added a comment -

        Thanks Abdullah,

        Very cursory review : looks good

        Show
        Jacques Le Roux added a comment - Thanks Abdullah, Very cursory review : looks good

          People

          • Assignee:
            Jacques Le Roux
            Reporter:
            Abdullah Shaikh
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development