ihsaan-ullah commented 2 months ago

@ mention of reviewers

@Didayolo

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

This PR solves all the issues related to making tasks public and using them in competitions either by owners or other users. The following changes are made to enable this functionality.

From Task model, condition removed that forced tasks to be validated first before making them public
Fron-end updated to show making a task public/private button. There was a condition to show this button only when a task is validated. Now that is removed
Updated task update method on the backend to enable updating is_public status of a task.
Task updation was never tested and there was a problem in the response of the update API. Now that is fixed and the API returns a serialized response to show updated status.
Removed key from the task detail serializer. In the task detail, there was always key present. That was not needed and is now removed
Some modifications were needed in TaskListSerializer to properly use validated status and sharedwith list. Now that is fixed.
Added is_public to TaskListSerializer to show a checkmark in the tasks list table.
Public tasks were shown in the tasks interface when a checkbox Show public tasks was checked but when this public task was clicked, details were not shown because it was not permitted from the backend. Now the backend is updated to return details for public tasks.
When you edit a competition->phase from the UI, public tasks were not shown in the tasks list, now users can see and select public tasks in their phases

Issues this PR resolves

1578

A checklist for hand testing

[x] With user 1 create a task and make it public using the task detail interface. Make sure that you can make the task public and private
[x] With user 2 go to Resource interface -> Tasks tab, click show public tasks checkbox. Check that the task made public by user 1 is shown here. Check that you can click the task and check its details
[x] With user 2 create a competition, go to edit competition->Phases. In one phase search the same task in the task dropdown and check that you can select this task for your phase and save changes.
[x] Now run a submission to check that the task works.
[x] check that only owners of tasks can make them public/private

Checklist

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

ihsaan-ullah commented 2 months ago

@Didayolo this PR is ready for review. Please ignore the documentation comment, I will complete the documentation later when other task related features are added.

Didayolo commented 1 month ago

@ihsaan-ullah This test failed:

FAILED src/apps/api/tests/test_tasks.py::TestTasks::test_task_shown_as_validated_properly

_______________ TestTasks.test_task_shown_as_validated_properly ________________

self = <api.tests.test_tasks.TestTasks testMethod=test_task_shown_as_validated_properly>

    def test_task_shown_as_validated_properly(self):
        user = UserFactory(username='test')
        solution = SolutionFactory(md5="12345")
        task = TaskFactory(created_by=user, solutions=[solution])
        competition = CompetitionFactory(created_by=user)
        phase = PhaseFactory(competition=competition, tasks=[task])
        submission = SubmissionFactory(md5="12345", phase=phase, status=Submission.FINISHED)
        url = reverse('task-detail', kwargs={'pk': task.id})
        self.client.login(username=user.username, password='test')

        # task should be validated because we have a successful submission matching
        # our solution
        resp = self.client.get(url)
        assert resp.status_code == 200
        assert resp.data["validated"]

        # make submission anything but Submission.FINISHED, task -> invalidated
        submission.status = Submission.FAILED
        submission.save()
        resp = self.client.get(url)
        assert resp.status_code == 200
>       assert not resp.data["validated"]
E       AssertionError: assert not True

src/apps/api/tests/test_tasks.py:30: AssertionError
----------------------------- Captured stdout call -----------------------------
This solution has no data associated with it...might be a test
This solution has no data associated with it...might be a test

ihsaan-ullah commented 1 month ago

This test is failing because of a change I made on this line in Task Serializer:

https://github.com/codalab/codabench/blob/de55ef3fc3642194d194fb79ab61aad6a3e25d6d/src/apps/api/serializers/tasks.py#L126

on this line task.validated was checked BUT task has no property named validated. Task model has a filed named _validated

Task model: https://github.com/codalab/codabench/blob/de55ef3fc3642194d194fb79ab61aad6a3e25d6d/src/apps/tasks/models.py#L30

We need to discuss this validation of task in the next meeting so that I can fix the test

Didayolo commented 1 month ago

@ihsaan-ullah

If the task validation code is not really related:

Revert back the changes
Create an issue about this validated field

ihsaan-ullah commented 1 month ago

Task validation problem is shifted to #1078

ihsaan-ullah commented 1 month ago

Documentation added here: https://github.com/codalab/codabench/wiki/Public-Tasks-and-Tasks-Sharing and also added in the FAQs here

Didayolo commented 1 month ago

Trying to update a task:

codabench-django-1          | Internal Server Error: /api/tasks/72/
codabench-django-1          | Traceback (most recent call last):
codabench-django-1          |   File "/usr/local/lib/python3.9/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.9/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.9/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.9/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.9/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.9/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.9/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.9/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.9/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/tasks.py", line 126, in update
codabench-django-1          |     return Response(task_detail_serializer.data)
codabench-django-1          |   File "/usr/local/lib/python3.9/site-packages/rest_framework/serializers.py", line 563, in data
codabench-django-1          |     ret = super(Serializer, self).data
codabench-django-1          |   File "/usr/local/lib/python3.9/site-packages/rest_framework/serializers.py", line 262, in data
codabench-django-1          |     self._data = self.to_representation(self.instance)
codabench-django-1          |   File "/usr/local/lib/python3.9/site-packages/rest_framework/serializers.py", line 530, in to_representation
codabench-django-1          |     ret[field.field_name] = field.to_representation(attribute)
codabench-django-1          |   File "/usr/local/lib/python3.9/site-packages/rest_framework/fields.py", line 1893, in to_representation
codabench-django-1          |     return method(value)
codabench-django-1          |   File "/app/src/apps/api/serializers/tasks.py", line 127, in get_validated
codabench-django-1          |     return task.validated is not None
codabench-django-1          | AttributeError: 'Task' object has no attribute 'validated'
codabench-django-1          | Internal Server Error: /api/tasks/72/
codabench-django-1          | Traceback (most recent call last):
codabench-django-1          |   File "/usr/local/lib/python3.9/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.9/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.9/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.9/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.9/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.9/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.9/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.9/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.9/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/tasks.py", line 126, in update
codabench-django-1          |     return Response(task_detail_serializer.data)
codabench-django-1          |   File "/usr/local/lib/python3.9/site-packages/rest_framework/serializers.py", line 563, in data
codabench-django-1          |     ret = super(Serializer, self).data
codabench-django-1          |   File "/usr/local/lib/python3.9/site-packages/rest_framework/serializers.py", line 262, in data
codabench-django-1          |     self._data = self.to_representation(self.instance)
codabench-django-1          |   File "/usr/local/lib/python3.9/site-packages/rest_framework/serializers.py", line 530, in to_representation
codabench-django-1          |     ret[field.field_name] = field.to_representation(attribute)
codabench-django-1          |   File "/usr/local/lib/python3.9/site-packages/rest_framework/fields.py", line 1893, in to_representation
codabench-django-1          |     return method(value)
codabench-django-1          |   File "/app/src/apps/api/serializers/tasks.py", line 127, in get_validated
codabench-django-1          |     return task.validated is not None
codabench-django-1          | AttributeError: 'Task' object has no attribute 'validated'

ihsaan-ullah commented 1 month ago

Solved the issue by using correct serializer TaskSerializer instead of TaskDetailSerializer. Also updated the problematic function to not raise this issue again

codalab / codabench

Public tasks -- updates to ensure public tasks can be viewed and used #1581

@ mention of reviewers

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

Issues this PR resolves

1578

A checklist for hand testing

Checklist