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

Add submission cancel feature #3456

Merged
merged 6 commits into from
Aug 8, 2021
Merged

Conversation

Ram81
Copy link
Member

@Ram81 Ram81 commented Jun 6, 2021

Description

This PR adds -

  • Allow user to cancel submissions if the status is SUBMITTED

@codecov-commenter
Copy link

codecov-commenter commented Jun 6, 2021

Codecov Report

Merging #3456 (164a371) into master (96968d6) will decrease coverage by 1.45%.
The diff coverage is 32.43%.

@@            Coverage Diff             @@
##           master    #3456      +/-   ##
==========================================
- Coverage   72.93%   71.47%   -1.46%     
==========================================
  Files          83       20      -63     
  Lines        5368     3232    -2136     
==========================================
- Hits         3915     2310    -1605     
+ Misses       1453      922     -531     
Impacted Files Coverage Δ
frontend/src/js/controllers/authCtrl.js 53.91% <6.38%> (-12.95%) ⬇️
frontend/src/js/controllers/profileCtrl.js 79.76% <20.00%> (-13.10%) ⬇️
frontend/src/js/controllers/permissionCtrl.js 36.36% <22.22%> (-63.64%) ⬇️
frontend/src/js/controllers/challengeCtrl.js 63.41% <30.64%> (-10.29%) ⬇️
frontend/src/js/controllers/updateProfileCtrl.js 82.55% <44.44%> (-10.30%) ⬇️
frontend/src/js/controllers/challengeListCtrl.js 95.74% <50.00%> (+1.06%) ⬆️
...ntend/src/js/controllers/challengeHostTeamsCtrl.js 70.50% <66.66%> (-1.18%) ⬇️
frontend/src/js/controllers/teamsCtrl.js 71.17% <75.00%> (ø)
frontend/src/js/controllers/ChallengeInviteCtrl.js 100.00% <100.00%> (ø)
frontend/src/js/controllers/SubmissionFilesCtrl.js 95.45% <100.00%> (ø)
... and 30 more
Impacted Files Coverage Δ
frontend/src/js/controllers/authCtrl.js 53.91% <6.38%> (-12.95%) ⬇️
frontend/src/js/controllers/profileCtrl.js 79.76% <20.00%> (-13.10%) ⬇️
frontend/src/js/controllers/permissionCtrl.js 36.36% <22.22%> (-63.64%) ⬇️
frontend/src/js/controllers/challengeCtrl.js 63.41% <30.64%> (-10.29%) ⬇️
frontend/src/js/controllers/updateProfileCtrl.js 82.55% <44.44%> (-10.30%) ⬇️
frontend/src/js/controllers/challengeListCtrl.js 95.74% <50.00%> (+1.06%) ⬆️
...ntend/src/js/controllers/challengeHostTeamsCtrl.js 70.50% <66.66%> (-1.18%) ⬇️
frontend/src/js/controllers/teamsCtrl.js 71.17% <75.00%> (ø)
frontend/src/js/controllers/ChallengeInviteCtrl.js 100.00% <100.00%> (ø)
frontend/src/js/controllers/SubmissionFilesCtrl.js 95.45% <100.00%> (ø)
... and 30 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7116eb1...164a371. Read the comment docs.

Copy link
Member

@RishabhJain2018 RishabhJain2018 left a comment

Choose a reason for hiding this comment

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

Can you please also add the screenshot of the UI?

@@ -110,6 +110,11 @@
views.update_submission_started_at,
name="update_submission_started_at",
),
url(
r"^challenge/(?P<challenge_pk>[0-9]+)/submissions/(?P<submission_pk>[0-9]+)/cancel/$",
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need challenge_pk here? I think submission_pk should be enough here, no?

Copy link
Member Author

Choose a reason for hiding this comment

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

We only want the participants of the team to cancel the submission using UI. To check whether team has participated in the challenge we need challenge pk

@@ -110,6 +110,11 @@
views.update_submission_started_at,
name="update_submission_started_at",
),
url(
r"^challenge/(?P<challenge_pk>[0-9]+)/submissions/(?P<submission_pk>[0-9]+)/cancel/$",
views.change_submission_status_to_cancel,
Copy link
Member

Choose a reason for hiding this comment

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

We should create a generic API that changes the status of the submissions to other status as well.

Copy link
Member Author

@Ram81 Ram81 Jun 12, 2021

Choose a reason for hiding this comment

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

This API is specific to the user functionality of cancelling submission. Why should this API be allowed to set other statuses?

apps/jobs/views.py Outdated Show resolved Hide resolved
"""
API Endpoint for setting submission status to CANCELLED.
"""
print(request.user, challenge_pk)
Copy link
Member

Choose a reason for hiding this comment

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

Please remove the print statement here.

Copy link
Member

Choose a reason for hiding this comment

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

@Ram81 Can you please remove it?

Copy link
Member

Choose a reason for hiding this comment

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

@Ram81 ?

apps/jobs/views.py Outdated Show resolved Hide resolved
frontend/src/js/controllers/challengeCtrl.js Outdated Show resolved Hide resolved
"""
API Endpoint for setting submission status to CANCELLED.
"""
print(request.user, challenge_pk)
Copy link
Member

Choose a reason for hiding this comment

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

@Ram81 Can you please remove it?

)

try:
participant_team = ParticipantTeam.objects.get(pk=participant_team_pk)
Copy link
Member

Choose a reason for hiding this comment

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

Can we please use/create get_participant_model like we have for challenges/challenge phases.

participant_team=participant_team,
)
except Submission.DoesNotExist:
response_data = {"error": "Submission does not exist"}
Copy link
Member

Choose a reason for hiding this comment

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

{"error": "Submission {} does not exist".format(submission_pk)}

Copy link
Member

Choose a reason for hiding this comment

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

)
except Submission.DoesNotExist:
response_data = {"error": "Submission does not exist"}
return Response(response_data, status=status.HTTP_403_FORBIDDEN)
Copy link
Member

Choose a reason for hiding this comment

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

It should be 404 , no?

"""
API Endpoint for setting submission status to CANCELLED.
"""
print(request.user, challenge_pk)
Copy link
Member

Choose a reason for hiding this comment

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

@Ram81 ?

participant_team=participant_team,
)
except Submission.DoesNotExist:
response_data = {"error": "Submission does not exist"}
Copy link
Member

Choose a reason for hiding this comment

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

@RishabhJain2018 RishabhJain2018 merged commit c509e38 into Cloud-CV:master Aug 8, 2021
vaheta pushed a commit to vahetag/EvalAI that referenced this pull request Dec 10, 2024
* Add submission cancel feature

* Rebase changes

* Remove unused code

* Fix reviews

* Fix review

Co-authored-by: Rishabh Jain <[email protected]>
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.

3 participants