Implement new penalty rules#3349
Open
Arashfa0301 wants to merge 4 commits into
Open
Conversation
Arashfa0301
added
help-wanted
ivarnakken
added
new-feature
norbye
reviewed
May 1, 2023
norbye
left a comment
norbye
left a comment
Member
There was a problem hiding this comment.
I don't really have enough insight into how this works to be able to give a thorough review without spending too much time on it right now, but I've skimmed through the majority on it and left a few comments(:
Arashfa0301
force-pushed
the
new-penalty-rules
branch
2 times, most recently
from
1df65ce to
7a487fa
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #3349 +/- ##
==========================================
- Coverage 88.30% 88.26% -0.04%
==========================================
Files 661 662 +1
Lines 20968 20786 -182
==========================================
- Hits 18515 18347 -168
+ Misses 2453 2439 -14
☔ View full report in Codecov by Sentry. |
juniwbjerde
reviewed
May 2, 2023
juniwbjerde
reviewed
May 2, 2023
juniwbjerde
reviewed
May 2, 2023
juniwbjerde
reviewed
May 2, 2023
juniwbjerde
reviewed
May 2, 2023
eikhr
reviewed
May 2, 2023
juniwbjerde
reviewed
May 2, 2023
juniwbjerde
reviewed
May 2, 2023
Arashfa0301
removed
help-wanted
Arashfa0301
force-pushed
the
new-penalty-rules
branch
from
a036bfc to
d531414
Compare
Arashfa0301
requested review from
AdrianAndersen,
LudvigHz,
PeterJFB,
danielyanghansen,
eikhr,
falbru,
itsisak,
ivarnakken,
jonasdeluna,
juniwbjerde,
norbye and
theodorsm
and removed request for
PeterJFB and
ivarnakken
falbru
requested changes
Sep 30, 2023
falbru
left a comment
falbru
left a comment
Contributor
There was a problem hiding this comment.
Good job!
I tried to migrate on my local backend. However, I got this error message. Do you have any idea what caused this issue?
Traceback (most recent call last):
File "/home/falk/projs/webkom/lego/manage.py", line 10, in <module>
execute_from_command_line(sys.argv)
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/core/management/__init__.py", line 446, in execute_from_command_line
utility.execute()
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/core/management/__init__.py", line 440, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/core/management/base.py", line 414, in run_from_argv
self.execute(*args, **cmd_options)
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/core/management/base.py", line 460, in execute
output = self.handle(*args, **options)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/core/management/base.py", line 98, in wrapped
res = handle_func(*args, **kwargs)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/core/management/commands/migrate.py", line 290, in handle
post_migrate_state = executor.migrate(
^^^^^^^^^^^^^^^^^
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/migrations/executor.py", line 131, in migrate
state = self._migrate_all_forwards(
^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/migrations/executor.py", line 163, in _migrate_all_forwards
state = self.apply_migration(
^^^^^^^^^^^^^^^^^^^^^
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/migrations/executor.py", line 248, in apply_migration
state = migration.apply(state, schema_editor)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/migrations/migration.py", line 131, in apply
operation.database_forwards(
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/migrations/operations/fields.py", line 108, in database_forwards
schema_editor.add_field(
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 641, in add_field
self.execute(sql, params)
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/backends/base/schema.py", line 192, in execute
cursor.execute(sql, params)
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/backends/utils.py", line 102, in execute
return super().execute(sql, params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/backends/utils.py", line 67, in execute
return self._execute_with_wrappers(
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
return executor(sql, params, many, context)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/backends/utils.py", line 84, in _execute
with self.db.wrap_database_errors:
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/utils.py", line 91, in __exit__
raise dj_exc_value.with_traceback(traceback) from exc_value
File "/home/falk/projs/webkom/lego/venv/lib/python3.11/site-packages/django/db/backends/utils.py", line 89, in _execute
return self.cursor.execute(sql, params)
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
django.db.utils.IntegrityError: column "penalty_group_id" contains null values
| .vscode/ | ||
|
|
||
| # mac | ||
| .DS_Store |
Contributor
There was a problem hiding this comment.
Suggested change
| .DS_Store | |
| **/.DS_Store |
Your current code only removes the .DS_Store in the root directory
eikhr
reviewed
Oct 4, 2023
eikhr
reviewed
Oct 4, 2023
|
|
||
| pools = Pool.objects.filter(permission_groups__in=self.all_groups).distinct() | ||
|
|
||
| # TODO: i believe wrong. |
eikhr
reviewed
Oct 4, 2023
Comment on lines
+648
to
+652
| # Generated by Django 4.0.10 on 2023-05-01 16:33 | ||
|
|
||
| from django.db import models | ||
|
|
||
|
|
juniwbjerde
approved these changes
Oct 4, 2023
juniwbjerde
left a comment
juniwbjerde
left a comment
Member
There was a problem hiding this comment.
Just a bit of nitpick, looks good otherwise!
| PenaltyGroup.objects.create( | ||
| user=user, | ||
| reason="Dette er en sammensetting av forrige prikkene", | ||
| source_event=dummy_event, |
Member
There was a problem hiding this comment.
I think that the source event here should be one of the events from the earlier penalties, and that the reason should include a list of the reasons that the user got the original penalties. Something like: "Dette er en sammensettning av tidligere prikker pga. migrering til nytt prikksystem. Prikkene som inngår i denne prikken er: 'kom for sent' (Julebord), 'slo til bedrep' (bedpress ned Warehouse), ..."
| .filter( | ||
| penalty_group__user=self, | ||
| ) | ||
| .count() |
Member
There was a problem hiding this comment.
This does not really make that much sense anymore, as a user never has any more than one penalty, but I suppose it doesn't hurt
ollfkaih
approved these changes
Oct 4, 2023
|
|
||
| pools = Pool.objects.filter(permission_groups__in=self.all_groups).distinct() | ||
|
|
||
| # TODO: i believe wrong. |
ABA-92 Implement new rules for prikk
After a meeting between all the committees responsible for events a conclusion to alter the rules for prikker was reached. This has to be implemented. The new rules are:
i.e. the number of prikks you have does not really matter anymore, it only matters how long you have them. pr: #3349 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implement new rules and consequences for penalties. The changes are as following:
The biggest change is the addition of PenaltyGroup for the purpose of separating penalty consequence calculation logic and the the information used in front-end.
I have deleted some tests that believe are not needed for our new implementation and i have also made changes to events app that i deemed necessary. I would appreciate reviewing specially these changes as it might be possible that i made some mistakes.
Resolves ABA-92