Skip to content
14 changes: 14 additions & 0 deletions backend/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,20 @@ def sample_problem(db):
problem.premises.add(premise)
return problem

@pytest.fixture
def hidden_problem(db):
"""Creates a hidden problem for testing."""
hypothesis = Sentence.objects.create(text="This is a hidden hypothesis.")
premise = Sentence.objects.create(text="This is a hidden premise.")
problem = Problem.objects.create(
dataset=Problem.Dataset.USER,
hypothesis=hypothesis,
entailment_label=Problem.EntailmentLabel.NEUTRAL,
extra_data={},
hidden=True,
)
problem.premises.add(premise)
return problem

@pytest.fixture
def annotator_session(db, annotator):
Expand Down
18 changes: 18 additions & 0 deletions backend/problem/migrations/0009_problem_hidden.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 4.2.27 on 2026-05-13 07:30

from django.db import migrations, models


class Migration(migrations.Migration):

dependencies = [
("problem", "0008_delete_knowledgebase"),
]

operations = [
migrations.AddField(
model_name="problem",
name="hidden",
field=models.BooleanField(default=False),
),
]
2 changes: 2 additions & 0 deletions backend/problem/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ class EntailmentLabel(models.TextChoices):
default=EntailmentLabel.UNKNOWN,
)

hidden = models.BooleanField(default=False)

extra_data = models.JSONField()

class Meta:
Expand Down
12 changes: 9 additions & 3 deletions backend/problem/problem_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from langpro_annotator.logger import logger
from problem.models import Problem
from user.models import User


@dataclass
Expand Down Expand Up @@ -51,7 +52,7 @@ def get_related_problem_ids(
)


def get_filters(query_params: QueryDict) -> Q | None:
def get_filters(query_params: QueryDict, user: User | None = None) -> Q | None:
"""
Constructs a Django Q object for filtering problems based on query parameters.
Return None if no valid filters are found in the parameters.
Expand All @@ -60,9 +61,9 @@ def get_filters(query_params: QueryDict) -> Q | None:
entailment_label = query_params.get("entailmentLabel")
gold = query_params.get("gold")
text = query_params.get("text")
hidden = query_params.get("hidden", None)

if not (dataset or entailment_label or gold or text):
return None
user_can_see_hidden = user.can_see_hidden_problems if user else False

filters = Q()
if dataset:
Expand All @@ -77,4 +78,9 @@ def get_filters(query_params: QueryDict) -> Q | None:
Q(hypothesis__text__icontains=text) | Q(premises__text__icontains=text)
)

if not user_can_see_hidden:
filters &= Q(hidden=False)
elif hidden and hidden.lower() in ('true', 'false'):
filters &= Q(hidden=hidden.lower() == 'true')

return filters
3 changes: 2 additions & 1 deletion backend/problem/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class Meta:
"entailmentLabel",
"extraData",
"base",
"hidden",
]

def get_premises(self, problem: Problem):
Expand Down Expand Up @@ -181,7 +182,7 @@ def update(self, instance: Problem, validated_data: dict) -> Problem:
return instance

return self._update_core_problem_fields(instance, validated_data)

def _update_core_problem_fields(self, instance: Problem, validated_data: dict) -> Problem:
"""
Updates core Problem fields (premises, hypothesis, base) from validated
Expand Down
28 changes: 27 additions & 1 deletion backend/problem/views/problem.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from rest_framework.status import (
HTTP_201_CREATED,
HTTP_200_OK,
HTTP_400_BAD_REQUEST,
HTTP_401_UNAUTHORIZED,
HTTP_403_FORBIDDEN,
)
Expand Down Expand Up @@ -39,6 +40,11 @@ def has_permission(self, request, view):
)


class ChangeProblemVisibilityPermission(IsAuthenticated):
def has_permission(self, request, view):
return super().has_permission(request, view) and request.user.can_change_problem_visibility


class ProblemView(ModelViewSet):
queryset = Problem.objects.all()
serializer_class = ProblemSerializer
Expand All @@ -48,6 +54,8 @@ def get_permissions(self):
return [CreateProblemPermission()]
if self.action == "partial_update":
return [EditProblemPermission()]
if self.action == "set_visibility":
return [ChangeProblemVisibilityPermission()]
return [IsAuthenticatedOrReadOnly()]

def list(self, request: Request) -> Response:
Expand All @@ -71,6 +79,23 @@ def first(self, request: Request) -> Response:
"""
return self._get_problem_response(request, pk=None)

@action(detail=True, methods=["post"], url_path="set-visibility")
def set_visibility(self, request: Request, pk: int) -> Response:
"""
Toggles the hidden status of a Problem.
Expects a JSON body with a boolean 'hidden' field.
"""
problem = get_object_or_404(Problem, id=pk)
hidden = request.data.get("hidden")
if not isinstance(hidden, bool):
return Response(
{"detail": "'hidden' must be a boolean."},
status=HTTP_400_BAD_REQUEST,
)
problem.hidden = hidden
problem.save(update_fields=["hidden"])
return Response({"hidden": problem.hidden}, status=HTTP_200_OK)

def retrieve(self, request: Request, pk: int | None = None) -> Response:
"""
Retrieves the requested Problem by ID.
Expand All @@ -82,7 +107,8 @@ def _get_problem_response(self, request: Request, pk: int | None) -> Response:
Helper method to build the problem response.
If pk is provided, retrieves that problem; otherwise returns the first problem.
"""
filters = get_filters(request.query_params)
user = request.user if request.user.is_authenticated else None
filters = get_filters(request.query_params, user)

qs = self.get_queryset()

Expand Down
93 changes: 93 additions & 0 deletions backend/problem/views/problem_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,42 @@ def test_master_annotator_can_retrieve_problem(
response = client.get(f"/api/problem/{sample_problem.id}/")
assert response.status_code == status.HTTP_200_OK

# Retrieve hidden problems

def test_user_without_permission_cannot_see_hidden_problem(
self, client, annotator, hidden_problem, sample_problem
):
"""Unauthenticated users should not be able to see hidden problems."""
client.force_login(user=annotator)
response = client.get(f"/api/problem/{hidden_problem.id}/")
assert response.status_code == status.HTTP_200_OK
assert response.data['problem']['id'] == sample_problem.id, "Unauthenticated users requesting a hidden problem should receive the first non-hidden problem instead."
Comment on lines +91 to +93
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I find this strange. Asking for a problem with a particular ID, which you're not allowed to see, and then getting another problem with a different ID instead. That will look like a bug to legitimate non-masters. Why not just return a 404?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The reasoning behind this is that this view is also used by users who change their filters in such a way that the problem they are currently viewing is excluded. It feels a bit heavy-handed to hit them with a 404, but I'm happy to be convinced otherwise.

Maybe we can return a 404 if there are no filters applied (since the user will then be looking for a specific problem, e.g. by manipulating the URL) anyway, and return the first available problem (as it is) otherwise?

A separate suggestion would be to show a small toast/pop-up message when a different problem is returned, so it is more obvious to the user that they have been 'redirected' to the first available problem, not the problem they requested.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The justification you're giving sounds like a list endpoint, where the query might return zero or more problems. In that case, if the list was filtered by a particular ID, I would expect it to return an empty list if the user cannot see the problem with that ID. Only if there was no ID-based filter criterion, would I expect that the user might get other results than the one they had in mind initially.

The test code I'm reading, on the other hand, seems to suggest that this is an endpoint that is supposed to return exactly one problem. If that is the case, the problem either exists or it doesn't (from the point of view of the user). In this case, where the user is not allowed to see the problem in question, there are two ways in which you can present this: "it exists but you're not allowed to see it" (403) or (I'm going to pretend) "it doesn't exist" (404).

Letting a user select one particular ID, like in this test, and then showing a problem with a different ID is never correct, as far as I'm concerned.

Perhaps the frontend is using this endpoint in a way that I'm not aware of? We can discuss this tomorrow or the day after if necessary.


def test_visitor_cannot_see_hidden_problem(self, client, visitor, hidden_problem, sample_problem):
"""Visitors should not be able to see hidden problems."""
client.force_login(user=visitor)
response = client.get(f"/api/problem/{hidden_problem.id}/")
assert response.status_code == status.HTTP_200_OK
assert response.data['problem']['id'] == sample_problem.id, "Visitors requesting a hidden problem should receive the first non-hidden problem instead."

def test_annotator_cannot_see_hidden_problem(
self, client, annotator, hidden_problem, sample_problem
):
"""Annotators should not be able to see hidden problems."""
client.force_login(user=annotator)
response = client.get(f"/api/problem/{hidden_problem.id}/")
assert response.status_code == status.HTTP_200_OK
assert response.data['problem']['id'] == sample_problem.id, "Annotators requesting a hidden problem should receive the first non-hidden problem instead."

def test_master_annotator_can_see_hidden_problem(
self, client, master_annotator, hidden_problem
):
"""Master annotators should be able to see hidden problems."""
client.force_login(user=master_annotator)
response = client.get(f"/api/problem/{hidden_problem.id}/")
assert response.status_code == status.HTTP_200_OK
assert response.data['problem']['id'] == hidden_problem.id, "Master annotators requesting a hidden problem should receive that hidden problem."

# Create

def test_unauthenticated_user_cannot_create_problem(
Expand Down Expand Up @@ -237,6 +273,63 @@ def test_master_annotator_cannot_update_non_user_problem(
assert sample_problem.hypothesis.text == old_hypothesis
assert sample_problem.premises.first().text == old_premise

# Update hidden status

def test_unauthenticated_user_cannot_update_problem_visibility(
self, client, sample_problem
):
"""Unauthenticated users should not be able to update problem visibility."""
response = client.post(
f"/api/problem/{sample_problem.id}/set-visibility/",
{"hidden": True},
content_type="application/json",
)
assert response.status_code == status.HTTP_401_UNAUTHORIZED
sample_problem.refresh_from_db()
assert sample_problem.hidden is False

def test_visitor_cannot_update_problem_visibility(
self, client, visitor, sample_problem
):
"""Visitors should not be able to update problem visibility."""
client.force_login(user=visitor)
response = client.post(
f"/api/problem/{sample_problem.id}/set-visibility/",
{"hidden": True},
content_type="application/json",
)
assert response.status_code == status.HTTP_403_FORBIDDEN
sample_problem.refresh_from_db()
assert sample_problem.hidden is False

def test_annotator_cannot_update_problem_visibility(
self, client, annotator, sample_problem
):
"""Annotators should not be able to update problem visibility."""
client.force_login(user=annotator)
response = client.post(
f"/api/problem/{sample_problem.id}/set-visibility/",
{"hidden": True},
content_type="application/json",
)
assert response.status_code == status.HTTP_403_FORBIDDEN
sample_problem.refresh_from_db()
assert sample_problem.hidden is False

def test_master_annotator_can_update_problem_visibility(
self, client, master_annotator, sample_problem
):
"""Master annotators should be able to update problem visibility."""
client.force_login(user=master_annotator)
response = client.post(
f"/api/problem/{sample_problem.id}/set-visibility/",
{"hidden": True},
content_type="application/json",
)
assert response.status_code == status.HTTP_200_OK
sample_problem.refresh_from_db()
assert sample_problem.hidden is True

# Update KB annotations

def test_unauthenticated_user_cannot_update_kb_annotations(
Expand Down
14 changes: 14 additions & 0 deletions backend/user/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,20 @@ def can_copy_problem(self) -> bool:
"""
return self.has_perm("problem.copy_problems")

@property
def can_see_hidden_problems(self) -> bool:
"""
Determines whether the user can see hidden problems.
"""
return self.has_perm("problem.view_hidden_problems")

@property
def can_change_problem_visibility(self) -> bool:
"""
Determines whether the user can change problem visibility (hidden status).
"""
return self.has_perm("problem.change_problem_visibility")

@property
def can_edit_kb(self) -> bool:
"""
Expand Down
8 changes: 7 additions & 1 deletion backend/user/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,13 @@ class CustomUserDetailsSerializer(UserDetailsSerializer):
read_only=True, source="can_create_problem"
)
canEditKb = serializers.BooleanField(read_only=True, source="can_edit_kb")
canAddLabelAnnotations = serializers.BooleanField(read_only=True, source="can_add_label_annotations")
canAddLabelAnnotations = serializers.BooleanField(
read_only=True, source="can_add_label_annotations"
)
canCopyProblem = serializers.BooleanField(read_only=True, source="can_copy_problem")
canChangeProblemVisibility = serializers.BooleanField(
read_only=True, source="can_change_problem_visibility"
)

class Meta(UserDetailsSerializer.Meta):

Expand All @@ -30,5 +35,6 @@ class Meta(UserDetailsSerializer.Meta):
"canEditKb",
"canAddLabelAnnotations",
"canCopyProblem",
"canChangeProblemVisibility",
)
read_only_fields = ["isStaff", "id", "email"]
1 change: 1 addition & 0 deletions backend/user/tests/test_user_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ def test_user_details(user_client, user_data):
"canEditKb": False,
"canCopyProblem": False,
"canAddLabelAnnotations": False,
"canChangeProblemVisibility": False,
}


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ describe("AnnotationInputComponent", () => {
const mockProblem: Problem = {
id: 123,
base: null,
hidden: false,
premises: ["First premise", "Second premise"],
hypothesis: "Test hypothesis",
entailmentLabel: EntailmentLabel.ENTAILMENT,
Expand Down Expand Up @@ -121,6 +122,7 @@ describe("AnnotationInputComponent", () => {
const mockProblem: Problem = {
id: 123,
base: null,
hidden: false,
premises: [],
hypothesis: "Empty test hypothesis",
entailmentLabel: EntailmentLabel.NEUTRAL,
Expand All @@ -143,6 +145,7 @@ describe("AnnotationInputComponent", () => {
const mockProblem: Problem = {
id: 1,
base: null,
hidden: false,
premises: ["Test premise"],
hypothesis: "Test hypothesis",
entailmentLabel: EntailmentLabel.CONTRADICTION,
Expand All @@ -166,6 +169,7 @@ describe("AnnotationInputComponent", () => {
const mockProblem: Problem = {
id: 12,
base: null,
hidden: false,
premises: [],
hypothesis: "",
entailmentLabel: EntailmentLabel.UNKNOWN,
Expand All @@ -187,6 +191,7 @@ describe("AnnotationInputComponent", () => {
const mockProblem: Problem = {
id: 17,
base: null,
hidden: false,
premises: [],
hypothesis: "",
entailmentLabel: EntailmentLabel.UNKNOWN,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,11 @@
@let appMode = appMode$ | async;
@let currentUser = currentUser$ | async;

@if (problemDetails(); as details) {
<div>
@if (currentUser?.canChangeProblemVisibility) {
<la-visibility-toggle [problem]="problem()" />
}
<table class="table table-sm table-borderless">
<tbody>
<tr>
Expand All @@ -25,7 +30,7 @@
<td>
<a [routerLink]="['/annotate', details.baseProblemId]">
<span>{{ details.baseProblemId }}</span>
<fa-icon [icon]="faArrowUpRight" class="ms-2"/>
<fa-icon [icon]="faArrowUpRight" class="ms-2" />
</a>
</td>
} @else if (details.baseProblemId === null) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const createMockProblem = (
): Problem => ({
id,
base: null,
hidden: false,
dataset,
entailmentLabel,
premises: ["premise"],
Expand Down
Loading
Loading