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

Currency hotfixes #1240

Merged
merged 2 commits into from
Oct 3, 2023
Merged

Currency hotfixes #1240

merged 2 commits into from
Oct 3, 2023

Conversation

zorun
Copy link
Collaborator

@zorun zorun commented Oct 3, 2023

Hardcode list of currencies, and temporarily disable some currency operations to prevent crashes.

Here is what is disabled:

  • setting or changing the default currency on an existing project
  • adding or editing a bill with a currency that differs from the default currency of the project

Adjust and skip tests as necessary.

The real fix will come after #1232 is discussed

@@ -615,6 +617,7 @@ def test_bills_with_calculation(self):
)
self.assertStatus(400, req)

@pytest.mark.skip(reason="Currency conversion is broken")
Copy link
Member

Choose a reason for hiding this comment

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

Yay, we benefit from new pytest markers :-)

@zorun
Copy link
Collaborator Author

zorun commented Oct 3, 2023

I'd like to release version 6.1.1 as soon as this is merged (which means really soon)

Copy link
Member

@almet almet left a comment

Choose a reason for hiding this comment

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

Hi, thanks for doing this.

What about adding a test scenario which reproduces the problems we have currently? So we can ensure this is fixed by the changes you're doing here?

If it's too long to do, I think we should merge this and release a bugfix release, but I think it could be a nice addition.

ihatemoney/tests/budget_test.py Show resolved Hide resolved
Here is what is disabled:

- setting or changing the default currency on an existing project

- adding or editing a bill with a currency that differs from the default
  currency of the project
@zorun
Copy link
Collaborator Author

zorun commented Oct 3, 2023

You mean simulating the exception that would be raised by the API call? It's a good idea, that would have allowed us to catch this way earlier. But I'm not motivated enough to do this now...

@almet
Copy link
Member

almet commented Oct 3, 2023

You mean simulating the exception that would be raised by the API call? It's a good idea, that would have allowed us to catch this way earlier. But I'm not motivated enough to do this now...

I understand, especially as it's a quick fix for an emergency release. Let's move on then, I won't have the time to add it either.

Thanks again for doing that!

@almet almet merged commit 1a2fa04 into master Oct 3, 2023
@almet almet deleted the currency_hotfix branch October 3, 2023 22:05
@zorun
Copy link
Collaborator Author

zorun commented Oct 3, 2023

Thanks for the review! I will make the release now.

TomRoussel pushed a commit to TomRoussel/ihatemoney that referenced this pull request Mar 2, 2024
* hotfix: hardcode list of currencies to workaround failing API calls

See spiral-project#1232 for a discussion on currencies

* Temporarily disable some currency operations to prevent crashes

Here is what is disabled:

- setting or changing the default currency on an existing project

- adding or editing a bill with a currency that differs from the default
  currency of the project

---------

Co-authored-by: Baptiste Jonglez <[email protected]>
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 this pull request may close these issues.

2 participants