fix: UTC-445: Remove allow_skip functionality in LSO (#9047)

Co-authored-by: Caitlin <[email protected]>
Co-authored-by: mcanu <[email protected]>

Author: mcanu

docs/source/guide/skip.md

@@ -27,8 +27,6 @@ You can see which tasks have been skipped using the **Cancelled** column in the
 !!! error Enterprise
     Task skipping, and how tasks can be skipped, is highly configurable in Label Studio Enterprise and Starter Cloud. For more information, see [the Enterprise documentation](https://docs.humansignal.com/guide/skip). 
 
-To make tasks unskippable, you can include a special key in the JSON task definition that you import to your project.
-
 </div>
 
 <div class="enterprise-only">
@@ -47,8 +45,6 @@ Whether annotators can skip tasks, and what should happen to skipped tasks, is c
 
 While you can disallow skipping entirely from the project settings, if you want to have specific tasks be unskippable, you will need to configure that by adding a special key as part of the JSON task definition that you import to your project.
 
-</div>
-
 ## Individual unskippable tasks
 
 To make an individual task unskippable, specify `"allow_skip": false` as part of the [JSON task definition](tasks#Basic-Label-Studio-JSON-format) that you import to your project.
@@ -71,8 +67,6 @@ For example, the following JSON snippet would result in one skippable task and o
 ]
 ```
 
-<div class="enterprise-only">
-
 !!! note
     Managers, Admins, and Owners can still skip these tasks. Only Annotators and Reviewers cannot skip tasks that have been marked unskippable using this method.  
 
@@ -82,13 +76,5 @@ For example, the following JSON snippet would result in one skippable task and o
 
 </div>
 
-<div class="opensource-only">
-
-!!! info Tip
-    Use the **Allow Skip** column to see which tasks have skipping disabled and filter for unskippable tasks. 
-
-</div>
-
-
 
 

label_studio/data_export/serializers.py

@@ -1,5 +1,5 @@
-"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license.
-"""
+"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license."""
+
 from core.label_config import replace_task_data_undefined_with_config_field
 from core.utils.common import load_func
 from data_export.models import DataExport
@@ -168,8 +168,7 @@ class TaskFilterOptionsSerializer(serializers.Serializer):
         choices=ONLY_OR_EXCLUDE_CHOICE,
         allow_null=True,
         required=False,
-        help_text='`only` - include all finished tasks (is_labeled = true)<br>'
-        '`exclude` - exclude all finished tasks',
+        help_text='`only` - include all finished tasks (is_labeled = true)<br>`exclude` - exclude all finished tasks',
     )
     annotated = serializers.ChoiceField(
         choices=ONLY_OR_EXCLUDE_CHOICE,

label_studio/data_manager/serializers.py

@@ -1,5 +1,5 @@
-"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license.
-"""
+"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license."""
+
 import os
 
 import ujson as json
@@ -117,7 +117,7 @@ def _build_filter_tree(filter_obj):
             # Add child filter if exists (only one level of nesting)
             child_filters = filter_obj.children.all()
             if child_filters:
-                child = child_filters[0]   # Only support one child
+                child = child_filters[0]  # Only support one child
                 child_item = {
                     'filter': child.column,
                     'operator': child.operator,
@@ -228,7 +228,6 @@ def _create_filters(filter_group, filters_data):
         """
 
         def _create_recursive(data, parent=None, index=None):
-
             # Extract nested children early (if any) and remove them from payload
             child_filter = data.pop('child_filter', None)
 
@@ -466,7 +465,7 @@ class DataManagerTaskSerializer(TaskSerializer):
     class Meta:
         model = Task
         ref_name = 'data_manager_task_serializer'
-        exclude = ('precomputed_agreement',)
+        exclude = ('precomputed_agreement', 'allow_skip')
         expandable_fields = {'annotations': (AnnotationSerializer, {'many': True})}
 
     def to_representation(self, obj):
@@ -483,10 +482,6 @@ def to_representation(self, obj):
             and flag_set('fflag_feat_fit_710_fsm_state_fields', user=user)
         ):
             ret.pop('state', None)
-        # Ensure allow_skip is always present in the response, even if None
-        # This is important for frontend logic that checks allow_skip !== false
-        if 'allow_skip' not in ret:
-            ret['allow_skip'] = obj.allow_skip
         return ret
 
     def _pretty_results(self, task, field, unique=False):

label_studio/io_storages/base_models.py

@@ -1,5 +1,5 @@
-"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license.
-"""
+"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license."""
+
 import base64
 import concurrent.futures
 import itertools
@@ -250,8 +250,7 @@ def health_check(self):
             )
             self.save(update_fields=['status', 'traceback'])
             logger.info(
-                f'Storage {self} status moved to `failed` '
-                f'because the job {self.last_sync_job} has too old ping time'
+                f'Storage {self} status moved to `failed` because the job {self.last_sync_job} has too old ping time'
             )
 
     def job_health_check(self):
@@ -276,7 +275,7 @@ def job_health_check(self):
                 'This typically occurs due to an out-of-memory (OOM) error.'
             )
             self.save(update_fields=['status', 'traceback'])
-            logger.info(f'Storage {self} status moved to `failed` ' f'because of the failed job {self.last_sync_job}')
+            logger.info(f'Storage {self} status moved to `failed` because of the failed job {self.last_sync_job}')
 
         # job is not found in redis (maybe deleted while redeploy), storage status is still active
         elif job_status == 'not found':
@@ -288,9 +287,7 @@ def job_health_check(self):
                 'or workers reloaded unexpectedly.'
             )
             self.save(update_fields=['status', 'traceback'])
-            logger.info(
-                f'Storage {self} status moved to `failed` ' f'because the job {self.last_sync_job} was not found'
-            )
+            logger.info(f'Storage {self} status moved to `failed` because the job {self.last_sync_job} was not found')
 
 
 class Storage(StorageInfo):
@@ -449,14 +446,14 @@ def add_task(cls, project, maximum_annotations, max_inner_id, storage, link_obje
         link_kwargs = asdict(link_object)
         data = link_kwargs.pop('task_data', None)
 
-        allow_skip = data.get('allow_skip', None)
+        allow_skip = data.get('allow_skip', True)
 
         # predictions
         predictions = data.get('predictions') or []
         if predictions:
             if 'data' not in data:
                 raise ValueError(
-                    'If you use "predictions" field in the task, ' 'you must put "data" field in the task too'
+                    'If you use "predictions" field in the task, you must put "data" field in the task too'
                 )
 
         # annotations
@@ -465,7 +462,7 @@ def add_task(cls, project, maximum_annotations, max_inner_id, storage, link_obje
         if annotations:
             if 'data' not in data:
                 raise ValueError(
-                    'If you use "annotations" field in the task, ' 'you must put "data" field in the task too'
+                    'If you use "annotations" field in the task, you must put "data" field in the task too'
                 )
             cancelled_annotations = len([a for a in annotations if a.get('was_cancelled', False)])
 
@@ -486,7 +483,7 @@ def add_task(cls, project, maximum_annotations, max_inner_id, storage, link_obje
                 total_annotations=len(annotations) - cancelled_annotations,
                 cancelled_annotations=cancelled_annotations,
                 inner_id=max_inner_id,
-                allow_skip=(allow_skip if allow_skip is not None else True),
+                allow_skip=allow_skip,
             )
             # Save with skip_fsm flag to bypass FSM during bulk import
             task.save(skip_fsm=True)
@@ -889,7 +886,6 @@ class Meta:
 
 
 class ImportStorageLink(models.Model):
-
     task = models.OneToOneField('tasks.Task', on_delete=models.CASCADE, related_name='%(app_label)s_%(class)s')
     key = models.TextField(_('key'), null=False, help_text='External link key')
 
@@ -919,7 +915,6 @@ class Meta:
 
 
 class ExportStorageLink(models.Model):
-
     annotation = models.ForeignKey(
         'tasks.Annotation', on_delete=models.CASCADE, related_name='%(app_label)s_%(class)s'
     )

label_studio/tasks/api.py

@@ -1,5 +1,5 @@
-"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license.
-"""
+"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license."""
+
 import logging
 
 from core.feature_flags import flag_set
@@ -603,7 +603,7 @@ def perform_create(self, ser):
         was_cancelled_data = self.request.data.get('was_cancelled', False)
         is_skipping = was_cancelled_get or was_cancelled_data
 
-        if is_skipping and not task.allow_skip:
+        if is_skipping and not task.can_be_skipped():
             raise ValidationError({'detail': 'This task cannot be skipped.'})
 
         # updates history

label_studio/tasks/mixins.py

@@ -36,6 +36,9 @@ def after_bulk_delete_actions(tasks_ids, project):
     def get_rejected_query(self):
         pass
 
+    def can_be_skipped(self) -> bool:
+        return True
+
 
 class AnnotationMixin:
     def has_permission(self, user: 'User') -> bool:  # noqa: F821

label_studio/tasks/serializers.py

@@ -1,5 +1,5 @@
-"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license.
-"""
+"""This file and its contents are licensed under the Apache License 2.0. Please see the included NOTICE for copyright information and LICENSE for a copy of the license."""
+
 import logging
 
 import ujson as json
@@ -213,7 +213,7 @@ def to_representation(self, instance):
 
     class Meta:
         model = Task
-        exclude = ('precomputed_agreement',)
+        exclude = ('precomputed_agreement', 'allow_skip')
 
 
 class BaseTaskSerializer(FlexFieldsModelSerializer):
@@ -266,16 +266,11 @@ def to_representation(self, instance):
             data = instance.data
             replace_task_data_undefined_with_config_field(data, project)
 
-        ret = super().to_representation(instance)
-        # Ensure allow_skip is always present in the response, even if None
-        # This is important for frontend logic that checks allow_skip !== false
-        if 'allow_skip' not in ret:
-            ret['allow_skip'] = instance.allow_skip
-        return ret
+        return super().to_representation(instance)
 
     class Meta:
         model = Task
-        exclude = ('precomputed_agreement',)
+        exclude = ('precomputed_agreement', 'allow_skip')
 
 
 class BaseTaskSerializerBulk(serializers.ListSerializer):
@@ -439,7 +434,6 @@ def create(self, validated_data):
 
         # to be sure we add tasks with annotations at the same time
         with transaction.atomic():
-
             # extract annotations, predictions, drafts, reviews, etc
             # all these lists will be grouped by tasks, e.g.:
             # task_annotations = [ [a1, a2], [a3, a4, a5], ... ]
@@ -537,7 +531,7 @@ def add_predictions(self, task_predictions):
                             prediction_score = float(prediction_score)
                         except ValueError:
                             logger.error(
-                                "Can't upload prediction score: should be in float format." 'Fallback to score=None'
+                                "Can't upload prediction score: should be in float format.Fallback to score=None"
                             )
                             prediction_score = None
 
@@ -683,7 +677,7 @@ def add_tasks(self, task_annotations, task_predictions, validated_tasks):
                 total_predictions=len(task_predictions[i]),
                 total_annotations=total_annotations,
                 cancelled_annotations=cancelled_annotations,
-                allow_skip=task.get('allow_skip', True),  # Default to True for backward compatibility
+                allow_skip=task.get('allow_skip', True),
             )
             db_tasks.append(t)
 
@@ -807,7 +801,6 @@ class Meta:
 
 
 class TaskWithAnnotationsAndPredictionsAndDraftsSerializer(TaskSerializer):
-
     predictions = serializers.SerializerMethodField(default=[], read_only=True)
     annotations = serializers.SerializerMethodField(default=[], read_only=True)
     drafts = serializers.SerializerMethodField(default=[], read_only=True)

label_studio/tasks/tests/test_api.py

@@ -1,7 +1,6 @@
 from organizations.tests.factories import OrganizationFactory
 from projects.tests.factories import ProjectFactory
 from rest_framework.test import APITestCase
-from tasks.models import Task
 from tasks.tests.factories import TaskFactory
 
 
@@ -51,7 +50,6 @@ def test_get_task(self):
             'comment_count': 0,
             'last_comment_updated_at': None,
             'unresolved_comment_count': 0,
-            'allow_skip': True,
         }
 
     def test_patch_task(self):
@@ -94,7 +92,6 @@ def test_patch_task(self):
             'comment_count': 0,
             'last_comment_updated_at': None,
             'unresolved_comment_count': 0,
-            'allow_skip': True,
         }
 
     def test_create_task_without_project_id_fails(self):
@@ -126,67 +123,3 @@ def test_create_task_with_project_id_succeeds(self):
         response_data = response.json()
         assert response_data['project'] == self.project.id
         assert response_data['data'] == {'text': 'test task'}
-
-    def test_get_task_includes_allow_skip(self):
-        """Test that GET task API includes allow_skip field"""
-        task = TaskFactory(project=self.project, allow_skip=False)
-
-        self.client.force_authenticate(user=self.user)
-        response = self.client.get(f'/api/tasks/{task.id}/')
-        assert response.status_code == 200
-        response_data = response.json()
-        assert 'allow_skip' in response_data
-        assert response_data['allow_skip'] is False
-
-    def test_create_task_with_allow_skip(self):
-        """Test that creating a task with allow_skip field succeeds"""
-        payload = {
-            'project': self.project.id,
-            'data': {'text': 'test task'},
-            'meta': {},
-            'allow_skip': False,
-        }
-
-        self.client.force_authenticate(user=self.user)
-        response = self.client.post('/api/tasks/', data=payload, format='json')
-
-        assert response.status_code == 201
-        response_data = response.json()
-        assert response_data['allow_skip'] is False
-        task = Task.objects.get(id=response_data['id'])
-        assert task.allow_skip is False
-
-    def test_skip_unskippable_task_fails(self):
-        """Test that skipping a task with allow_skip=False fails"""
-        task = TaskFactory(project=self.project, allow_skip=False)
-
-        self.client.force_authenticate(user=self.user)
-        response = self.client.post(
-            f'/api/tasks/{task.id}/annotations/', data={'result': [], 'was_cancelled': True}, format='json'
-        )
-
-        assert response.status_code == 400
-        response_data = response.json()
-        assert 'cannot be skipped' in str(response_data).lower()
-
-    def test_skip_skippable_task_succeeds(self):
-        """Test that skipping a task with allow_skip=True succeeds"""
-        task = TaskFactory(project=self.project, allow_skip=True)
-
-        self.client.force_authenticate(user=self.user)
-        response = self.client.post(
-            f'/api/tasks/{task.id}/annotations/', data={'result': [], 'was_cancelled': True}, format='json'
-        )
-
-        assert response.status_code == 201
-
-    def test_skip_task_with_default_allow_skip_succeeds(self):
-        """Test that skipping a task without explicit allow_skip (defaults to True) succeeds"""
-        task = TaskFactory(project=self.project)  # allow_skip defaults to True
-
-        self.client.force_authenticate(user=self.user)
-        response = self.client.post(
-            f'/api/tasks/{task.id}/annotations/', data={'result': [], 'was_cancelled': True}, format='json'
-        )
-
-        assert response.status_code == 201

web/libs/datamanager/src/sdk/lsf-sdk.js

@@ -805,10 +805,11 @@ export class LSFWrapper {
     // Manager roles that can force-skip unskippable tasks (OW=Owner, AD=Admin, MA=Manager)
     const MANAGER_ROLES = ["OW", "AD", "MA"];
     const task = this.task;
-    const taskAllowSkip = task?.allow_skip !== false;
+    const isEnterprise = window.APP_SETTINGS?.billing?.enterprise;
+    const skipDisabled = isEnterprise ? task?.allow_skip === false : false;
     const userRole = window.APP_SETTINGS?.user?.role;
     const hasForceSkipPermission = MANAGER_ROLES.includes(userRole);
-    const canSkip = taskAllowSkip || hasForceSkipPermission;
+    const canSkip = !skipDisabled || hasForceSkipPermission;
     if (!canSkip) {
       console.warn("Task cannot be skipped: allow_skip is false and user lacks manager role");
       this.showOperationToast(400, null, "This task cannot be skipped", { error: "Task cannot be skipped" });