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

Feature MobilePay transaction integration #204

Merged
merged 80 commits into from
Nov 18, 2020

Conversation

falkecarlsen
Copy link
Member

@falkecarlsen falkecarlsen commented Sep 13, 2020

Processing payments as a Treo can be frustrating as you're referencing two lists; payments on stregsystemet and transactions from MobilePay. Sometimes Treos do payments in reverse order, making it harder to figure out which transactions are processed and which are missing. Sometimes multiple Treos are making them concurrently and I unfortunately haven't found a human lock library for Python.

Maybe this can be fully automated for 'correct' MobilePay-comments in the future when the MobilePay Transactions API has been set up. This has also been discussed in #203.
Meanwhile, MobilePay offers functionality to download a CSV containing the same information. This PR adds functionality to upload, parse, and create MobilePayment objects automatically for these transactions along with a utility for processing these transactions, creating ordinary Payments when approved by a Treo.

Thanks @joandrsn for figuring out MobilePay API import 👌

image

@hrjakobsen
Copy link
Member

Expected human lock library. 10/10 disappoint.

Copy link
Contributor

@BoAnd BoAnd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small thing. I have not test run the code :'(

stregsystem/admin.py Outdated Show resolved Hide resolved
stregsystem/admin.py Outdated Show resolved Hide resolved
stregsystem/views.py Outdated Show resolved Hide resolved
VirtualSatai
VirtualSatai previously approved these changes Sep 14, 2020
Copy link
Collaborator

@VirtualSatai VirtualSatai left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

God ide at få dette lavet. @DelusionalLogic og jeg havde tidligere tænkt på at lave det samme, men ifølge daværende formand kunne man ende i problemer hvis ikke der var en person i loop'et. Denne løsning har ikke samme problem, så go for it :)

stregsystem/models.py Outdated Show resolved Hide resolved
stregsystem/views.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@DelusionalLogic DelusionalLogic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel better living in a world where this code is inside the stregsystem.

Copy link
Contributor

@asger-weirsoee asger-weirsoee left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome to have implemented logging, though I think this maybe should've been its own PR, yet all other comments I have are minor things.

Good job, with everything! 👍 :)

stregsystem/models.py Outdated Show resolved Hide resolved
stregsystem/templates/admin/stregsystem/mobilepaytool.html Outdated Show resolved Hide resolved
stregsystem/views.py Outdated Show resolved Hide resolved
@codecov-io
Copy link

codecov-io commented Nov 18, 2020

Codecov Report

Merging #204 (ec96922) into next (3cd9954) will decrease coverage by 0.45%.
The diff coverage is 76.61%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next     #204      +/-   ##
==========================================
- Coverage   82.85%   82.39%   -0.46%     
==========================================
  Files          30       30              
  Lines        2006     2244     +238     
  Branches      135      157      +22     
==========================================
+ Hits         1662     1849     +187     
- Misses        321      370      +49     
- Partials       23       25       +2     
Impacted Files Coverage Δ
stregsystem/urls.py 75.00% <ø> (ø)
stregsystem/views.py 60.95% <10.52%> (-11.62%) ⬇️
stregsystem/forms.py 77.27% <68.75%> (-22.73%) ⬇️
stregsystem/admin.py 73.40% <72.41%> (+1.01%) ⬆️
stregsystem/utils.py 72.61% <81.81%> (+7.91%) ⬆️
stregsystem/models.py 90.62% <88.88%> (+0.74%) ⬆️
stregsystem/tests.py 99.73% <100.00%> (+0.03%) ⬆️
treo/settings.py 97.82% <100.00%> (+0.09%) ⬆️
... and 1 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3cd9954...ec96922. Read the comment docs.

@krestenlaust
Copy link
Member

VippsMobilepay-fusion's new API might have opened up for automatic matching without having to write information in the comment-feature on transactions. The last 4-digits and legal name of payer is received. #416 Documents implementation of this new API.

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

Successfully merging this pull request may close these issues.

9 participants