Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PayInRecurringId is missing in Payins (get) #546

Closed
H4wKs opened this issue Mar 21, 2022 · 9 comments · Fixed by #552
Closed

PayInRecurringId is missing in Payins (get) #546

H4wKs opened this issue Mar 21, 2022 · 9 comments · Fixed by #552

Comments

@H4wKs
Copy link
Contributor

H4wKs commented Mar 21, 2022

Hey guyz,

While implementing recurring payment using the new CIT / MIT features, I noticed that MangoPay API return the PayInRecurringId on Payins (GET) calls, but unless I am wrong, this information is missing in the SDK for the payins_get function, and it would really nice to have this informations to avoid storing it earlier in the process before any valid payment have been done. I think it's also a problem that unless you store that information locally on creation, it's simply unavailable anywhere using the SDK.

The main reason of this behavious is that RecurringPayinRegistrationId was added to PayInRecurring which extend PayIn, but is therefore never returned on Payins get as this function return a PayIn object.

I am happy to help here, I can even make a PR, but I am not sure what would be the best way to go here.

  • We could add RecurringPayinRegistrationId to a specific payment detail, like currently it would only make sense inside of PaymentDetailsCard, but I am not sure this is nice.
  • We could move RecurringPayinRegistrationId from PayInRecurring class to PayIn class, and keep PayInRecurring as en empty class, just extending PayIn class, this way all the current code will still work the same and we would have a clean logic using PayInRecurring when it make sense from a code point of view, but the RecurringPayinRegistrationId value is stored in the main PayIn object / class. The downside of this is that it make no sense to have RecurringPayinRegistrationId available in code completion when you create a new PayIn object and could lead to confusion.

What you think ? Any other idea @fredericdelordm @SoloJr of achieving this in a clean way ?

Cheers,

Marc.

@chrs-myrs
Copy link

Seems to me that all payin info specific to recurring pay-ins is missing from the API. Managed to avoid needing this by using hooks and performing CITs synchronously.

@H4wKs
Copy link
Contributor Author

H4wKs commented Mar 31, 2022

Thank you for trying to help out here @chrs-myrs, and sharing your solution.

Seems to me that all payin info specific to recurring pay-ins is missing from the API.

It's not missing from the API, only from the SDK. The API give you the all the information you need unless you looking for more than the RecurringPayinRegistrationId ?

Managed to avoid needing this by using hooks and performing CITs synchronously.

I already have a system where I send an email to the customer warning him that his subscription is going to end, and that if he want to keep it, he need to renew it following the link or connecting to the platform. It's why I want to implement MIT in order to skip this step whenever a CIT is not required.

I actually made a fork and made the changes I need to work with CIT / MIT, I just wanted to discuss what would be the best way to implement it to avoid updating my own code later, but this is just taking too much time.
I did the choice to move RecurringPayinRegistrationId from PayInRecurring class to PayIn class so I can access this information on all payIns call. I will make a PR with my update, and in the mean time I will run my fork version of the SDK instead of the official version.

@fredericdelordm
Copy link
Contributor

Hello @H4wKs,

Thank you for the feedback. We are going to work on it this week.

@H4wKs
Copy link
Contributor Author

H4wKs commented Apr 1, 2022

Thank you for the feedback. We are going to work on it this week.

I just hope you're not making a first April fool joke here @fredericdelordm 😂

@fredericdelordm
Copy link
Contributor

@H4wKs Haha, not joking. Task in Jira for @SoloJr :)

@H4wKs
Copy link
Contributor Author

H4wKs commented Apr 14, 2022

Any news / update / ETA about this @fredericdelordm / @SoloJr ?
Have you at least decided how / where you going to implement this ?

@fredericdelordm
Copy link
Contributor

Hello @H4wKs,

I was ooo these days. Let me circle back internally and get back to you regarding this

@SoloJr
Copy link
Contributor

SoloJr commented Apr 28, 2022

Hi @H4wKs

Feel free to create the pull request, and we'll decide together how we move on :)

@H4wKs
Copy link
Contributor Author

H4wKs commented Apr 28, 2022

Feel free to create the pull request, and we'll decide together how we move on :)

@SoloJr done :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants