Add further checks for invalid code to workshop request forms

[elichad]

Depends on #2552 . Closes #2533. Updated 08 Nov.

This PR:

• checks that membership is active at time of form submission
• checks that membership has workshops remaining
• adds info about related membership to WR details page (edit: this fixes Update workshop request form: Warning to admin that workshop may not be allowed  #2393!)

[elichad]

@sheraaronhurt @Talishask I want to check this with you:

What is your usual workflow when a member has no workshops remaining but wants to request another workshop? How do you update AMY to represent this? Do you treat it as a paid COW separate from the membership?
I'm trying to understand if any nuance is necessary when validating the code and discovering there are no workshops left, or if it's okay to just block submission in that case.

Edit: after reading through the memberships page, I understand that members contact us to purchase additional workshops. Where do we track this extra allowance in AMY (if at all)?

[sheraaronhurt]

@elichad thanks for checking. While this is a rare occurrence, yes they will contact us and we give them additional workshops at a discounted rate. In AMY it would be tracked by the discounted workshops section. @Talishask feel free to chime in :)

[elichad]

So as I understand it, we agree on extra workshops with the member, but there's nothing in AMY that indicates 'this member is allowed X more workshops than the original contract (and should be allowed to submit X further requests).' We just remember that they have extras when we're processing the requests that come through with their code. Looking at the code, the 'discounted workshops' field is actually just counting all the Events connected with the membership, and subtracting the original allowance.

If that's how things work, then those members won't actually be able to submit requests if they've used their original allowance, because the form will see '0 workshops remaining' in AMY and block the submission.

To solve that, I think we need to add an 'additional workshops' number field to the membership, in the same manner as the 'additional instructor training seats' that we have at the moment. That way, when a member buys an additional 2 workshops, we can edit their membership to record that, and then they will be able to submit 2 more workshop requests after that.

Does that make sense?

[Talishask]

Great questions.

Member Workshops
When members need additional workshops they typically submit a workshop request form before contacting us and then we discuss this with them. My current process is to update the number of allowed workshops once the member is invoiced. In most cases Danielle is already working with them. The downside of updating the allowed workshops field is that we lose data on added workshops, so this is not optimal. We used to have a 'Members included in membership" and an 'Additional workshops' field but we lost the second field with some updates last year. I agree and would be excited to get that field back.

Given that members typically submit requests before they are invoiced for additional workshops, I would recommend we have a warning internally but do not block any submissions due to '0 workshops remaining.

Discounted Workshops Field
The current discounted workshops field assumes that any COWs above the allowed workshops are discounted. That is not correct. Typically these are workshops that are either above the limit or not yet paid. It would be more helpful for workshop allowances to be structured just the same as IT seats with "allowed", "used", "remaining".

Alacarte Workshops
Alacarte workshops are a little bit different. Our current workflow is to create the membership (so that we can tie the event to it because planning can start while we are working on paperwork), but wait to add the allowed workshop until the invoicing happens. Essentially we are waiting to confirm payment before adding the allowed workshop. We changed this order for a la carte workshops because they are easier to fall through the cracks and not get invoiced. I have a report that lets me see workshops where the allowed is zero then I check to see if we received payment or not and update the allowed workshops once we invoice it. It might be a good idea to have a conversation with the TT about some of these things to help me find better solutions and standardize more with our regular workflows.

[elichad]

@Talishask Thanks for this information, I'll schedule a short call with you to talk through a couple of follow-up questions I have

[elichad]

From conversation with @Talishask:

• remove the check for workshops remaining. We often do invoicing for extra workshops alongside the planning process, so we need to be able to get workshops into the database before
• instead, add a warning message on the WR details page if the workshop would exceed the member's allocation
• add the 'additional workshops' field back into the Membership model, and display it similarly to how we do IT seats (best to do this as part of Allow additional inperson/online workshops for each membership #2391)

[elichad]

@pbanaszkiewicz this is now ready for review!

[pbanaszkiewicz]

Looks good, I left some suggestion but nothing too important.

[pbanaszkiewicz]

Is it expected for this tag to work on objects other than Membership?

If it's expected then the type hinting is not correct. Otherwise I'd remove this condition.

[pbanaszkiewicz]

My suggestion is to turn this tag into a Django template. There's a lot of template generation going on compared to "difficult template logic".

Here, the logic that may be difficult or "ugly" to achieve in the Django template is the calculation of alert type. I suggest to use a template tag to determine if this message is just informational or a warning.

[pbanaszkiewicz]

In the Django template you can use pluralize template filter to achieve the plural form of "workshop".

[pbanaszkiewicz]

After reading https://docs.djangoproject.com/en/3.2/ref/utils/#django.utils.html.format_html I think we should review our use of mark_safe, as format_html may be more appropriate.

[elichad]

Yes, I agree. Created #2574

[pbanaszkiewicz]

Naming suggestion: get_membership_by_code_or_none, as or_none automatically suggest what is returned if membership is not found.

On the other hand, my suggestion may be flawed by suggesting fetching the membership "by code or by None".

[elichad]

Thanks Piotr! I addressed all your comments so I'll go ahead and merge.