diff --git a/kolibri/core/auth/test/sync_utils.py b/kolibri/core/auth/test/sync_utils.py index 958aab0b904..7a015cf23ea 100644 --- a/kolibri/core/auth/test/sync_utils.py +++ b/kolibri/core/auth/test/sync_utils.py @@ -56,6 +56,7 @@ def __init__( self.port = get_free_tcp_port() self.baseurl = "http://127.0.0.1:{}/".format(self.port) self.enable_automatic_download = enable_automatic_download + self._instance = None if seeded_kolibri_home is not None: shutil.rmtree(self.env["KOLIBRI_HOME"]) shutil.copytree(seeded_kolibri_home, self.env["KOLIBRI_HOME"]) @@ -132,7 +133,8 @@ def _wait_for_server_start(self, timeout=20): def kill(self): try: subprocess.Popen("kolibri stop", env=self.env, shell=True) - self._instance.kill() + if self._instance is not None: + self._instance.kill() shutil.rmtree(self.env["KOLIBRI_HOME"]) except OSError: pass @@ -188,6 +190,7 @@ def generate_base_data(self): class multiple_kolibri_servers: def __init__(self, count=2, **server_kwargs): self.server_count = count + self.servers = [] self.server_kwargs = [ { key: value[i] if isinstance(value, (list, tuple)) else value @@ -197,6 +200,19 @@ def __init__(self, count=2, **server_kwargs): ] def __enter__(self): + try: + self._start_servers() + except BaseException: + # Servers left running hold connections that stop every later test + # creating its own database. BaseException: Django exits rather than + # raises when it cannot clobber one. + self.__exit__(None, None, None) + raise + return self.servers + + def _start_servers(self): + # the same instance is reused for every invocation, so start from scratch + self.servers = [] # spin up the servers if "sqlite" in connection.vendor: tempserver = KolibriServer( @@ -208,12 +224,15 @@ def __enter__(self): tempserver.delete_model(DatabaseIDModel) preseeded_home = tempserver.env["KOLIBRI_HOME"] - self.servers = [ - KolibriServer( - seeded_kolibri_home=preseeded_home, **self.server_kwargs[i] + for i in range(self.server_count): + # track before starting, so a failed start is still shut down + server = KolibriServer( + autostart=False, + seeded_kolibri_home=preseeded_home, + **self.server_kwargs[i], ) - for i in range(self.server_count) - ] + self.servers.append(server) + server.start() # calculate the DATABASE settings for server in self.servers: @@ -259,17 +278,22 @@ def __enter__(self): server_conn.close() server.start() - return self.servers - def __exit__(self, typ, val, traceback): - # make sure all the servers are shut down + # kill every server before touching any database, so that a database that + # refuses to drop cannot abort the loop and leave later servers running for server in self.servers: server.kill() + for server in self.servers: + # a server abandoned before its alias was registered has no database + if server.db_alias not in connections.databases: + continue # destroy the test databases server_conn = connections[server.db_alias] try: server_conn.creation.destroy_test_db() - except OSError: + except Exception: + # Nothing narrower will do: Django surfaces a missing database + # as a RuntimeError from _nodb_cursor. pass server_conn.close() # Remove the database alias from settings to prevent subsequent tests diff --git a/kolibri/core/auth/test/test_auth_tasks.py b/kolibri/core/auth/test/test_auth_tasks.py index 4d734eb108d..bc7f6494580 100644 --- a/kolibri/core/auth/test/test_auth_tasks.py +++ b/kolibri/core/auth/test/test_auth_tasks.py @@ -74,6 +74,8 @@ class dummy_orm_job_data: interval = 8600 retry_interval = 5 max_retries = 3 + last_finished_state = None + last_finished_time = None @patch("kolibri.core.tasks.viewsets.tasks.job_storage") diff --git a/kolibri/core/tasks/job.py b/kolibri/core/tasks/job.py index 78d5eb6cf8c..d139bed6bb4 100644 --- a/kolibri/core/tasks/job.py +++ b/kolibri/core/tasks/job.py @@ -86,6 +86,12 @@ class State: CANCELING, } + FINISHED_STATES = { + FAILED, + CANCELED, + COMPLETED, + } + JobStatus = namedtuple("Status", ("title", "text")) @@ -263,6 +269,18 @@ def __init__( self._supervisor_id = NO_VALUE self.func = callable_to_import_path(func) + def reset_for_new_run(self): + """ + Clear the fields describing a single run, so a repeating job does not + inherit the previous run's progress or outcome. extra_metadata is kept: + it is what the task manager renders for the run that just finished. + """ + self.exception = None + self.traceback = "" + self.progress = 0 + self.total_progress = 0 + self.result = None + def _check_storage_attached(self): if self._storage is None: raise ReferenceError( diff --git a/kolibri/core/tasks/migrations/0005_add_last_finished_fields.py b/kolibri/core/tasks/migrations/0005_add_last_finished_fields.py new file mode 100644 index 00000000000..7e04e86f4ec --- /dev/null +++ b/kolibri/core/tasks/migrations/0005_add_last_finished_fields.py @@ -0,0 +1,21 @@ +from django.db import migrations +from django.db import models + + +class Migration(migrations.Migration): + dependencies = [ + ("kolibritasks", "0004_add_supervisor_registry"), + ] + + operations = [ + migrations.AddField( + model_name="job", + name="last_finished_state", + field=models.CharField(blank=True, max_length=20, null=True), + ), + migrations.AddField( + model_name="job", + name="last_finished_time", + field=models.DateTimeField(blank=True, null=True), + ), + ] diff --git a/kolibri/core/tasks/models.py b/kolibri/core/tasks/models.py index ea4949e0e9f..36d0de6bb6f 100644 --- a/kolibri/core/tasks/models.py +++ b/kolibri/core/tasks/models.py @@ -55,6 +55,11 @@ class Job(models.Model): # Maximum number of retries allowed for the job max_retries = models.IntegerField(null=True, blank=True) + # The outcome of the last finished run, which a repeating job's own state + # stops describing as soon as the job is re-queued. + last_finished_state = models.CharField(max_length=20, null=True, blank=True) + last_finished_time = models.DateTimeField(null=True, blank=True) + # References the supervisor currently responsible for this job. # No FK constraint to avoid complexity with supervisor cleanup. supervisor_id = UUIDField(null=True, blank=True) diff --git a/kolibri/core/tasks/storage.py b/kolibri/core/tasks/storage.py index 66b05d85795..ae74e16e1eb 100644 --- a/kolibri/core/tasks/storage.py +++ b/kolibri/core/tasks/storage.py @@ -475,11 +475,7 @@ def clear(self, queue=None, job_id=None, force=False): # filter only by the finished jobs, if we are not specified to force if not force: - queryset = queryset.filter( - Q(state=State.COMPLETED) - | Q(state=State.FAILED) - | Q(state=State.CANCELED) - ) + queryset = queryset.filter(state__in=State.FINISHED_STATES) if self._hooks: for orm_job in queryset: @@ -673,7 +669,7 @@ def reschedule_finished_job_if_needed( # noqa: C901 orm_job = self.get_orm_job(job_id) # Only allow this function to be run on a job that is in a finished state. - if orm_job.state not in {State.COMPLETED, State.FAILED, State.CANCELED}: + if orm_job.state not in State.FINISHED_STATES: raise JobNotRestartable( "Cannot reschedule job with state={}".format(orm_job.state) ) @@ -712,10 +708,7 @@ def reschedule_finished_job_if_needed( # noqa: C901 current_retries = orm_job.retries if orm_job.retries is not None else 0 kwargs["retries"] = current_retries + 1 - elif ( - orm_job.state in {State.COMPLETED, State.FAILED, State.CANCELED} - and kwargs["repeat"] != 0 - ): + elif orm_job.state in State.FINISHED_STATES and kwargs["repeat"] != 0: # Otherwise, if we are in a finished state and repeat is not 0, then we can reschedule, either because # repeat is None, or because repeat is not None and is greater than 0. if kwargs["repeat"] is not None: @@ -847,6 +840,9 @@ def _write_job_update( self, job, orm_job, state, supervisor_id, kwargs, repeat=NO_VALUE ): if state is not None: + # orm_job.state is the state being left, until the assignment below. + if state == State.RUNNING and orm_job.state != State.RUNNING: + job.reset_for_new_run() orm_job.state = job.state = state # Ownership exists only in supervised states; a bare re-mark # preserves the owner, a terminal state clears it. @@ -855,6 +851,9 @@ def _write_job_update( orm_job.supervisor_id = supervisor_id else: orm_job.supervisor_id = None + if state in State.FINISHED_STATES: + orm_job.last_finished_state = state + orm_job.last_finished_time = self._now() # repeat is nullable, so None is a real value; NO_VALUE means "leave it". if repeat is not NO_VALUE: orm_job.repeat = repeat diff --git a/kolibri/core/tasks/test/taskrunner/test_storage.py b/kolibri/core/tasks/test/taskrunner/test_storage.py index bf3f56c834b..e1df2978b2e 100644 --- a/kolibri/core/tasks/test/taskrunner/test_storage.py +++ b/kolibri/core/tasks/test/taskrunner/test_storage.py @@ -484,6 +484,106 @@ def test_reschedule_finished_job_failed(self, defaultbackend, simplejob): assert requeued_job.state == State.QUEUED assert requeued_orm_job.scheduled_time > previous_scheduled_time + def test_reschedule_recurring_job_keeps_last_finished( + self, defaultbackend, simplejob + ): + job_id = defaultbackend.enqueue_at( + local_now(), simplejob, QUEUE, interval=10, repeat=None + ) + assert defaultbackend.get_orm_job(job_id).last_finished_state is None + defaultbackend.complete_job(job_id) + + defaultbackend.reschedule_finished_job_if_needed(job_id) + + requeued_orm_job = defaultbackend.get_orm_job(job_id) + + assert requeued_orm_job.state == State.QUEUED + assert requeued_orm_job.last_finished_state == State.COMPLETED + assert requeued_orm_job.last_finished_time is not None + + def test_reschedule_with_delay_keeps_last_finished(self, defaultbackend, simplejob): + job_id = defaultbackend.enqueue_job(simplejob, QUEUE) + defaultbackend.complete_job(job_id) + + defaultbackend.reschedule_finished_job_if_needed( + job_id, delay=datetime.timedelta(seconds=5) + ) + + requeued_orm_job = defaultbackend.get_orm_job(job_id) + + assert requeued_orm_job.state == State.QUEUED + assert requeued_orm_job.last_finished_state == State.COMPLETED + assert requeued_orm_job.last_finished_time is not None + + def test_last_finished_describes_the_most_recent_run( + self, defaultbackend, simplejob + ): + exception = ValueError("Error") + job_id = defaultbackend.enqueue_job( + simplejob, QUEUE, retry_interval=5, max_retries=3 + ) + defaultbackend.mark_job_as_failed(job_id, exception, "Traceback") + defaultbackend.reschedule_finished_job_if_needed(job_id, exception=exception) + assert defaultbackend.get_orm_job(job_id).last_finished_state == State.FAILED + defaultbackend.mark_job_as_running(job_id) + + defaultbackend.complete_job(job_id) + + assert defaultbackend.get_orm_job(job_id).last_finished_state == State.COMPLETED + + def test_clear_leaves_rescheduled_recurring_job(self, defaultbackend, simplejob): + job_id = defaultbackend.enqueue_at( + local_now(), simplejob, QUEUE, interval=10, repeat=None + ) + defaultbackend.complete_job(job_id) + defaultbackend.reschedule_finished_job_if_needed(job_id) + + defaultbackend.clear(force=False) + + assert defaultbackend.get_orm_job(job_id).state == State.QUEUED + + def test_marking_running_clears_completed_run_fields( + self, defaultbackend, simplejob + ): + job_id = defaultbackend.enqueue_job(simplejob, QUEUE) + defaultbackend.mark_job_as_running(job_id) + defaultbackend.update_job_progress(job_id, 5, 10) + defaultbackend.complete_job(job_id, result="run one") + + defaultbackend.mark_job_as_running(job_id) + + job = defaultbackend.get_job(job_id) + + assert job.progress == 0 + assert job.total_progress == 0 + assert job.result is None + + def test_marking_running_clears_failed_run_fields(self, defaultbackend, simplejob): + job_id = defaultbackend.enqueue_job(simplejob, QUEUE) + defaultbackend.mark_job_as_running(job_id) + defaultbackend.mark_job_as_failed(job_id, ValueError("Error"), "Traceback") + + defaultbackend.mark_job_as_running(job_id) + + job = defaultbackend.get_job(job_id) + + assert job.exception is None + assert job.traceback == "" + + def test_marking_running_keeps_last_finished(self, defaultbackend, simplejob): + job_id = defaultbackend.enqueue_at( + local_now(), simplejob, QUEUE, interval=10, repeat=None + ) + defaultbackend.complete_job(job_id) + defaultbackend.reschedule_finished_job_if_needed(job_id) + + defaultbackend.mark_job_as_running(job_id) + + orm_job = defaultbackend.get_orm_job(job_id) + + assert orm_job.last_finished_state == State.COMPLETED + assert orm_job.last_finished_time is not None + def test_reschedule_finished_job_invalid_state_queued( self, defaultbackend, simplejob ): diff --git a/kolibri/core/tasks/test/test_api.py b/kolibri/core/tasks/test/test_api.py index 6f3b44cc545..64f5f3f5a30 100644 --- a/kolibri/core/tasks/test/test_api.py +++ b/kolibri/core/tasks/test/test_api.py @@ -20,11 +20,13 @@ from kolibri.core.tasks.exceptions import JobRunning from kolibri.core.tasks.job import Job from kolibri.core.tasks.job import State +from kolibri.core.tasks.main import job_storage from kolibri.core.tasks.permissions import CanManageContent from kolibri.core.tasks.permissions import IsSuperAdmin from kolibri.core.tasks.registry import RegisteredTask from kolibri.core.tasks.registry import TaskRegistry from kolibri.core.tasks.validation import JobValidator +from kolibri.utils.time_utils import local_now DUMMY_PASSWORD = "password" @@ -55,6 +57,8 @@ class dummy_orm_job_data: interval = 8600 retry_interval = 5 max_retries = 3 + last_finished_state = None + last_finished_time = None class BaseAPITestCase(APITestCase): @@ -280,6 +284,8 @@ def add(x, y): "extra_metadata": {}, "facility_id": None, "scheduled_datetime": dummy_orm_job_data.scheduled_time.isoformat(), + "last_finished_status": None, + "last_finished_datetime": None, "repeat": dummy_orm_job_data.repeat, "repeat_interval": dummy_orm_job_data.interval, "retry_interval": dummy_orm_job_data.retry_interval, @@ -340,6 +346,8 @@ def add(**kwargs): "extra_metadata": {}, "facility_id": None, "scheduled_datetime": dummy_orm_job_data.scheduled_time.isoformat(), + "last_finished_status": None, + "last_finished_datetime": None, "repeat": dummy_orm_job_data.repeat, "repeat_interval": dummy_orm_job_data.interval, "retry_interval": dummy_orm_job_data.retry_interval, @@ -359,6 +367,8 @@ def add(**kwargs): "extra_metadata": {}, "facility_id": None, "scheduled_datetime": dummy_orm_job_data.scheduled_time.isoformat(), + "last_finished_status": None, + "last_finished_datetime": None, "repeat": dummy_orm_job_data.repeat, "repeat_interval": dummy_orm_job_data.interval, "retry_interval": dummy_orm_job_data.retry_interval, @@ -444,6 +454,8 @@ def add(**kwargs): "facility": "kolibri HQ", }, "scheduled_datetime": dummy_orm_job_data.scheduled_time.isoformat(), + "last_finished_status": None, + "last_finished_datetime": None, "repeat": dummy_orm_job_data.repeat, "repeat_interval": dummy_orm_job_data.interval, "retry_interval": dummy_orm_job_data.retry_interval, @@ -537,6 +549,8 @@ def add(x, y): "facility": "kolibri HQ", }, "scheduled_datetime": dummy_orm_job_data.scheduled_time.isoformat(), + "last_finished_status": None, + "last_finished_datetime": None, "repeat": dummy_orm_job_data.repeat, "repeat_interval": dummy_orm_job_data.interval, "retry_interval": dummy_orm_job_data.retry_interval, @@ -558,6 +572,8 @@ def add(x, y): "facility": "kolibri HQ", }, "scheduled_datetime": dummy_orm_job_data.scheduled_time.isoformat(), + "last_finished_status": None, + "last_finished_datetime": None, "repeat": dummy_orm_job_data.repeat, "repeat_interval": dummy_orm_job_data.interval, "retry_interval": dummy_orm_job_data.retry_interval, @@ -1221,6 +1237,8 @@ def subtract(x, y): "kwargs": {}, "extra_metadata": {}, "scheduled_datetime": dummy_orm_job_data.scheduled_time.isoformat(), + "last_finished_status": None, + "last_finished_datetime": None, "repeat": dummy_orm_job_data.repeat, "repeat_interval": dummy_orm_job_data.interval, "retry_interval": dummy_orm_job_data.retry_interval, @@ -1240,6 +1258,8 @@ def subtract(x, y): "kwargs": {}, "extra_metadata": {}, "scheduled_datetime": dummy_orm_job_data.scheduled_time.isoformat(), + "last_finished_status": None, + "last_finished_datetime": None, "repeat": dummy_orm_job_data.repeat, "repeat_interval": dummy_orm_job_data.interval, "retry_interval": dummy_orm_job_data.retry_interval, @@ -1259,6 +1279,8 @@ def subtract(x, y): "kwargs": {}, "extra_metadata": {}, "scheduled_datetime": dummy_orm_job_data.scheduled_time.isoformat(), + "last_finished_status": None, + "last_finished_datetime": None, "repeat": dummy_orm_job_data.repeat, "repeat_interval": dummy_orm_job_data.interval, "retry_interval": dummy_orm_job_data.retry_interval, @@ -1488,3 +1510,52 @@ def test_csrf_protected_task(self): reverse("kolibri:core:task-list"), {}, format="json" ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + +class RepeatingTaskResponseAPITestCase(BaseAPITestCase): + TASK_ID = "kolibri.core.tasks.test.test_api.add" + + def setUp(self): + @register_task(permission_classes=[IsSuperAdmin], queue="kolibri") + def add(x, y): + return x + y + + TaskRegistry[self.TASK_ID] = add + self.client.login(username=self.superuser.username, password=DUMMY_PASSWORD) + + def tearDown(self): + # Only drop what this case registered - clearing the whole registry + # leaves later test modules without the tasks they import. + del TaskRegistry[self.TASK_ID] + job_storage.clear(force=True) + + def _run_and_reschedule_recurring_job(self): + job_id = job_storage.enqueue_at( + local_now(), + Job(self.TASK_ID, args=(1, 2)), + queue="kolibri", + interval=10, + repeat=None, + ) + job_storage.complete_job(job_id) + job_storage.reschedule_finished_job_if_needed(job_id) + + def _get_only_task(self): + response = self.client.get(reverse("kolibri:core:task-list")) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.data), 1) + return response.data[0] + + def test_rescheduled_recurring_job_reports_last_run(self): + self._run_and_reschedule_recurring_job() + + task = self._get_only_task() + + self.assertEqual(task["status"], State.QUEUED) + self.assertEqual(task["last_finished_status"], State.COMPLETED) + self.assertTrue(task["last_finished_datetime"]) + + def test_rescheduled_recurring_job_is_not_clearable(self): + self._run_and_reschedule_recurring_job() + + self.assertFalse(self._get_only_task()["clearable"]) diff --git a/kolibri/core/tasks/viewsets/tasks.py b/kolibri/core/tasks/viewsets/tasks.py index 77936d1eb55..b77064099f6 100644 --- a/kolibri/core/tasks/viewsets/tasks.py +++ b/kolibri/core/tasks/viewsets/tasks.py @@ -92,12 +92,16 @@ def _job_to_response(self, job): "percentage": job.percentage_progress, "id": job.job_id, "cancellable": job.cancellable, - "clearable": job.state in [State.FAILED, State.CANCELED, State.COMPLETED], + "clearable": job.state in State.FINISHED_STATES, "facility_id": job.facility_id, "args": job.args, "kwargs": job.kwargs, "extra_metadata": job.extra_metadata, "scheduled_datetime": orm_job.scheduled_time.isoformat(), + "last_finished_status": orm_job.last_finished_state, + "last_finished_datetime": orm_job.last_finished_time.isoformat() + if orm_job.last_finished_time + else None, "repeat": orm_job.repeat, "repeat_interval": orm_job.interval, "retry_interval": orm_job.retry_interval, @@ -323,7 +327,7 @@ def clear(self, request, pk=None): """ job_to_clear = self._get_job_for_pk(request, pk) - if job_to_clear.state not in (State.COMPLETED, State.FAILED, State.CANCELED): + if job_to_clear.state not in State.FINISHED_STATES: raise serializers.ValidationError( "Cannot clear job with state: {}".format(job_to_clear.state) ) diff --git a/kolibri/plugins/device/frontend/views/FacilitiesPage/FacilitiesTasksPage.vue b/kolibri/plugins/device/frontend/views/FacilitiesPage/FacilitiesTasksPage.vue index 8676412bde3..549e7a9acc2 100644 --- a/kolibri/plugins/device/frontend/views/FacilitiesPage/FacilitiesTasksPage.vue +++ b/kolibri/plugins/device/frontend/views/FacilitiesPage/FacilitiesTasksPage.vue @@ -16,7 +16,7 @@ -

+

{{ deviceString('emptyTasksMessage') }}

diff --git a/kolibri/plugins/device/frontend/views/FacilitiesPage/__tests__/FacilitiesPage.spec.js b/kolibri/plugins/device/frontend/views/FacilitiesPage/__tests__/FacilitiesPage.spec.js new file mode 100644 index 00000000000..b01edb6b52b --- /dev/null +++ b/kolibri/plugins/device/frontend/views/FacilitiesPage/__tests__/FacilitiesPage.spec.js @@ -0,0 +1,57 @@ +import { render, screen } from '@testing-library/vue'; +import TaskResource from 'kolibri/apiResources/TaskResource'; +import FacilityResource from 'kolibri-common/apiResources/FacilityResource'; +import { crossComponentTranslator } from 'kolibri/utils/i18n'; +import { TaskStatuses } from 'kolibri-common/utils/syncTaskUtils'; +import FacilityNameAndSyncStatus from 'kolibri-common/components/syncComponentSet/FacilityNameAndSyncStatus'; +import { + FACILITY_ID, + FACILITY_NAME, + syncSchedule, +} from 'kolibri-common/utils/__tests__/syncSchedule'; +import FacilitiesPage from '../index.vue'; + +jest.mock('kolibri/urls'); +jest.mock('kolibri-plugin-data', () => ({ + __esModule: true, + default: { deprecationWarnings: {} }, +})); +jest.mock('kolibri/apiResources/TaskResource', () => ({ + list: jest.fn(), +})); +jest.mock('kolibri-common/apiResources/FacilityResource', () => ({ + fetchCollection: jest.fn(), +})); + +const { syncing$ } = crossComponentTranslator(FacilityNameAndSyncStatus); + +const FACILITY = { + id: FACILITY_ID, + name: FACILITY_NAME, + dataset: { registered: true }, + last_successful_sync: null, +}; + +async function renderPage(tasks) { + TaskResource.list.mockResolvedValue(tasks); + FacilityResource.fetchCollection.mockResolvedValue([FACILITY]); + render(FacilitiesPage, { + routes: [{ path: '/facilities/tasks', name: 'FACILITIES_TASKS_PAGE' }], + }); + await global.flushPromises(); +} + +describe('FacilitiesPage', () => { + it('shows a facility as syncing while its scheduled sync is running', async () => { + await renderPage([syncSchedule({ status: TaskStatuses.RUNNING })]); + + expect(screen.getByText(syncing$())).toBeInTheDocument(); + }); + + it('does not show a facility as syncing between runs of its sync schedule', async () => { + await renderPage([syncSchedule({ lastFinishedStatus: TaskStatuses.COMPLETED })]); + + expect(screen.getByText(FACILITY_NAME)).toBeInTheDocument(); + expect(screen.queryByText(syncing$())).not.toBeInTheDocument(); + }); +}); diff --git a/kolibri/plugins/device/frontend/views/FacilitiesPage/__tests__/FacilitiesTasksPage.spec.js b/kolibri/plugins/device/frontend/views/FacilitiesPage/__tests__/FacilitiesTasksPage.spec.js new file mode 100644 index 00000000000..390e63b9f38 --- /dev/null +++ b/kolibri/plugins/device/frontend/views/FacilitiesPage/__tests__/FacilitiesTasksPage.spec.js @@ -0,0 +1,47 @@ +import { render, screen } from '@testing-library/vue'; +import TaskResource from 'kolibri/apiResources/TaskResource'; +import { coreString } from 'kolibri/uiText/commonCoreStrings'; +import { syncFacilityTaskDisplayInfo, TaskStatuses } from 'kolibri-common/utils/syncTaskUtils'; +import { syncSchedule } from 'kolibri-common/utils/__tests__/syncSchedule'; +import { deviceStrings } from '../../commonDeviceStrings'; +import FacilitiesTasksPage from '../FacilitiesTasksPage'; + +jest.mock('kolibri/apiResources/TaskResource', () => ({ + list: jest.fn(), +})); + +const { emptyTasksMessage$ } = deviceStrings; + +const SYNC_HEADING = syncFacilityTaskDisplayInfo(syncSchedule()).headingMsg; + +async function renderPage(tasks) { + TaskResource.list.mockResolvedValue(tasks); + render(FacilitiesTasksPage, { + routes: [{ path: '/facilities', name: 'FACILITIES_PAGE' }], + }); + // Let the poll resolve, so an empty list and a filtered-out one differ. + await global.flushPromises(); +} + +describe('FacilitiesTasksPage', () => { + it('lists a repeating sync showing the run that just finished, with no clear or retry', async () => { + const task = syncSchedule({ lastFinishedStatus: TaskStatuses.COMPLETED }); + // Wording is asserted in syncTaskUtils.spec.js; here only that it is rendered. + const { statusMsg, bytesTransferredMsg } = syncFacilityTaskDisplayInfo(task); + await renderPage([task]); + + expect(screen.getByText(SYNC_HEADING)).toBeInTheDocument(); + expect(screen.getByText(statusMsg)).toBeInTheDocument(); + expect(screen.getByText(bytesTransferredMsg)).toBeInTheDocument(); + for (const action of ['clearAction', 'retryAction', 'cancelAction']) { + expect(screen.queryByRole('button', { name: coreString(action) })).not.toBeInTheDocument(); + } + }); + + it('shows the no-tasks message when a schedule that has never run is the only task', async () => { + await renderPage([syncSchedule()]); + + expect(screen.getByText(emptyTasksMessage$())).toBeInTheDocument(); + expect(screen.queryByText(SYNC_HEADING)).not.toBeInTheDocument(); + }); +}); diff --git a/kolibri/plugins/device/frontend/views/FacilitiesPage/facilityTasksQueue.js b/kolibri/plugins/device/frontend/views/FacilitiesPage/facilityTasksQueue.js index da469367090..0aa781cc3e7 100644 --- a/kolibri/plugins/device/frontend/views/FacilitiesPage/facilityTasksQueue.js +++ b/kolibri/plugins/device/frontend/views/FacilitiesPage/facilityTasksQueue.js @@ -10,10 +10,13 @@ function taskFacilityMatch(task, facility) { } function isActiveTask(task) { - // Helper function filter tasks by whether they are 'active' - // i.e. has a user just queued a non-repeating task, or is a repeating task - // that is currently running. - return task.repeat !== null || task.status === TaskStatuses.RUNNING; + // A schedule that has never run is not a task the user is watching — it + // belongs on Manage sync schedule. + return ( + task.repeat !== null || + task.status === TaskStatuses.RUNNING || + Boolean(task.last_finished_datetime) + ); } export default { @@ -50,12 +53,13 @@ export default { return this.facilityTasks.filter(isActiveTask); }, facilityIsSyncing() { - return function isSyncing(facility) { - const inProcessSyncTasks = this.activeFacilityTasks.filter( - t => isSyncTask(t) && !t.clearable, + return facility => + this.facilityTasks.some( + task => + isSyncTask(task) && + task.status === TaskStatuses.RUNNING && + taskFacilityMatch(task, facility), ); - return Boolean(inProcessSyncTasks.find(task => taskFacilityMatch(task, facility))); - }; }, facilityIsDeleting() { return function isDeleting(facility) { diff --git a/kolibri/plugins/device/frontend/views/FacilitiesPage/index.vue b/kolibri/plugins/device/frontend/views/FacilitiesPage/index.vue index 2a9869c1dea..5299158c22c 100644 --- a/kolibri/plugins/device/frontend/views/FacilitiesPage/index.vue +++ b/kolibri/plugins/device/frontend/views/FacilitiesPage/index.vue @@ -204,7 +204,7 @@ import RegisterFacilityModal from 'kolibri-common/components/syncComponentSet/RegisterFacilityModal'; import ConfirmationRegisterModal from 'kolibri-common/components/syncComponentSet/ConfirmationRegisterModal'; import SyncFacilityModalGroup from 'kolibri-common/components/syncComponentSet/SyncFacilityModalGroup'; - import { TaskStatuses, TaskTypes } from 'kolibri-common/utils/syncTaskUtils'; + import { runEndedSince, TaskStatuses, TaskTypes } from 'kolibri-common/utils/syncTaskUtils'; import some from 'lodash/some'; import useSnackbar from 'kolibri/composables/useSnackbar'; import { pageLoading } from 'kolibri-common/composables/usePageLoading'; @@ -331,10 +331,8 @@ return false; } const prevTask = prevTasks.find(({ id }) => id === task.id); - if (!prevTask) { - return false; - } - return prevTask.status === TaskStatuses.RUNNING && task.status === TaskStatuses.QUEUED; + // A RUNNING -> QUEUED transition is missed whenever polling straddles it. + return Boolean(prevTask) && runEndedSince(task, prevTask.last_finished_datetime); }, facilityOptions() { return [ diff --git a/kolibri/plugins/device/frontend/views/ManageContentPage/TasksBar.vue b/kolibri/plugins/device/frontend/views/ManageContentPage/TasksBar.vue index 4ba9ed21b88..8e022cde964 100644 --- a/kolibri/plugins/device/frontend/views/ManageContentPage/TasksBar.vue +++ b/kolibri/plugins/device/frontend/views/ManageContentPage/TasksBar.vue @@ -39,6 +39,7 @@ import sumBy from 'lodash/sumBy'; import commonCoreStrings from 'kolibri/uiText/commonCoreStrings'; import commonTaskStrings from 'kolibri-common/uiText/tasks'; + import { taskIsFinished } from 'kolibri-common/utils/syncTaskUtils'; export default { name: 'TasksBar', @@ -60,22 +61,24 @@ totalTasks() { return this.tasks.length; }, - clearableTasks() { - return this.tasks.filter(t => t.clearable); + // Splitting on `clearable` would count a repeating task as in progress + // forever, since it is re-queued the instant a run ends. + finishedTasks() { + return this.tasks.filter(taskIsFinished); }, inProgressTasks() { - return this.tasks.filter(t => !t.clearable); + return this.tasks.filter(t => !taskIsFinished(t)); }, progress() { return ( - ((this.clearableTasks.length + sumBy(this.inProgressTasks, 'percentage')) / + ((this.finishedTasks.length + sumBy(this.inProgressTasks, 'percentage')) / this.totalTasks) * 100 ); }, tasksString() { return this.$tr('someTasksComplete', { - done: this.clearableTasks.length, + done: this.finishedTasks.length, total: this.totalTasks, }); }, diff --git a/kolibri/plugins/facility/frontend/views/DataPage/SyncInterface/index.vue b/kolibri/plugins/facility/frontend/views/DataPage/SyncInterface/index.vue index 43156e1b7ae..2650778bcfa 100644 --- a/kolibri/plugins/facility/frontend/views/DataPage/SyncInterface/index.vue +++ b/kolibri/plugins/facility/frontend/views/DataPage/SyncInterface/index.vue @@ -120,7 +120,11 @@ import commonCoreStrings from 'kolibri/uiText/commonCoreStrings'; import CoreMenu from 'kolibri/components/CoreMenu'; import CoreMenuOption from 'kolibri/components/CoreMenu/CoreMenuOption'; - import { TaskStatuses } from 'kolibri-common/utils/syncTaskUtils'; + import { + runEndedSince, + taskDisplayStatus, + TaskStatuses, + } from 'kolibri-common/utils/syncTaskUtils'; import { SyncPageNames } from 'kolibri-common/components/SyncSchedule/constants'; import useFacility from 'kolibri-common/composables/useFacility'; import PrivacyModal from './PrivacyModal'; @@ -163,6 +167,7 @@ kdpProject: null, // { name, token } modalShown: null, syncTaskId: '', + syncTaskLastFinished: null, isSyncing: false, syncHasFailed: false, Modals, @@ -219,13 +224,19 @@ pollSyncTask() { // Like facilityTaskQueue, just keep polling until component is destroyed TaskResource.get(this.syncTaskId).then(task => { - if (task.clearable) { + if (runEndedSince(task, this.syncTaskLastFinished)) { this.isSyncing = false; - TaskResource.clear(this.syncTaskId); + // Clearing a repeating row would delete the facility's sync + // schedule, and it is briefly clearable between the run ending and + // the re-schedule that requeues it. + if (task.clearable && task.repeat === 0) { + TaskResource.clear(this.syncTaskId); + } this.syncTaskId = ''; - if (task.status === TaskStatuses.FAILED) { + const status = taskDisplayStatus(task); + if (status === TaskStatuses.FAILED) { this.syncHasFailed = true; - } else if (task.status === TaskStatuses.COMPLETED) { + } else if (status === TaskStatuses.COMPLETED) { this.fetchFacility(); } } else if (this.syncTaskId) { @@ -244,9 +255,12 @@ this.fetchFacilityConfig(); this.closeModal(); }, - handleSyncFacilitySuccess(taskId) { + handleSyncFacilitySuccess(task) { this.isSyncing = true; - this.syncTaskId = taskId; + this.syncTaskId = task.id; + // A manual sync may land on an existing schedule, which already carries + // the previous run's snapshot; the run we started is what changes it. + this.syncTaskLastFinished = task.last_finished_datetime; this.pollSyncTask(); }, handleSyncFacilityFailure() { @@ -260,7 +274,7 @@ this.closeModal(); this.startKdpSyncTask(this.facilityId) .then(task => { - this.handleSyncFacilitySuccess(task.id); + this.handleSyncFacilitySuccess(task); }) .catch(() => { this.handleSyncFacilityFailure(); @@ -273,7 +287,7 @@ device_id: peerData.id, }) .then(task => { - this.handleSyncFacilitySuccess(task.id); + this.handleSyncFacilitySuccess(task); }) .catch(() => { this.handleSyncFacilityFailure(); diff --git a/packages/kolibri-common/components/SyncSchedule/ManageSyncSchedule.vue b/packages/kolibri-common/components/SyncSchedule/ManageSyncSchedule.vue index fa80f2cfeee..a2c05ee2cf9 100644 --- a/packages/kolibri-common/components/SyncSchedule/ManageSyncSchedule.vue +++ b/packages/kolibri-common/components/SyncSchedule/ManageSyncSchedule.vue @@ -37,10 +37,11 @@