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

Add field and check donation for phone verified #4594

Merged
merged 20 commits into from
Sep 14, 2024

Conversation

Krishnag09
Copy link
Contributor

@Krishnag09 Krishnag09 commented Jul 25, 2024

The PR adds a new profile field to track donation, to act as a paywall for phone verification

closes the issue - #4349

--->

Backend checklist

  • Formatted my code by running ruff check --select I --fix . && ruff check . && ruff format . in app/backend
  • Added tests for any new code or added a regression test if fixing a bug
  • All tests pass
  • Run the backend locally and it works
  • Added migrations if there are any database changes, rebased onto develop if necessary for linear migration history

Web frontend checklist

  • Formatted my code with make format
  • There are no warnings from make lint
  • There are no console warnings when running the app
  • Added any new components to storybook
  • Added tests where relevant
  • All tests pass
  • Clicked around my changes running locally and it works
  • Checked Desktop, Mobile and Tablet screen sizes

Krishnag09 and others added 5 commits July 15, 2024 22:57

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
Copy link

vercel bot commented Jul 25, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
couchers ✅ Ready (Inspect) Visit Preview Sep 14, 2024 11:13pm

Copy link
Member

@aapeliv aapeliv left a comment

Choose a reason for hiding this comment

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

I think there are two things that still need to be implemented:

  1. It would be good to return the has_donated flag in GetAccountInfo (account.proto/account.py)
  2. We need to somehow populate the has_donated flag when we set it up so that those who have already donated get it ticked off. I can help make this happen in the migration.

app/backend/src/couchers/models.py Outdated Show resolved Hide resolved
app/backend/src/couchers/servicers/donations.py Outdated Show resolved Hide resolved
app/proto/api.proto Outdated Show resolved Hide resolved
Copy link
Member

@aapeliv aapeliv left a comment

Choose a reason for hiding this comment

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

  1. Database migrations (update the has_donated flag based on existing data in the database) -- I can help with this: it's going to be complicated

This needs a database query to check who has already donated based on existing tables, so we set has_donated=True for those users.

  1. Add a env var flag or constant in constants.py as to whether to enforce this or not. So we can update the frontend first.

app/backend/src/tests/test_fixtures.py Outdated Show resolved Hide resolved
app/media/src/media/server.py Outdated Show resolved Hide resolved
@aapeliv
Copy link
Member

aapeliv commented Sep 8, 2024

@Krishnag09 : could you please rebase onto develop or merge develop into this branch, please?

Are you OK if I push a commit to fix up the migrations onto this branch? Other than that and minor formatting stuff (I can fix that up too), I think this is looking good to go!

@aapeliv aapeliv force-pushed the backend/feature/strong-verification branch 2 times, most recently from 541e727 to be6d591 Compare September 8, 2024 21:23
@aapeliv aapeliv force-pushed the backend/feature/strong-verification branch from be6d591 to 178f3b2 Compare September 8, 2024 21:28
@aapeliv aapeliv force-pushed the backend/feature/strong-verification branch from 178f3b2 to 1f22e6c Compare September 8, 2024 21:31
@Krishnag09
Copy link
Contributor Author

  1. Database migrations (update the has_donated flag based on existing data in the database) -- I can help with this: it's going to be complicated

This needs a database query to check who has already donated based on existing tables, so we set has_donated=True for those users.

  1. Add a env var flag or constant in constants.py as to whether to enforce this or not. So we can update the frontend first.

do you still want me to do these?

@aapeliv
Copy link
Member

aapeliv commented Sep 9, 2024

do you still want me to do these?

It's all done now in the commit I pushed. I think it just needs an update to the migration (to say down revision is 3ac39dcf3b5a).

But let's wait a little bit before merging so I can do the frontend part first!

@aapeliv aapeliv merged commit c4988b1 into develop Sep 14, 2024
3 checks passed
@aapeliv aapeliv deleted the backend/feature/strong-verification branch September 14, 2024 23:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants