search

spiral-project / ihatemoney

A simple shared budget manager web application
https://ihatemoney.org
Other
1.2k stars 270 forks source link

Changing any settings is prevented when project has existing currency #1292

Closed Turtle6665 closed 8 months ago

Turtle6665 commented 8 months ago

This PR fixes #1291.

Summary of the issue

It's a small fix to a small annoying bug : when a project as some bills, a default currency (different than "XXX") and you want to change the project settings an error message is shown : "Cannot change project currency because currency conversion is broken". The only way to change the project settings is then to remove the currency (set to "XXX"). Therefore, the error message is not constant with the behavior.

The new behavior

The default currency field is check against the current project default currency. If the project has bills and the new input default currency doesn't match the current one, the error "Cannot change project currency because currency conversion is broken" is shown. I have tested the behavior. This is true for the API as well as the GUI input.

Turtle6665 commented 8 months ago

One of the automatic test has failed (Test & Docs / build (3.12, normal, sqlite)). After reviewing the logs, I don't have any clue on what action to take. There seams to be looking for a non-existing module: 'distutils'. This doesn't seem to be related to my change.

Turtle6665 commented 8 months ago

Well, after a quick search on internet, I found out that distutils package is apparently removed in python version 3.12 (thanks to this Stack Overflow answer).

It cites the python 3.12 change log :

gh-95299: Do not pre-install setuptools in virtual environments created with venv. This means that distutils, setuptools, pkg_resources, and easy_install will no longer available by default; to access these run pip install setuptools in the activated virtual environment.

So a pip install setuptools should probably be added to the GitHub action for the python 3.12. Would this be better done on a different PR as it's a different issue @almet ? BTW, I'm not confident enough to successfully adapt the GitHub action, so I will let that to someone else to do.

almet commented 8 months ago

Hey, thanks for the research on this. I find this particularly weird, because this seem already covered by #1258.

Including distutils in the dependencies might be the answer for the interim, even if I'm not sure why this is failing atm (with the fix already present in the released babel package)

zorun commented 8 months ago

I just released 6.1.5 with this fix