Skip to content

Save event-type's currency on db#2404

Merged
zomars merged 5 commits intocalcom:mainfrom
kommitters:fix/wrong-event-type-currency
Apr 7, 2022
Merged

Save event-type's currency on db#2404
zomars merged 5 commits intocalcom:mainfrom
kommitters:fix/wrong-event-type-currency

Conversation

@miguelnietoa
Copy link
Copy Markdown
Contributor

@miguelnietoa miguelnietoa commented Apr 6, 2022

What does this PR do?

This pull request sends the currency to the mutation so that this value is updated on the database. Previously, this value was not sent at all, so it took the default value which is "usd".

Fixes #2327
Fixes #2394

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

  • Connect Stripe integration
  • Enable the checkbox "Require payment ..."
  • Put a value
  • Save the changes
  • Then, go to the preview page, and you can see the currency is the same set on the event type settings ✅

Screenshots:

pic1

pic2

pic3

@miguelnietoa miguelnietoa requested a review from zomars April 6, 2022 17:02
@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 6, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployments, click below or on the icon next to each commit.

docs – ./apps/docs

🔍 Inspect: https://vercel.com/cal/docs/AzuEV2geQ4Frwfnv6MQiEcommwL2
✅ Preview: Canceled

[Deployment for ce64329 canceled]

calendso – ./apps/web

🔍 Inspect: https://vercel.com/cal/calendso/91U2wkbPqVZZ68DMzrmuuQJQt54D
✅ Preview: https://calendso-git-fork-kommitters-fix-wrong-event-type-currency-cal.vercel.app

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 6, 2022

@miguelnietoa is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@vercel vercel Bot temporarily deployed to Preview – docs April 6, 2022 17:02 Inactive
@vercel vercel Bot temporarily deployed to Preview – docs April 6, 2022 17:11 Inactive
@zomars zomars added the ♻️ autoupdate tells kodiak to keep this branch up-to-date label Apr 6, 2022
@vercel vercel Bot temporarily deployed to Preview – docs April 6, 2022 17:22 Inactive
Copy link
Copy Markdown
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution @miguelnietoa although it is evident that this code wasn't properly tested. I would recommend providing a loom video demonstrating that the solution actually works so we can speed up the reviewing process.

Comment thread apps/web/pages/event-types/[type].tsx
@miguelnietoa
Copy link
Copy Markdown
Contributor Author

Thank you for your contribution @miguelnietoa although it is evident that this code wasn't properly tested. I would recommend providing a loom video demonstrating that the solution actually works so we can speed up the reviewing process.

Ok @zomars I will record it 👌🏻

@vercel vercel Bot temporarily deployed to Preview – calendso April 6, 2022 19:48 Inactive
@miguelnietoa
Copy link
Copy Markdown
Contributor Author

miguelnietoa commented Apr 6, 2022

I did the tests with the currency set on "eur"
https://github.com/calcom/cal.com/blob/652b15c9e759f56d30b4794dd8a7b605c9b1b29f/apps/web/pages/event-types/%5Btype%5D.tsx#L2036-L2038

  • Before:
    Look that here the currency is shown as "eur", but in the database, the currency is set as "usd" because it is the default one.

    Before.mp4
  • Now:
    Notice that the value now is set to "eur" in the database, for that reason now in the Preview of the event type is shown as "eur" instead of "usd", which is good.

    After.mp4

@vercel vercel Bot temporarily deployed to Preview – docs April 7, 2022 03:14 Inactive
@miguelnietoa miguelnietoa requested a review from zomars April 7, 2022 14:38
Copy link
Copy Markdown
Contributor

@zomars zomars left a comment

Choose a reason for hiding this comment

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

Thanks @miguelnietoa let's update with the main branch and wait for tests to pass.

@vercel vercel Bot temporarily deployed to Preview – docs April 7, 2022 14:46 Inactive
@miguelnietoa
Copy link
Copy Markdown
Contributor Author

Thanks @miguelnietoa let's update with the main branch and wait for tests to pass.

Done

@vercel vercel Bot temporarily deployed to Preview – calendso April 7, 2022 15:18 Inactive
@zomars zomars merged commit 06df6c9 into calcom:main Apr 7, 2022
@miguelnietoa miguelnietoa deleted the fix/wrong-event-type-currency branch April 7, 2022 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

♻️ autoupdate tells kodiak to keep this branch up-to-date

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Currency symbol mismatch Pricing doesn't convert

2 participants