feat: show simple shift statistics by crosspolar · Pull Request #649 · SuperCoopBerlin/tapir

crosspolar · 2025-08-08T16:44:01Z

Resolves #407 .

Simple markup but I think currently sufficient. Let me know if you see it somewhere else...

Theophile-Madet

The shift detail page already has a lot of information, I don't think we should add more, especially if they are not relevant for everyday use.

I would either make a separate page for shift stats, or maybe add a button to the header with just the stats icon (material-icon calculate) that shows a popup or a tooltip.

Also we'll need tests.

Theophile-Madet

Should we show the same stats on the shift template detail page?
Can you try to check the amount of DB requests this creates when there are a lot of past shifts? I added a comment on tapir/shifts/views/views.py. Let me know if you need help with Silk or how to use selected_related and prefetch_related.
The main stats page is intended as an overview of the key stats to reach the break-even. We removed all other stats on purpose to keep the overview clear, I think it should stay as it is. We could rename the current page to "Stats for break-even" or "Key stats" and add another one called "secondary stats"? There are a few stats from the old member stats page that we removed for clarity and because they are untested, but we could move them to a secondary stats page later.

tapir/shifts/views/views.py

crosspolar · 2025-09-06T15:30:22Z

Hey,

number of queries: I didn't keep the stats but the annotate (which seems great for aggregations) reduced number of queries from around 10 to 2.
I removed the overall statistics since a bit unsure where to add. Maybe we can add it on some later release again

Theophile-Madet

A few more style suggestions but I think we're getting there.

tapir/shifts/views/views.py

tapir/shifts/tests/test_get_past_shifts_data.py

…ird "one people..."

This reverts commit 81022b9

This reverts commit 69e5f13.

crosspolar · 2025-09-09T14:43:03Z

@Theophile-Madet , thanks for your feedback. I really try to comply with the test-naming-style-guide, but keep forgetting sorry.
About the open comment: Maybe I misunderstood your suggestion, but I would sleep better with the current flexibility.

Theophile-Madet · 2025-09-15T09:07:09Z

Are the force push always needed? After a force push I can't use the "show differences since last review" feature.

crosspolar · 2025-09-16T07:19:59Z

Are the force push always needed? After a force push I can't use the "show differences since last review" feature.

No, of course not. I just do rebases to update the PRs but if we squash before merge I think I don't have to

tapir/shifts/views/views.py

tapir/shifts/tests/test_get_past_shifts_data.py

Theophile-Madet · 2026-02-22T11:26:33Z

tapir/shifts/tests/test_get_past_shifts_data.py

+        )
+
+        context = ShiftDetailView.get_past_shifts_data(shift_template)
+        self.assertAlmostEqual(context["total_hours"], 7.0)


I think this can just be a normal self.assertEqual? Is there a reason for using assertAlmostEqual?

Floating-point numbers, you know. The print(0.1+0.2 == 0.3) giving False.

https://stackoverflow.com/a/33199669 Some "evidence"

But have you actually had that problem here?

No, but I would say it's good practice preventing problems

Theophile-Madet · 2026-02-22T11:27:25Z

tapir/shifts/templatetags/shifts.py

+
+@register.simple_tag
+def random_shift_text(total_hours, no_of_past_shifts, first_shift_date):
+    texts = [


I'm not sure I understand the point of this, is it not all the same sentence but formulated slightly differently?

I didn't like any text so I thought it would be nice if they change from time to time.

but I'm also not fighting for this feature

Then I'd remove it, otherwise it's not clear why it's here.

Co-authored-by: Théophile MADET <theo.madet@posteo.net>

# Conflicts: # tapir/translations/locale/de/LC_MESSAGES/django.po

Theophile-Madet · 2026-02-27T09:07:21Z

tapir/shifts/templatetags/shifts.py

+
+@register.simple_tag
+def random_shift_text(total_hours, no_of_past_shifts, first_shift_date):
+    texts = [


Then I'd remove it, otherwise it's not clear why it's here.

tapir/shifts/tests/test_get_past_shifts_data.py

# Conflicts: # tapir/translations/locale/de/LC_MESSAGES/django.po

crosspolar · 2026-03-13T14:28:46Z

So without the type hint you get a warning from PyCharm? What's the warning?

mh, it's gone idk. But I leave if it it's okay since the typing is also in other files. It was something like "ShiftFactory does not have slots"

# Conflicts: # tapir/translations/locale/de/LC_MESSAGES/django.po

crosspolar requested a review from Theophile-Madet August 8, 2025 16:44

crosspolar changed the title ~~feat: show simple statistics per shift~~ feat: show simple shift statistics Aug 8, 2025

Theophile-Madet requested changes Aug 12, 2025

View reviewed changes

crosspolar force-pushed the shift_stats branch from fa843d0 to 97634eb Compare August 12, 2025 16:19

crosspolar marked this pull request as ready for review August 12, 2025 16:19

crosspolar requested a review from Theophile-Madet August 12, 2025 16:28

Theophile-Madet requested changes Aug 25, 2025

View reviewed changes

tapir/shifts/views/views.py Outdated Show resolved Hide resolved

tapir/shifts/views/views.py Outdated Show resolved Hide resolved

tapir/shifts/views/views.py Show resolved Hide resolved

crosspolar force-pushed the shift_stats branch from 433d8f5 to 857aa04 Compare September 6, 2025 13:44

crosspolar requested a review from Theophile-Madet September 6, 2025 17:56

Theophile-Madet requested changes Sep 8, 2025

View reviewed changes

crosspolar added 13 commits September 9, 2025 16:38

feat: show simple statistics per shift

8e365e0

feat: show simple statistics for whole coop

f06da84

forgot Shift-import

d6a1bd2

new text

0db1c0f

only show when more than one person worked in it so you don't have we…

152ae4d

…ird "one people..."

Revert "feat: show simple statistics for whole coop"

81562bd

This reverts commit 81022b9

Revert "forgot Shift-import"

8aa4292

This reverts commit 69e5f13.

use annotations to reduce number of queries

087dcfc

move to function

cfc2939

test

70e4daa

fix tests

a0d341d

refactor tests

ef7ac3d

translation

df4a345

crosspolar force-pushed the shift_stats branch from 2361c82 to df4a345 Compare September 9, 2025 14:40

crosspolar requested a review from Theophile-Madet September 9, 2025 14:40

translation

372e17b

crosspolar added 10 commits February 7, 2026 21:08

Merge branch 'master' into shift_stats

b706d46

get_past_shifts_data should accept ShiftTemplate

0eb3b08

test_get_past_shifts_data_changedShiftTemplateDuration_correctSum

7f824d0

typing

c435d6c

remove non-needed imports

f962f3f

ensure consistent slot assignment

dc4f0d9

transl

fd44868

also show date of first_shift

db15129

show random text

36e3674

transl

fe14c97

crosspolar requested a review from Theophile-Madet February 8, 2026 10:50

Theophile-Madet requested changes Feb 22, 2026

View reviewed changes

crosspolar and others added 6 commits February 24, 2026 20:11

Update tapir/shifts/tests/test_get_past_shifts_data.py

44dd717

Co-authored-by: Théophile MADET <theo.madet@posteo.net>

Update tapir/shifts/tests/test_get_past_shifts_data.py

dcb891d

Co-authored-by: Théophile MADET <theo.madet@posteo.net>

fix mutable context

c8fa274

test: use constant not expensive expression

2d8050c

Merge branch 'master' into shift_stats

a81191e

# Conflicts: # tapir/translations/locale/de/LC_MESSAGES/django.po

transl

9b51401

crosspolar requested a review from Theophile-Madet February 24, 2026 19:40

Theophile-Madet previously approved these changes Feb 27, 2026

View reviewed changes

Merge branch 'refs/heads/master' into shift_stats

daa101d

# Conflicts: # tapir/translations/locale/de/LC_MESSAGES/django.po

crosspolar added 2 commits March 13, 2026 15:29

remove random_shift_text

28ea082

remove random_shift_text

5d670be

crosspolar dismissed Theophile-Madet’s stale review via 5d670be March 13, 2026 14:32

crosspolar added 4 commits March 13, 2026 15:33

Merge branch 'master' into shift_stats

1a7211e

# Conflicts: # tapir/translations/locale/de/LC_MESSAGES/django.po

transl

c2cfaa1

Merge branch 'master' into shift_stats

7c684df

# Conflicts: # tapir/translations/locale/de/LC_MESSAGES/django.po

translation

c19bab2

crosspolar requested a review from Theophile-Madet March 28, 2026 11:31

Conversation

crosspolar commented Aug 8, 2025

Uh oh!

Theophile-Madet left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Theophile-Madet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crosspolar commented Sep 6, 2025

Uh oh!

Theophile-Madet left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

crosspolar commented Sep 9, 2025

Uh oh!

Theophile-Madet commented Sep 15, 2025

Uh oh!

crosspolar commented Sep 16, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

crosspolar commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Theophile-Madet left a comment •

edited

Loading

crosspolar commented Mar 13, 2026 •

edited

Loading