apluslms / a-plus

A+ frontend portal - A+ LMS documentation:
https://apluslms.github.io/
Other
67 stars 72 forks source link

Deleting student tags does not work #1261

Closed jsorva closed 12 months ago

jsorva commented 1 year ago

As it says in the issue title, deleting student tags fails. For example, if I go here: https://plus.cs.aalto.fi/o1/2023/teachers/tags/ and try to remove some tags, I get a confirmation page (not a confirmation dialog BTW, which would have been more expected) where I can click a button to the effect that "Yes, remove the tag". But nothing happens even if I confirm. The page looks "stuck", and the tag remains.

This has already caused some minor annoyance for me personally, and it will be more annoying in a few days’ time when we’re relying on student tags to distribute some work amongst our 30+ teaching assistants.

markkuriekkinen commented 12 months ago

It looks like that UserTagDeleteView is failing the form validation and thus, the tag is not deleted. One approach for a fix is to remove the validation by setting the form class to django.forms.Form.

markkuriekkinen commented 12 months ago

Django 4.0 changed the DeleteView so that UserTagDeleteView started to fail due to form validation that never passes. There is a form_class attribute in the super class UserTagMixin. Since Django 4.0, deleting tags has been broken since the form validation never passes in this case. This can be fixed by replacing the form in UserTagDeleteView with the default empty form that does not validate anything.

https://docs.djangoproject.com/en/4.2/releases/4.0/#generic-views

DeleteView now uses FormMixin, allowing you to provide a Form subclass, with a checkbox for example, to confirm deletion. In addition, this allows DeleteView to function with django.contrib.messages.views.SuccessMessageMixin. In accordance with FormMixin, object deletion for POST requests is handled in form_valid(). Custom delete logic in delete() handlers should be moved to form_valid(), or a shared helper method, as needed.

https://docs.djangoproject.com/en/4.2/releases/4.0/#deleteview-changes

DeleteView now uses FormMixin to handle POST requests. As a consequence, any custom deletion logic in delete() handlers should be moved to form_valid(), or a shared helper method, if required.