drenther / upi_pay

Flutter plugin for UPI (Only in India)
MIT License
58 stars 71 forks source link

Supporting the new Android plugins APIs & compatibility with flutter 3 #62

Open GJJ2019 opened 2 years ago

GJJ2019 commented 2 years ago

Changes in this PR:

Reference GitHub Issues:

44 #60

Related Screenshots:

Screenshot 2022-05-18 at 12 19 27 PM
drenther commented 2 years ago

Thanks for the PR @reeteshranjan and I will take a look shortly

GJJ2019 commented 2 years ago

I have fixed those two unnecessary issues @drenther @reeteshranjan

reeteshranjan commented 2 years ago

@GJJ2019 thank you so much for this awesome work. It seems the Intent related changes are finally done. I had got stuck there trying to make the code move to new Flutter abstractions and removing the ones marked obsolete.

Could you please let us know what kind of testing have you done? When I expanded the package's support to multiple apps, there were some issues found and though the package itself was not at fault, it was crucial to add a workaround, which was neither violating nor breaking any standard. To ensure that everything works with such a major structural change, I would like to see that several UPI apps, say around 5 major and 5 minor, are verified to work fine i.e. a) payment goes through, b) payment response is collected and can be printed or shown in an alert box, c) unsuccessful payment responses are collected and can be printed or shown in an alert box.

GJJ2019 commented 2 years ago

Ok @reeteshranjan will try to do this stuff asap

mrinaljain commented 2 years ago

Tried using this for my production app, but I am not getting callbacks of success/failures after the payment.

GJJ2019 commented 2 years ago

HI @mrinaljain, i have added adding onActivityResult callback on my recent commit can u check that if its working or not.

FYI @reeteshranjan @drenther

GJJ2019 commented 2 years ago

My upi account is not working it seems so I'm not able to test properly @reeteshranjan

reeteshranjan commented 2 years ago

My upi account is not working it seems so I'm not able to test properly @reeteshranjan

What is the error? Is it specific to a particular app? Which apps have you tried to test?

GJJ2019 commented 2 years ago

Im not sure but i have raised complaint about it my bank will proceed, its something to do with my bank

GJJ2019 commented 2 years ago

I'm sure plugin will work just fine but we need to test it properly. I'm not able to do that

mrinaljain commented 2 years ago

HI @mrinaljain, i have added adding onActivityResult callback on my recent commit can u check that if its working or not.

FYI @reeteshranjan @drenther

HI @GJJ2019 I JUST CHECKED AGAIN , still I am not receiving any callbacks for success or failure.

Basically I want to await for the upi transaction result

UpiTransactionResponse _UpiTransactionResponse = await UpiPay.initiateTransaction()

mrinaljain commented 2 years ago

any updates on this one ? @GJJ2019

GJJ2019 commented 2 years ago

I'm not sure will check this week & fix, test all the issues for sure :)

GJJ2019 commented 2 years ago

I have fixed that activity result not getting called functionality There is open issue regarding that i used one of the workaround PluginRegistry.ActivityResultListener: not triggered

can u test the payment @mrinaljain

FYI @reeteshranjan @drenther

MDSADABWASIM commented 2 years ago

@drenther @reeteshranjan @mrinaljain Please review it, the issue that this PR is fixing is important.

GJJ2019 commented 2 years ago

yes but can u also provide test result so that its easy to merge the code @MDSADABWASIM

MDSADABWASIM commented 2 years ago

yes but can u also provide test result so that its easy to merge the code @MDSADABWASIM

With these changes, I can build successfully, but payment is failing for me, this is the first time I'm integrating this plugin so don't know if the issue is caused by changes or my implementation @GJJ2019

mrinaljain commented 2 years ago

I'll test this plugin over the weekend, and update here!

gururaja-kambala commented 2 years ago

yes but can u also provide test result so that its easy to merge the code @MDSADABWASIM

With these changes, I can build successfully, but payment is failing for me, this is the first time I'm integrating this plugin so don't know if the issue is caused by changes or my implementation @GJJ2019

@MDSADABWASIM what is the error you are getting? is it failing a) on getting app link b) invoking the app from app list, c) after going inside the upi app?

Also is it possible to share error message ?

if your answer is 'b' can you try with : https://github.com/gururaja-kambala/upi_pay.git

crossxsecure commented 2 years ago

Hi @GJJ2019 I'm following this PR since last week for the Only safe (?.) or non-null asserted (!!.) calls are allowed on a nullable receiver of type Activity? error, and it seems this is fixed now in the above PR's. Can you please quickly help me which version I need to upgrade in pubsec.yaml file.. Currently I'm using upi_pay: ^1.0.1 and seeing the above error. By scrolling through other PR's I could see somewhere version 2.0.0 but that version seems to be not exist/public.

GJJ2019 commented 2 years ago

@drenther is right man to ask

GJJ2019 commented 2 years ago

I think activity will get initialise when flutter engine fires so mostly it will be non nullable without some edge cases

crossxsecure commented 2 years ago

Hi @GJJ2019 I'm following this PR since last week for the Only safe (?.) or non-null asserted (!!.) calls are allowed on a nullable receiver of type Activity? error, and it seems this is fixed now in the above PR's. Can you please quickly help me which version I need to upgrade in pubsec.yaml file.. Currently I'm using upi_pay: ^1.0.1 and seeing the above error. By scrolling through other PR's I could see somewhere version 2.0.0 but that version seems to be not exist/public.

@drenther - Can you please help here a bit, it will be highly appreciated.

GJJ2019 commented 2 years ago

@crossxsecure I have created another plugin for easy_upi_payment

crossxsecure commented 2 years ago

@crossxsecure I have created another plugin for easy_upi_payment

Highly appreciated your efforts @GJJ2019, my core requirement is I need it for iOS and I believe this is the only plugin available for it?

Moreover, I went through your plugin and that's really a great offering, but I feel you should also include a optional bundleName/packageName in the input parameter so that it can be used utilized as per the custom needs as well. Apart from that it's awesome and a big thanks to all plugin builders.

drenther commented 2 years ago

Hi @GJJ2019 I'm following this PR since last week for the Only safe (?.) or non-null asserted (!!.) calls are allowed on a nullable receiver of type Activity? error, and it seems this is fixed now in the above PR's. Can you please quickly help me which version I need to upgrade in pubsec.yaml file.. Currently I'm using upi_pay: ^1.0.1 and seeing the above error. By scrolling through other PR's I could see somewhere version 2.0.0 but that version seems to be not exist/public.

@drenther - Can you please help here a bit, it will be highly appreciated.

version 2.0.0 is this PR and since this PR is not merged yet you can understand why that is not published on pub.dev Manual tests for this PR are still failing for most people. Once this PR is tested and merged it will be published under 2.0.0.

reeteshranjan commented 2 years ago

@mrinaljain and anyone else who were looking into testing this PR, could you please provide any update on the testing?

gururaja-kambala commented 2 years ago

Manual tests for this PR are still failing for most people

Is there any place I can get details of those cases? is it on return status post transaction? or before invoking app?

Is it because, it is not plugin will not be compatible with lower version of flutter, once we handle changes required for flutter 3.0?

mrinaljain commented 2 years ago

@mrinaljain and anyone else who was looking into testing this PR, could you please provide any update on the testing?

@reeteshranjan I tested the updated PR and It's working fine. Basically, I wanted to await the UPI transaction result which was previously not working.

but now I am able to use the code below

UpiTransactionResponse _UpiTransactionResponse = await UpiPay.initiateTransaction()

reeteshranjan commented 2 years ago

@mrinaljain and anyone else who was looking into testing this PR, could you please provide any update on the testing?

@reeteshranjan I tested the updated PR and It's working fine. Basically, I wanted to await the UPI transaction result which was previously not working.

but now I am able to use the code below

UpiTransactionResponse _UpiTransactionResponse = await UpiPay.initiateTransaction()

Thanks! I will verify with my setup of multiple apps on Android and iOS.

mrinaljain commented 2 years ago

@reeteshranjan Any updates on this PR , when can we start using for production build ?

reeteshranjan commented 2 years ago

@reeteshranjan Any updates on this PR , when can we start using for production build ?

Sorry have been very busy. No bandwidth. Hopefully, other devs can pitch in for testing.

reeteshranjan commented 2 years ago

I have re-asked my questions to NPCI about #38 . If on Twitter, please do look at https://twitter.com/reeteshr08/status/1561430970930081793 and support.

reeteshranjan commented 2 years ago

Testing Update

Updating results of few apps I have tested so far.

Device Details

Device: Redmi Note 10 Android version: 12

Codes

UPI Setup Statuses (USS)

  1. Works
  2. Reserved
  3. Bank account holders only
  4. Error post UPI setup
  5. UPI setup fails
  6. UPI disabled
  7. Under maintenance
  8. Device verification fails
  9. Device verification SMS fails
  10. Unstable

Transaction Functioning Status (TFS)

  1. Works
  2. UPI ref not returned
  3. Does not return to caller
  4. Risk Threshold Exceeded Error
  5. Daily Transaction Limit Exceeded Error
  6. Security Error
  7. Payment fails
  8. Payment form is incomplete
  9. Payment form not shown
  10. Bails out stating invalid params
  11. Bails out refusing the recipient
  12. Unstable behaviour for intent call
  13. Returns immediately with fail/canceled status

Findings

Tested various apps. Depending on the findings of testing, a PR Health Indication is derived. It's one of:

  1. OK: PR works properly
  2. Needs Review: There seems to be a regression or a behavioural problem that was not seen with earlier version of code and app tested when APPS.md was created. We should try to test the older version of the code with the current version of the app to eliminate any app version change issues. If the app current version has the same issue with older code, then the indication will change to OK.
  3. Cannot Say: Difficult to mark the indication as either OK or Needs Review.

Last date of testing PR: 24-Aug-2022

PR Health Indication: Needs Review

Name Last Version Last TFS Current Version Current USS Current TFS "Unverified Source" warning Crash Stack Snapshot(s) Test UPI response
BHIM AU 1.0.11 7 1.4.1 Works 12 aupay-crash.txt
BHIM Baroda Pay 3.4.10 4 3.4.16 Works 12 bob-crash.txt
BOI UPI 2.0.3 3 2.0.11 Works 13 boi-upi-log.txt
BHIM CSB Pay 1.2.1 7 1.2.2 Works 12 csb-pay-crash.txt
BHIM IndusPay UPI 3.2.3 7 3.2.71 Works 7, 3 Yes Screenshot_2022-08-24-11-45-17-935_com mgs induspsp Screenshot_2022-08-24-11-47-26-851_com mgs induspsp induspay-log.txt
BHIM Maha UPI 1.7.0 Worked 1.7.4 Works 3 maha-upi-crash.txt maha-upi-log.txt
Mobikwik 22.5.2 Worked 22.41.1 Works 6, 3 Screenshot_2022-08-24-12-17-18-028_com mobikwik_new Needs manual canceling mobikwik-log.txt

PR Health Indication: Cannot Say

Name Last Version Last TFS Current Version Current USS Current TFS "Unverified Source" warning Crash Stack Snapshot(s) Test UPI response
BHIM Cent UPI 2.0.11 9 2.0.20 10 - cent-upi-log.txt
BHIM DCB UPI 2.1.16 Worked 2.1.16 9 -
BHIM Equitas API 2.0.1 12 2.2.0 Works 4, 3 Yes Screenshot_2022-08-24-11-39-42-512_com equitasbank upi Screenshot_2022-08-24-11-40-18-323_com equitasbank upi equitas-upi-log.txt
BHIM LOTZA PAY 3.5.1 12 4.0 Works 12 Screenshot_2022-08-24-12-01-41-805_com upi federalbank org lotza Screenshot_2022-08-24-12-03-59-414_com upi federalbank org lotza Needs manual canceling lotza-pay-log.txt

PR Health Indication: OK

Name Last Version Last TFS Current Version Current USS Current TFS "Unverified Source" warning Crash Stack Snapshot(s) Test UPI response
Amazon Pay 22.7.0.330 5 24.14.0.300 Works 5 Screenshot_2022-08-24-11-15-57-135_in amazon mShop android shopping amazon-pay-log.txt
BHIM Axis Pay 2.6.10.10 3 2.7.17 Works Works axis-pay-log.txt
BHIM Bandhan UPI 1.6.2 7 1.7.1 Works 7 Screenshot_2022-08-24-11-01-27-095_com fisglobal bandhanupi app bandhan-upi-log.txt
BHIM 2.6.5 4 2.9.7 Works 4 Screenshot_2022-08-24-11-16-27-161_in org npci upiapp bhim-log.txt
BHIM CUB eWallet 1.6.1 Worked 1.6.3 Works Works Yes cub-ewallet-log.txt
BHIM DLB UPI 1.1.0 4 1.1.5 Works 6 Screenshot_2022-08-24-11-37-36-419_com lcode dlbupi dlb-upi-log.txt
GPay 126.1.2 5 159.1.1 Works 4 gpay-log.txt
BHIM IDFC UPI 1.1.15 10 1.1.15 Works 10 Screenshot_2022-08-24-11-44-06-058_com fss idfcpsp idfc-upi-log.txt
BHIM JetPay 2.0.0 12 2.0.8 Works 12
BHIM KBL UPI 1.2.7 10 1.2.8. Works 4 Screenshot_2022-08-24-11-55-45-853_com lcode smartz kbl-upi-log.txt
KVB Upay 1.1.24 7 1.1.30 Works 4 Screenshot_2022-08-24-11-57-03-142_com mycompany kvb kvb-upay-log.txt
Mi Pay 2.14.0-g Worked 2.15.1-g Works Works mi-pay-log.txt
reeteshranjan commented 2 years ago

How to use the above updates?

Testing focused on finding any cases where the updated code caused any new communication and response returning issues, which could imply regression in integration with payment apps.

There are few such cases found, for which the PR Health Indication column has been marked as Needs Review. In several of these cases, there is a crash stack log also added. Initial look at these crashes points to some regression, to me. Need @GJJ2019 and other devs to take a better look.

What's the current status of merging this PR?

Should be done once all rows with PR Health Indication marked as Needs Review are resolved as not a problem with the new code.

More testing?

Several more apps need to be tested on Android and iOS. Will update here as more testing is done. However; given that we found some cases where there could be a regression, we should look at these cases and attempt resolving them first.

GJJ2019 commented 2 years ago

I'm not sure about need review thing only thing i did was migrated api to v2 that's it

reeteshranjan commented 2 years ago

I'm not sure about need review thing only thing i did was migrated api to v2 that's it

Let's just take one case. Could you please install and configure Maha UPI (Bank of Maharashtra app), and reproduce the crash? It's related to intents. Exactly what should not be happening with any major upgrade where regression of working stuff happens.

If we could determine that the crash is not because of the upgrade in our package; but due to some compatibility issue in the app we are trying to use, it's fine and we can move ahead with that properly marked. I'll update the APPS.md accordingly that a particular working app is no longer supported, and that's not due to what we have done. Point is maintaining the detailed status of support data for several BHIM UPI apps with utmost transparency, which is what my objective is with APPS.md.

I definitely want at least @GJJ2019 and @drenther to look into my test results and come up with the calls/explanations.

GJJ2019 commented 2 years ago

just one question @reeteshranjan are those same issues happening in previous version ?

reeteshranjan commented 2 years ago

just one question @reeteshranjan are those same issues happening in previous version ?

  1. Please check the table of results. It shows what's the previous version testing behaviour.
  2. I have marked only those rows as "review needed" where there is an outright regression or behaviour that should be investigated.
reeteshranjan commented 2 years ago

Updated the testing findings comment to organise the information better. @GJJ2019

nayanAubie commented 1 year ago

@reeteshranjan When can I expect to merge this PR?

grit-ameen commented 1 year ago

@GJJ2019 still showing a deprecated issue, why is this pr still not merged, could anyone help?

altafkhan8719 commented 1 year ago

when are u going to release??

altafkhan8719 commented 1 year ago

please merge the pr and release it. I am using it in production and i have to upgrade to Flutter3

reeteshranjan commented 1 year ago

@nayanAubie @altafkhan8719 @grit-ameen please check the history to see that testing of the PR threw out few things that should be looked at before merging.

drenther commented 1 year ago

This PR is not ready to be merged as @reeteshranjan has pointed out in details here

Both @reeteshranjan and I do not currently have the capacity as of now to take this PR up and wrap it up.

If any of you can, in this PR or a new one, contribute the code needed along with thorough test cases covered (in the lines of what @reeteshranjan has added in the repo's App support documentation page as well as the comment shared on this PR). We would be happy to review and merge.