Fixes for single currency form, templatetags and tests refactoring #78

krzysztofwolski · 2019-07-03T00:57:09Z

Updated:

available_currencies is now a list of currency codes
FixedCurrencyWidget is now using always 2 widgets (hidden one for currency)
When form field is optional and the amount is empty, None is returned
splitted tests into separate files. Only new tests in test_forms

tests/forms.py

django_prices/forms.py

rafalp · 2019-07-03T18:42:50Z

tests/test_forms.py

+    assert 'name="price_1"' in result
+    assert 'key="value"' in result
+    assert 'value="5"' in result
+    assert "USD" in result


Perhaps this could be tested with snapshot?

Also consider changing tests names to descriptive ones, eg test_money_input_widget_renders

Not sure about adding snapshottest for one or two tests. I would say it's good enough.

tests/test_forms.py

tests/test_templatetags.py

patrys · 2019-07-04T09:52:04Z

tests/test_templatetags.py

+    #     https://github.com/python-babel/babel/issues/30
+    translation.activate("oO_Oo")
+    amount = prices_i18n.amount(money_fixture, format="html")
+    assert amount  # No exception, success!


This is a noop. It does not test anything as any exception would be raised in the previous line. Why don't you instead document what the result should be with a snapshot?

I'm not the original author of this test

patrys

Since we now explicitly depend on Babel, I don't think it makes sense to support the non-i18n-aware version of the template tags. Can we move the i18n ones to prices and have prices_i18n call the ones from prices with a deprecation warning?

README.md

django_prices/forms.py

patrys · 2019-07-04T12:57:46Z

django_prices/forms.py

+        if amount in self.empty_values and self.required:
+            raise ValidationError("Enter a valid amount of money")
+        if currency in self.empty_values and self.required:
+            raise ValidationError("Enter a valid currency code")


Do users enter a currency code?

I believe it's for those who use custom widgets.

I'm asking about the wording: "Enter a valid currency code" vs "Select a valid currency" as the currency is unlikely to be something entered (typed into the field) directly.

This occurs when somebody uses fixed currency form and submits (by example using curl) intentionally different value.
Currency Field in multicurrency form has its own validation.

django_prices/templatetags/prices_i18n.py

django_prices/validators.py

django_prices/widgets.py

django_prices/forms.py

django_prices/templatetags/prices_i18n.py

rafalp · 2019-07-04T15:44:42Z

django_prices/utils/formatting.py

+    pattern = locale.currency_formats.get("standard").pattern
+
+    if html:
+        pattern = re.sub("(\xa4+)", '<span class="currency">\\1</span>', pattern)


Maybe-ish putting the currency code somewhere in there may be useful? data-currency="USD".

tests/test_forms.py

Co-Authored-By: Rafał Pitoń <[email protected]>

Co-Authored-By: Adam Bogdał <[email protected]>

krzysztofwolski · 2019-07-05T14:53:25Z

@patrys Rebased! Can be released as alpha 🎉

krzysztofwolski force-pushed the form-issues branch from 4047390 to 3b947f9 Compare July 3, 2019 00:57

patrys reviewed Jul 3, 2019

View reviewed changes

tests/forms.py Outdated Show resolved Hide resolved

krzysztofwolski requested review from Kwaidan00, rafalp, patrys and bogdal July 3, 2019 16:59

rafalp suggested changes Jul 3, 2019

View reviewed changes

Kwaidan00 approved these changes Jul 3, 2019

View reviewed changes

bogdal requested changes Jul 4, 2019

View reviewed changes

tests/test_forms.py Outdated Show resolved Hide resolved