Provide a product group type dropdown when creating/editing them #1326
Conversation
It's still just a free text thing in the database. Fixes #1322.
"is not None" is not a valid Jinja2 test; "is not none" is, because "none" is the name of a Jinja2 test.
| product_group_type = PRODUCT_GROUP_TYPES_DICT.get( | ||
| product.parent.type | ||
| ) | ||
| purchase_cls = ( | ||
| product_group_type.purchase_cls | ||
| if product_group_type | ||
| else Purchase | ||
| ) |
There was a problem hiding this comment.
Could combine these two, by providing a dict lookup fallback/default?
[ removed to prevent accidental commit ]
(although arguably less clear, partly due to the text wrapping to fit within line limits)
There was a problem hiding this comment.
Ah, nevermind; I see that there's an additional level of indirection for the purchase_cls lookup. Hrm. I'll ponder that a bit, but this seems OK as-is, and my suggestion's not valid.
There was a problem hiding this comment.
The problem is that the purchase class is an attribute on the thing in the dict, rather than directly being an element of it, so this doesn't work in and of itself.
| @@ -16,7 +16,7 @@ <h2>Payment {{ payment.id }}</h2> | |||
| {% endif %} | |||
| {% if payment.provider == 'banktransfer' and payment.state == 'inprogress' %} | |||
| <a class="btn btn-primary" href="{{ url_for('.reset_expiry', payment_id=payment.id) }}">Reset expiry</a> | |||
| <a class="btn btn-warning {% if payment.reminder_sent_at is not None -%} disabled {%- endif %}" href="{{ url_for('.send_reminder', payment_id=payment.id) }}">Remind</a> | |||
| <a class="btn btn-warning {% if payment.reminder_sent_at is not none -%} disabled {%- endif %}" href="{{ url_for('.send_reminder', payment_id=payment.id) }}">Remind</a> | |||
There was a problem hiding this comment.
is none was news to me, but yep, that checks out - nice find.
| purchase_cls: type[Purchase] | ||
|
|
||
|
|
||
| PRODUCT_GROUP_TYPES = [ |
There was a problem hiding this comment.
I'm muddling my way back through the design factors for the change: is there a reason these should go here instead of alongside the similar-ish constants in purchase.py?
There was a problem hiding this comment.
That list doesn't seem to be used at all, so it can probably be removed.
The root issue here is that there's inevitable duplication because we have a polymorphic type hierarchy under Purchase. Which of those Purchase types is created is determined by the Product's ProductGroup.type. And the point of #1214 (I haven't checked how relevant it still is) is that we should probably only be checking one of those.
Anyway I'm happy that this PR is a net improvement to the situation.
Resolves #1322.
...also fix a bug when viewing payments in the admin panel where it reports "is not None" as an error because "is" has a specific meaning in Jinja2.