Implemented User-Storys 3, 8, 9 #8

yduman · 2016-12-09T13:44:05Z

enabled logging for state transitions
enabled state transition through barcode scan
enabled a notification for the user, if the lecturer is "N.N"

Take your time with this PR, since it is a big commit (we had some issues while branching). If there a open questions, just email me :)

coveralls · 2016-12-09T13:45:46Z

Coverage increased (+0.7%) to 91.116% when pulling 94379ed on yduman:master into 2bac9ca on d120:master.

ckleemann · 2016-12-09T16:30:01Z

src/feedback/admin.py

-from django import forms
+
+def status_angelegt(modeladmin, request, queryset):
+    queryset.update(status=100)


please use Veranstaltung.STATUS_ANGELEGT instead of 100 same for the other functions

ckleemann · 2016-12-09T16:37:13Z

src/feedback/models/base.py

@@ -205,13 +204,6 @@ class AlternativVorname(models.Model):
    vorname = models.CharField(_('first name'), max_length=30, blank=True)
    person = models.ForeignKey(Person, on_delete=models.CASCADE)

-    @staticmethod


i guess the remove of this function is a glitch in the pr. Maybe you like to add the function again.

coveralls · 2016-12-09T20:28:24Z

Coverage increased (+0.7%) to 91.116% when pulling 0114ca2 on yduman:master into 2bac9ca on d120:master.

coveralls · 2016-12-09T20:28:24Z

Coverage increased (+0.7%) to 91.116% when pulling 0114ca2 on yduman:master into 2bac9ca on d120:master.

ckleemann

Please also read the outdatet comment because it is still valid

ckleemann · 2016-12-12T15:56:43Z

src/feedback/admin.py

+    queryset.update(status=900)
+    for veranstaltung in queryset:
+        veranstaltung.log(True, False)
+status_boegen_gescannt.short_description = 'Status: gescannt'



This solution requires a new function each time a new Status is added. Please refactore to one function "Status ändern" which redirects the user to a intermediate page where the user select the targed state. See https://docs.djangoproject.com/en/1.10/ref/contrib/admin/actions/#actions-that-provide-intermediate-pages

ckleemann · 2016-12-12T15:58:38Z

src/feedback/admin.py

    list_display_links = ['name']
    list_filter = ('typ', 'semester', 'grundstudium', 'evaluieren', 'sprache')
    search_fields = ['name']
    filter_horizontal = ('veranstalter', 'ergebnis_empfaenger')  # @see http://stackoverflow.com/a/5386871
    readonly_fields = ('link_veranstalter',)


+class VeranstaltungStatus(Veranstaltung):


Just drop this model and the admin for it and integrate the function in to the normal Admin for the Veranstaltung model.

In the Veranstaltung admin you can add a table inline admin which lists all the log entrys of the instance of a Veranstaltung. Please make sure that you can not add a new log entry manual.

ckleemann · 2016-12-12T16:00:22Z

src/feedback/admin.py

+    ]
+
+
+class LogAdmin(admin.ModelAdmin):


It sould not be possible for the user to manual manipulate a log. So just drop the admin for log

ckleemann · 2016-12-12T16:03:48Z

src/feedback/models/base.py

@@ -316,6 +324,18 @@ class Veranstaltung(models.Model):
    freiefrage2 = models.TextField(verbose_name='2. Freie Frage', blank=True)
    kleingruppen = models.TextField(verbose_name='Kleingruppen', blank=True)

+    def get_next_state(self):


Just use a dict for it where the source state is the key and the targed state is the value.
Whats about the case that there are more then one possible targed state? So you might better return a tuple containing all targed states then a single value.

ckleemann · 2016-12-12T16:10:28Z

src/feedback/models/base.py

+    veranstaltung = models.ForeignKey(Veranstaltung, null=True)
+    timestamp = models.DateTimeField(auto_now_add=True)
+    status = models.IntegerField(choices=Veranstaltung.STATUS_CHOICES, default=Veranstaltung.STATUS_ANGELEGT)
+    verursacher = models.CharField(max_length=30, blank=True)


Please use two foreign Key relations for verursacher instead of a CharField. If the verursacher is a user or a Veranstalter you should link to the Django auth user model. There should be a Veranstalter user in auth ;)
If the Verursacher is a Barcode scanner you should link to the Barcode scanner.

ckleemann · 2016-12-12T16:12:05Z

src/feedback/models/base.py

+    timestamp = models.DateTimeField(auto_now_add=True)
+    status = models.IntegerField(choices=Veranstaltung.STATUS_CHOICES, default=Veranstaltung.STATUS_ANGELEGT)
+    verursacher = models.CharField(max_length=30, blank=True)
+    interface = models.CharField(max_length=30, blank=True)


The number of interface is limited to 3 so please use a select choices for it.

ckleemann · 2016-12-12T16:16:15Z

src/feedback/views/public_class_view.py

+            if next_state is None:
+                response_message = "Nachster Status ungultig."
+            else:
+                time_offset = datetime.date.today() - datetime.timedelta(hours=1)


The limit should 1 Minute.

coveralls · 2016-12-15T13:19:30Z

Coverage increased (+0.7%) to 91.116% when pulling 57d573d on yduman:master into daf155e on d120:master.

coveralls · 2016-12-15T16:16:02Z

Coverage increased (+1.02%) to 91.401% when pulling addb3bf on yduman:master into daf155e on d120:master.

ckleemann

Protection against double scaning is not working

ckleemann · 2016-12-15T17:44:22Z

src/feedback/tests/test_views_public.py

+        self.veranstaltung.refresh_from_db()
+        self.assertEqual(self.veranstaltung.status, Veranstaltung.STATUS_GEDRUCKT)
+
+    def test_double_scan(self):


This testsetup is incomplete: Because the scanner only can reach one state. There is no testing of the time constraint at all!

ckleemann · 2016-12-15T17:45:31Z

src/feedback/tests/test_views_public.py

+        self.barcode_scanner_no_access_right = BarcodeScanner.objects.create(token="LRh73Ds22", description="")
+
+        self.barcode_scanner = BarcodeScanner.objects.create(token="LRh73Ds23", description="")
+        BarcodeAllowedState.objects.create(barcode_scanner=self.barcode_scanner,


For double scan testing another allowed State is required e.g.

ckleemann · 2016-12-15T17:48:29Z

src/feedback/views/public_class_view.py

@@ -25,7 +25,7 @@ def barcodedrop(request):
            if next_state is None:
                response_message = "Nachster Status ungultig."
            else:
-                time_offset = datetime.date.today() - datetime.timedelta(hours=1)
+                time_offset = datetime.date.today() - datetime.timedelta(minutes=1)


datetime.date.today() is not what you want. datetime.datetime.now() has a time resolution and not only a date resulution.

coveralls · 2016-12-15T19:36:07Z

Coverage increased (+1.05%) to 91.433% when pulling 640be75 on yduman:master into daf155e on d120:master.

coveralls · 2016-12-15T22:18:37Z

Coverage increased (+1.08%) to 91.458% when pulling 5012631 on yduman:master into daf155e on d120:master.

yduman added 3 commits December 9, 2016 14:32

updated requirements

2bfc4a2

finished user-storys 3,8,9

1c53899

merged migration

94379ed

ckleemann reviewed Dec 9, 2016

View reviewed changes

refactored setting of status in admin actions

0114ca2

ckleemann reviewed Dec 12, 2016

View reviewed changes

Merge remote-tracking branch 'upstream/master'

57d573d

yduman added 2 commits December 15, 2016 17:08

fixed remarks

5966bd1

merged migration

addb3bf

ckleemann requested changes Dec 15, 2016

View reviewed changes

Add complete test for double barcode scan

640be75

Fixed wrong date format in barcode scanner and test for it.

5012631

ckleemann approved these changes Dec 16, 2016

View reviewed changes

ckleemann merged commit b8ab0db into d120:master Dec 16, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implemented User-Storys 3, 8, 9 #8

Implemented User-Storys 3, 8, 9 #8

yduman commented Dec 9, 2016

coveralls commented Dec 9, 2016 •

edited

Loading

ckleemann Dec 9, 2016

ckleemann Dec 9, 2016

coveralls commented Dec 9, 2016

coveralls commented Dec 9, 2016

ckleemann left a comment •

edited

Loading

ckleemann Dec 12, 2016

ckleemann Dec 12, 2016

ckleemann Dec 12, 2016

ckleemann Dec 12, 2016

ckleemann Dec 12, 2016

ckleemann Dec 12, 2016

ckleemann Dec 12, 2016

coveralls commented Dec 15, 2016 •

edited

Loading

coveralls commented Dec 15, 2016 •

edited

Loading

ckleemann left a comment

ckleemann Dec 15, 2016

ckleemann Dec 15, 2016 •

edited

Loading

ckleemann Dec 15, 2016

coveralls commented Dec 15, 2016 •

edited

Loading

coveralls commented Dec 15, 2016 •

edited

Loading

Implemented User-Storys 3, 8, 9 #8

Implemented User-Storys 3, 8, 9 #8

Conversation

yduman commented Dec 9, 2016

coveralls commented Dec 9, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Dec 9, 2016

coveralls commented Dec 9, 2016

ckleemann left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Dec 15, 2016 • edited Loading

coveralls commented Dec 15, 2016 • edited Loading

ckleemann left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ckleemann Dec 15, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Dec 15, 2016 • edited Loading

coveralls commented Dec 15, 2016 • edited Loading

coveralls commented Dec 9, 2016 •

edited

Loading

ckleemann left a comment •

edited

Loading

coveralls commented Dec 15, 2016 •

edited

Loading

coveralls commented Dec 15, 2016 •

edited

Loading

ckleemann Dec 15, 2016 •

edited

Loading

coveralls commented Dec 15, 2016 •

edited

Loading

coveralls commented Dec 15, 2016 •

edited

Loading