feat: status transition and allowed fields consolidated

ticketsystem/forms.py

@@ -15,6 +15,19 @@ class CommentForm(forms.ModelForm):
 
 
 class TicketForm(forms.ModelForm):
+    # Zentrale Definition der Status-Übergänge
+    STATUS_TRANSITIONS = {
+        'new': ['in_progress'],
+        'in_progress': ['resolved', 'closed'],
+        'resolved': ['closed'],
+        'closed': [],  # Keine weiteren Übergänge
+    }
+
+    # Zentrale Definition welche Felder wann required sind
+    REQUIRED_FIELDS_BY_STATUS = {
+        'resolved': ['answer'],  # Answer required when resolving
+    }
+
     class Meta:
         model = Ticket
         fields = ["title", "description", "status", "priority", "course", "answer", "material"]
@@ -26,25 +39,30 @@ class TicketForm(forms.ModelForm):
         }
 
     def __init__(self, *args, **kwargs):
-        # User und ticket aus kwargs holen (werden von der View übergeben)
         self.user = kwargs.pop('user', None)
         self.ticket = kwargs.pop('ticket', None)
         super().__init__(*args, **kwargs)
 
-        # Status-Choices basierend auf aktueller Situation einschränken
         if self.ticket and self.user:
-            self._limit_status_choices()
-            self._set_field_permissions()
+            self._configure_form_based_on_permissions()
 
-    def _set_field_permissions(self):
-        """Setzt welche Felder bearbeitet werden dürfen"""
+    def _configure_form_based_on_permissions(self):
+        """Konfiguriert das Formular basierend auf Benutzerrechten"""
         is_creator = self.user == self.ticket.created_by
         is_tutor = self.user == self.ticket.assigned_to
         is_superuser = self.user.is_superuser
 
+        # Status-Choices einschränken
+        self._limit_status_choices(is_tutor, is_creator)
+
+        # Feld-Berechtigungen setzen
+        self._set_field_permissions(is_tutor, is_creator, is_superuser)
+
+    def _set_field_permissions(self, is_tutor, is_creator, is_superuser):
+        """Setzt welche Felder bearbeitet werden dürfen"""
         if is_tutor and not is_superuser:
             # Tutor darf nur Status und Answer ändern
-            readonly_fields = ['title', 'description', 'course', 'priority']
+            readonly_fields = ['title', 'description']
             for field_name in readonly_fields:
                 if field_name in self.fields:
                     self.fields[field_name].disabled = True
@@ -53,80 +71,68 @@ class TicketForm(forms.ModelForm):
             for field_name in self.fields:
                 self.fields[field_name].disabled = True
 
-    def _limit_status_choices(self):
-        """Beschränkt die verfügbaren Status-Optionen basierend auf Rolle und aktuellem Status"""
+    def _limit_status_choices(self, is_tutor, is_creator):
+        """Beschränkt verfügbare Status-Optionen basierend auf der zentralen Logik"""
         current_status = self.ticket.status
-        is_creator = self.user == self.ticket.created_by
-        is_tutor = self.user == self.ticket.assigned_to
-
-        # Alle möglichen Status
-        all_choices = list(self.fields['status'].choices)
-        allowed_choices = []
 
         if is_tutor:
-            if current_status == 'new':
-                allowed_choices = ['new', 'in_progress']
-            elif current_status == 'in_progress':
-                allowed_choices = ['in_progress', 'resolved', 'new']
-            elif current_status == 'resolved':
-                allowed_choices = ['resolved', 'closed']
-            elif current_status == 'closed':
-                allowed_choices = ['closed']
+            allowed_statuses = self._get_allowed_transitions(current_status)
         else:
             # Nicht-Tutoren sehen nur aktuellen Status
-            allowed_choices = [current_status]
+            allowed_statuses = [current_status]
 
-        # Filtern der erlaubten Choices
+        # Status-Choices filtern
+        all_choices = list(self.fields['status'].choices)
         self.fields['status'].choices = [
             choice for choice in all_choices
-            if choice[0] in allowed_choices
+            if choice[0] in allowed_statuses
         ]
 
+    def _get_allowed_transitions(self, from_status):
+        """Gibt erlaubte Status-Übergänge zurück"""
+        return [from_status] + self.STATUS_TRANSITIONS.get(from_status, [])
+
+    def _is_transition_allowed(self, from_status, to_status):
+        """Prüft ob ein Status-Übergang erlaubt ist"""
+        if from_status == to_status:
+            return True
+        return to_status in self.STATUS_TRANSITIONS.get(from_status, [])
+
+    def _get_required_fields_for_status(self, status):
+        """Gibt zurück welche Felder für einen Status required sind"""
+        return self.REQUIRED_FIELDS_BY_STATUS.get(status, [])
+
     def clean(self):
         cleaned_data = super().clean()
         status = cleaned_data.get('status')
-        answer = cleaned_data.get('answer')
 
-        # Wenn Status auf "resolved" gesetzt wird, muss eine Antwort vorhanden sein
-        if status == 'resolved' and not answer:
+        if not self.ticket or not status:
+            return cleaned_data
+
+        # Prüfe Status-Übergang
+        old_status = self.ticket.status
+        is_tutor = self.user == self.ticket.assigned_to
+
+        if is_tutor and not self._is_transition_allowed(old_status, status):
             raise ValidationError({
-                'answer': 'Eine Antwort ist erforderlich, wenn der Status auf "Gelöst" gesetzt wird.'
+                'status': f'Übergang von "{self.ticket.get_status_display()}" zu "{dict(self.fields["status"].choices)[status]}" ist nicht erlaubt.'
             })
 
-        # Zusätzliche Validierung: Status-Übergang erlaubt?
-        if self.ticket and status:
-            old_status = self.ticket.status
-            if not self._is_status_transition_allowed(old_status, status):
+        # Prüfe required fields für neuen Status
+        required_fields = self._get_required_fields_for_status(status)
+        for field_name in required_fields:
+            if not cleaned_data.get(field_name):
+                field_label = self.fields[field_name].label
                 raise ValidationError({
-                    'status': f'Übergang von "{self.ticket.get_status_display()}" zu "{dict(self.fields["status"].choices)[status]}" ist nicht erlaubt.'
+                    field_name: f'{field_label} ist erforderlich, wenn der Status auf "{dict(self.fields["status"].choices)[status]}" gesetzt wird.'
                 })
 
         return cleaned_data
 
-    def _is_status_transition_allowed(self, old_status, new_status):
-        """Prüft ob ein Status-Übergang erlaubt ist"""
-        if old_status == new_status:
-            return True
-
-        is_tutor = self.user == self.ticket.assigned_to
-
-        # Erlaubte Übergänge für Tutoren
-        allowed_transitions = {
-            'new': ['in_progress'],
-            'in_progress': ['resolved', 'new'],
-            'resolved': ['closed'],
-            'closed': []  # Keine Änderung von closed
-        }
-
-        if is_tutor and old_status in allowed_transitions:
-            return new_status in allowed_transitions[old_status]
-
-        return False
-
     def save(self, commit=True):
         ticket = super().save(commit=False)
 
-        # Automatische Tutor-Zuweisung basierend auf dem ausgewählten Kurs
+        # Automatische Tutor-Zuweisung
         if ticket.course and ticket.course.tutor:
             ticket.assigned_to = ticket.course.tutor
 
@@ -137,4 +143,4 @@ class TicketForm(forms.ModelForm):
 
         if commit:
             ticket.save()
-        return ticket
+        return ticket