-
Notifications
You must be signed in to change notification settings - Fork 149
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
Add location to F2F payment option (backend) #867
Conversation
ec562e6
to
cb75397
Compare
cb75397
to
b6b94c2
Compare
django.core.validators.MaxValueValidator(15.0), | ||
], | ||
), | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
bond_size
was generated together with makemigrations
, not sure if it was just missing or a mistake on my side
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a mistake, but it is an undesired change. It is best to delete lines L39 to L51 before merging (that is migrations.AlterField(...)
. Alternatively, you can also delete the migration file, edit .env
to MIN_BOND_SIZE = 2
(instead of current value of 1) and the run manage.py makemigrations
again. Should yield the same migration without altering the field bond_size
.
The reason why this happened is that /api/models/order.py
Order field bond_size
depends on two config parameters that are read from the .env
file. In my setup the .env
file defines the MIN_BOND_SIZE
as 2%, while the .env-sample
defines it as 1% (at some point, there was a deviation, I changed my own .env
without updating the .env-sample
...). This migration will therefore allow bond sizes to be smaller than the current minimum of 2%.
I have opened a new issue (#873) so we make sure backend models are not dependant on any coordinator specific setting (.env
) and therefore, we do not have any sort of conflict at run time when coordinators change values on their .env
file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @KoalaSat the PR looks good to go for me! It is best to delete the bond_size
alter.
I left a comment with a deeper explanation of what has gone wrong and opened an issue so we do not have this problem during future development again.
django.core.validators.MaxValueValidator(15.0), | ||
], | ||
), | ||
), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not a mistake, but it is an undesired change. It is best to delete lines L39 to L51 before merging (that is migrations.AlterField(...)
. Alternatively, you can also delete the migration file, edit .env
to MIN_BOND_SIZE = 2
(instead of current value of 1) and the run manage.py makemigrations
again. Should yield the same migration without altering the field bond_size
.
The reason why this happened is that /api/models/order.py
Order field bond_size
depends on two config parameters that are read from the .env
file. In my setup the .env
file defines the MIN_BOND_SIZE
as 2%, while the .env-sample
defines it as 1% (at some point, there was a deviation, I changed my own .env
without updating the .env-sample
...). This migration will therefore allow bond sizes to be smaller than the current minimum of 2%.
I have opened a new issue (#873) so we make sure backend models are not dependant on any coordinator specific setting (.env
) and therefore, we do not have any sort of conflict at run time when coordinators change values on their .env
file.
api/migrations/0043_order_latitude_order_longitude_alter_order_bond_size.py
Outdated
Show resolved
Hide resolved
…o 0043_order_latitude_order_longitude.py
This PR is ready for merge! This PR is part of the "face-to-face" feature that has a development reward, I think it's probably best to payout the reward fully once the frontend work PR is ready. Talking about this reward, it now looks too small for the amount of work needed to get it done nicely following the current plan, so I propose doubling the reward to 2M Sats. |
* Add location to F2F payment option * Fix py linterns * Include migration * Revert docker-compose changes * Remove bond_size from migration * Rename 0043_order_latitude_order_longitude_alter_order_bond_size.py to 0043_order_latitude_order_longitude.py
What does this PR do?
Backend work for #760
This PR introduces the new
latitude
andlongitude
attributes for an Order so users can assign a geographical coordinate for F2F payments.This only includes the back-end changes.
Checklist before merging
pip install pre-commit
, thenpre-commit install
. Pre-commit installs git hooks that automatically check the codebase. If pre-commit fails when you commit your changes, please fix the problems it points out.