Details
-
Type:
Bug
-
Status:
Reopened
-
Priority:
Major
-
Resolution: Unresolved
-
Affects Version/s: Current Version
-
Fix Version/s: None
-
Component/s: OIOIOI
-
Labels:None
Description
Sometimes when a teacher is adding a contest, the contest is created with an empty `controller_name`, which causes `controller` to be None, which causes tons of AttributeErrors on 2/3 of pages, including the main portal / landing page.
Stacktrace from a sanity check I added to diagnose this issue:
https://sentry.io/the-sio2-project/szkopul/issues/351545304/
{code}
ValueError: Tried to save contest with empty controller
File "oioioi/contests/models.py", line 79, in save
raise ValueError("Tried to save contest with empty controller")
File "oioioi/contests/forms.py", line 76, in save
instance.save()
File "oioioi/base/admin.py", line 333, in add_view
return model_admin.add_view(request, form_url, extra_context)
...
(12 additional frame(s) were not displayed)
ValueError: Tried to save contest with empty controller
{code}
Probable cause:
If there are no rounds specified, this code saves the contest even if it's not supposed to (commit=False):
https://github.com/sio2project/oioioi/blob/master/oioioi/contests/forms.py#L73-L82
Git blame suggests it's been there since the very beginning, I have on idea why this issue didn't appear before.
Stacktrace from a sanity check I added to diagnose this issue:
https://sentry.io/the-sio2-project/szkopul/issues/351545304/
{code}
ValueError: Tried to save contest with empty controller
File "oioioi/contests/models.py", line 79, in save
raise ValueError("Tried to save contest with empty controller")
File "oioioi/contests/forms.py", line 76, in save
instance.save()
File "oioioi/base/admin.py", line 333, in add_view
return model_admin.add_view(request, form_url, extra_context)
...
(12 additional frame(s) were not displayed)
ValueError: Tried to save contest with empty controller
{code}
Probable cause:
If there are no rounds specified, this code saves the contest even if it's not supposed to (commit=False):
https://github.com/sio2project/oioioi/blob/master/oioioi/contests/forms.py#L73-L82
Git blame suggests it's been there since the very beginning, I have on idea why this issue didn't appear before.
Activity
- All
- Comments
- History
- Activity
- Transitions
- Commits
Also, the `rounds` variable there is a queryset, and when adding a new contest it will always be empty, because the function changeform_view in django/contrib/admin/options.py saves the main form (which is about Contest in this case) before it saves formsets (which are about Rounds, etc)
Show
Wojciech Dubiel
added a comment - Also, the `rounds` variable there is a queryset, and when adding a new contest it will always be empty, because the function changeform_view in django/contrib/admin/options.py saves the main form (which is about Contest in this case) before it saves formsets (which are about Rounds, etc)
Wojciech Dubiel
added a comment - Also, the `rounds` variable there is a queryset, and when adding a new contest it will always be empty, because the function changeform_view in django/contrib/admin/options.py saves the main form (which is about Contest in this case) before it saves formsets (which are about Rounds, etc) By the way, I removed my "sanity check", because it had kind of false-positives (maybe some other code is most of the time saving the contest with proper controller name later on), and it made it impossible for teachers to add contests at all.
Show
Wojciech Dubiel
added a comment - By the way, I removed my "sanity check", because it had kind of false-positives (maybe some other code is most of the time saving the contest with proper controller name later on), and it made it impossible for teachers to add contests at all.
Wojciech Dubiel
added a comment - By the way, I removed my "sanity check", because it had kind of false-positives (maybe some other code is most of the time saving the contest with proper controller name later on), and it made it impossible for teachers to add contests at all. The last Sentry entry is dated Sep 19, 2017 9:19:23 PM UTC. It looks like the problem has been fixed. Please reopen if you know it isn't.
Show
Szymon Acedański
added a comment - The last Sentry entry is dated Sep 19, 2017 9:19:23 PM UTC. It looks like the problem has been fixed. Please reopen if you know it isn't.
Szymon Acedański
added a comment - The last Sentry entry is dated Sep 19, 2017 9:19:23 PM UTC. It looks like the problem has been fixed. Please reopen if you know it isn't. I put the following tempfix in `Contest.save()`, that's why the issue is no longer happening on production.
{code}
def save(self, *args, **kwargs):
if not self.controller_name:
self.controller_name = 'oioioi.teachers.controllers.TeacherContestController'
super(Contest, self).save(*args, **kwargs)
{/code}
{code}
def save(self, *args, **kwargs):
if not self.controller_name:
self.controller_name = 'oioioi.teachers.controllers.TeacherContestController'
super(Contest, self).save(*args, **kwargs)
{/code}
Show
Wojciech Dubiel
added a comment - - edited I put the following tempfix in `Contest.save()`, that's why the issue is no longer happening on production.
{code}
def save(self, *args, **kwargs):
if not self.controller_name:
self.controller_name = 'oioioi.teachers.controllers.TeacherContestController'
super(Contest, self).save(*args, **kwargs)
{/code}
Wojciech Dubiel
added a comment - - edited I put the following tempfix in `Contest.save()`, that's why the issue is no longer happening on production.
{code}
def save(self, *args, **kwargs):
if not self.controller_name:
self.controller_name = 'oioioi.teachers.controllers.TeacherContestController'
super(Contest, self).save(*args, **kwargs)
{/code} This issue was not updated in a year. Maybe it's not "Priority: critical"?
Show
Artur Puzio
added a comment - - edited This issue was not updated in a year. Maybe it's not "Priority: critical"?
Artur Puzio
added a comment - - edited This issue was not updated in a year. Maybe it's not "Priority: critical"? Well, we have a tempfix so I'm changing it to major.
Still, the tempfix is very szkopul-specific and the contest creation code could use some refactor anyway.
Still, the tempfix is very szkopul-specific and the contest creation code could use some refactor anyway.
Show
Wojciech Dubiel
added a comment - Well, we have a tempfix so I'm changing it to major.
Still, the tempfix is very szkopul-specific and the contest creation code could use some refactor anyway.
Wojciech Dubiel
added a comment - Well, we have a tempfix so I'm changing it to major.
Still, the tempfix is very szkopul-specific and the contest creation code could use some refactor anyway. This issue has been automatically closed as Obsolete due to no activity for 365 days.
Feel free to reopen it or create a new one if it's still relevant.
Feel free to reopen it or create a new one if it's still relevant.
Show
Szymon Acedański
added a comment - This issue has been automatically closed as Obsolete due to no activity for 365 days.
Feel free to reopen it or create a new one if it's still relevant.
Szymon Acedański
added a comment - This issue has been automatically closed as Obsolete due to no activity for 365 days.
Feel free to reopen it or create a new one if it's still relevant. Still no proper fix. The workaround is szkopul-specific, and not in upstream.
Show
Wojciech Dubiel
added a comment - Still no proper fix. The workaround is szkopul-specific, and not in upstream.
Wojciech Dubiel
added a comment - Still no proper fix. The workaround is szkopul-specific, and not in upstream. 
At least this round.save():
https://github.com/sio2project/oioioi/blob/master/oioioi/contests/forms.py#L79
Will save the contest even if SimpleContestForm.save is called with commit=False and there were no rounds, as there is no way to insert the new round into the database without inserting the contest first, as there is a foreign key configured.
Maybe something changed with the upgraded version of Django.