Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce "too late" as an event presence status #3564

Merged
merged 3 commits into from
Mar 15, 2024

Conversation

ivarnakken
Copy link
Member

@ivarnakken ivarnakken commented Mar 6, 2024

Similar to the "not present" status, the user will automatically receive a penalty. I figured all previous penalties of type "presence" can be removed since they should not stack.

Also, this resolves a bug, as it is currently not possible to auto delete previous penalties related to the event due to the .delete() not being at the end the atomic block. Not sure why it had to be atomic in the first place.

@ivarnakken ivarnakken added new-feature Pull requests that introduce or issues that suggest a new feature bug-fix Pull requests that fix a bug review-needed Pull requests that need review question Pull requests with an unresolved question labels Mar 6, 2024
@ivarnakken ivarnakken requested a review from a team March 6, 2024 23:36
@ivarnakken ivarnakken self-assigned this Mar 6, 2024
Copy link

linear bot commented Mar 6, 2024

@ivarnakken ivarnakken added the testing-needed Pull requests that need testing, e.g. stress tests by users or specs label Mar 6, 2024
Copy link

codecov bot commented Mar 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.27%. Comparing base (c48fb05) to head (d7117b0).
Report is 7 commits behind head on master.

❗ Current head d7117b0 differs from pull request most recent head c9acb5f. Consider uploading reports for the commit c9acb5f to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3564      +/-   ##
==========================================
+ Coverage   88.24%   88.27%   +0.03%     
==========================================
  Files         677      679       +2     
  Lines       21385    21441      +56     
==========================================
+ Hits        18871    18927      +56     
  Misses       2514     2514              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

lego/apps/events/models.py Show resolved Hide resolved
lego/apps/events/models.py Outdated Show resolved Hide resolved
Similar to the "not present" status, the user will automatically recieve
a penalty. I figured all previous penalties can be removed since only
the newest one should matter.

I would like some input as to how this will affect other penalties,
e.g. given due to late payments (or other things?), as they will also
be removed if the presence is changed to something worthy of a penalty.
Should we consider to introduce "penalty types" stored on the record,
e.g. "presence", "payment", "other"?

Also, this resolves a bug, as it is currently not possible to auto delete
previous penalties related to the event due to the `.delete()` not being
at the end the atomic block. Not sure why it had to be atomic in the
first place.
@ivarnakken ivarnakken force-pushed the ivarnakken/aba-868-too-late branch 2 times, most recently from e6f4aae to 425e53c Compare March 14, 2024 10:50
@ivarnakken ivarnakken removed testing-needed Pull requests that need testing, e.g. stress tests by users or specs question Pull requests with an unresolved question labels Mar 14, 2024
Copy link
Member

@norbye norbye left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome stuff!

lego/apps/users/constants.py Show resolved Hide resolved
lego/apps/users/constants.py Show resolved Hide resolved
@ivarnakken ivarnakken added the approved Pull requests that have been approved label Mar 15, 2024
Used to distinguish and delete the correct penalties, as penalties from
the same type (and event) should in theory never stack.
@ivarnakken ivarnakken merged commit 46c514f into master Mar 15, 2024
1 check passed
@ivarnakken ivarnakken deleted the ivarnakken/aba-868-too-late branch March 15, 2024 20:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Pull requests that have been approved bug-fix Pull requests that fix a bug new-feature Pull requests that introduce or issues that suggest a new feature review-needed Pull requests that need review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants