Skip to content

Conversation

@ihsaan-ullah
Copy link
Collaborator

@ihsaan-ullah ihsaan-ullah commented Jun 19, 2023

@ mention of reviewers

@Didayolo

A brief description of the purpose of the changes contained in this PR.

Now users can add new phase in a competition

Issues this PR resolves

Checklist

  • Code review by me
  • Hand tested by me
  • I'm proud of my work
  • Code review by reviewer
  • Hand tested by reviewer
  • CircleCi tests are passing
  • Ready to merge

@Didayolo
Copy link
Member

@bbearce Hi Benjamin, would you have a chance to review and test this PR?

@Didayolo
Copy link
Member

Didayolo commented Jun 20, 2023

When I add a new phase, then the editor's Save button does nothing.

Note that I selected the right start and end dates, and put the phase in the right order.

Here are the logs I get:

codabench-django-1          | Internal Server Error: /api/competitions/12/
codabench-django-1          | Traceback (most recent call last):
codabench-django-1          |   File "/usr/local/lib/python3.8/site-packages/django/core/handlers/exception.py", line 34, in inner
codabench-django-1          |     response = get_response(request)
codabench-django-1          |   File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py", line 115, in _get_response
codabench-django-1          |     response = self.process_exception_by_middleware(e, request)
codabench-django-1          |   File "/usr/local/lib/python3.8/site-packages/django/core/handlers/base.py", line 113, in _get_response
codabench-django-1          |     response = wrapped_callback(request, *callback_args, **callback_kwargs)
codabench-django-1          |   File "/usr/local/lib/python3.8/site-packages/django/views/decorators/csrf.py", line 54, in wrapped_view
codabench-django-1          |     return view_func(*args, **kwargs)
codabench-django-1          |   File "/usr/local/lib/python3.8/site-packages/rest_framework/viewsets.py", line 116, in view
codabench-django-1          |     return self.dispatch(request, *args, **kwargs)
codabench-django-1          |   File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 495, in dispatch
codabench-django-1          |     response = self.handle_exception(exc)
codabench-django-1          |   File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 455, in handle_exception
codabench-django-1          |     self.raise_uncaught_exception(exc)
codabench-django-1          |   File "/usr/local/lib/python3.8/site-packages/rest_framework/views.py", line 492, in dispatch
codabench-django-1          |     response = handler(request, *args, **kwargs)
codabench-django-1          |   File "/usr/local/lib/python3.8/site-packages/rest_framework/mixins.py", line 84, in partial_update
codabench-django-1          |     return self.update(request, *args, **kwargs)
codabench-django-1          |   File "/app/src/apps/api/views/competitions.py", line 221, in update
codabench-django-1          |     auto_migrate_to_this_phase=phase["auto_migrate_to_this_phase"],
codabench-django-1          | KeyError: 'auto_migrate_to_this_phase'
codabench-django-1          |   File "/usr/local/lib/python3.8/site-packages/rest_framework/mixins.py", line 84, in partial_update
codabench-django-1          |     return self.update(request, *args, **kwargs)
codabench-django-1          |   File "/app/src/apps/api/views/competitions.py", line 221, in update
codabench-django-1          |     auto_migrate_to_this_phase=phase["auto_migrate_to_this_phase"],
codabench-django-1          | KeyError: 'auto_migrate_to_this_phase'

@ihsaan-ullah
Copy link
Collaborator Author

When I add a new phase, then the editor's Save button does nothing.

Latest commit has solved this issue

@Didayolo
Copy link
Member

But now we have this error in CircleCI:

E               django.db.utils.IntegrityError: duplicate key value violates unique constraint "leaderboards_column_leaderboard_id_key_aa81662d_uniq"
E               DETAIL:  Key (leaderboard_id, key)=(7, nature) already exists.

/usr/local/lib/python3.8/site-packages/django/db/backends/utils.py:84: IntegrityError
=========================== short test summary info ============================
FAILED src/apps/api/tests/test_competitions.py::CompetitionResultDatatypesTests::test_get_competition_leaderboard_as_json
===== 1 failed, 167 passed, 9 deselected, 89 warnings in 84.84s (0:01:24) ======

@ihsaan-ullah
Copy link
Collaborator Author

I don't know why it failed but rerunning the test fixed the failure :D

@ihsaan-ullah
Copy link
Collaborator Author

ihsaan-ullah commented Jun 20, 2023

To check

  • when a competition is deleted, its phases are also deleted?

Yes phase is deleted with competition

@Didayolo
Copy link
Member

Depending on the order and dates of phase, I still have the same problem: nothing happens when I click on save.

First test

I put the newly created phase as the oldest (top of the list), with start and end dates before the next phase. It saves and work fine.

Second test

I put the newly created phase as the newest (bottom of the list), with a starting date and no end date. I edit the dates of the other phases so they start and finish before the newly created phase. This DOES NOT save.

@Didayolo
Copy link
Member

Didayolo commented Jun 22, 2023

I don't know why it failed but rerunning the test fixed the failure :D

Isn't it a bit concerning that it failed in the first place?

@ihsaan-ullah
Copy link
Collaborator Author

Isn't it a bit concerning that it failed in the first place?

It seems that the issue was because of an entry in the db. Not sure what exactly caused it

@ihsaan-ullah
Copy link
Collaborator Author

ihsaan-ullah commented Jun 22, 2023

More tests:

I have a competition with 2 phases: Phase 1 and Phase 2
Screenshot 2023-06-22 at 8 25 47 PM

Screenshot 2023-06-22 at 8 25 59 PM

Phase 1 : 01 Jan 2023 - 31 Jan 2023.
Phase 2 : 01 Feb 2023 - 28 Feb 2023.

Adding two phases to this competiiton: one before phase 1 and one after phase 2

Phase 0 01 Dec 2022 - 31 Dec 2022. ✅
Screenshot 2023-06-22 at 8 29 55 PM

Phase 3 01 Mar 2023 - 31 Mar 2023. ✅
Screenshot 2023-06-22 at 8 31 29 PM

Screenshot 2023-06-22 at 8 32 58 PM

@Didayolo I think everything is working. Can you try once more with a fresh branch and with collectstatic.

@Didayolo
Copy link
Member

Didayolo commented Jun 22, 2023

In my case, the last phase was "never ending" before I changed the dates. Maybe the problem comes from that? It was a fresh deployement with collect static.

@ihsaan-ullah
Copy link
Collaborator Author

In my case, the last phase was "never ending" before I changed the dates. Maybe the problem comes from that? It was a fresh deployement with collect static.

i will check this condition too

@ihsaan-ullah
Copy link
Collaborator Author

@Didayolo Final phase has no end date. And when i add a phase after that, I get this error which seems reasonable. What do you think?

Screenshot 2023-06-22 at 9 55 55 PM

@Didayolo
Copy link
Member

Didayolo commented Jun 22, 2023

Final phase has no end date. And when i add a phase after that, I get this error which seems reasonable. What do you think?

OK, so I was exactly in this situation, and updated Final phase and Final Final (new addition) phase dates to remove the warning:

  • Put an end date to Final
  • Put a start date to Final Final (new addition) and no end date

So the warning disappear but then the Save button does not work.

@ihsaan-ullah
Copy link
Collaborator Author

@Didayolo that was a nice catch. Now last phase can have no end date.

@Didayolo
Copy link
Member

The fix is a nice catch too!

@Didayolo Didayolo merged commit 7b72adf into develop Jun 23, 2023
@Didayolo Didayolo deleted the add_phase branch June 23, 2023 11:19
@Didayolo Didayolo mentioned this pull request Jun 23, 2023
18 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants