Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
38 changes: 38 additions & 0 deletions openedx_filters/learning/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -1445,3 +1445,41 @@ def run_filter(cls, schedules: QuerySet) -> QuerySet | None:
"""
data = super().run_pipeline(schedules=schedules)
return data.get("schedules")


class AccountSettingsReadOnlyFieldsRequested(OpenEdxPublicFilter):
"""
Filter used to expand the set of read-only fields on the account settings API.

Purpose:
This filter is triggered when the account settings API validates which fields
may be updated for a given user. Pipeline steps may add field names to
``readonly_fields`` to mark those fields as read-only for the requesting user.

Filter Type:
org.openedx.learning.account.settings.read_only_fields.requested.v1

Trigger:
- Repository: openedx/edx-platform
- Path: openedx/core/djangoapps/user_api/accounts/api.py
- Function or Method: _validate_read_only_fields
"""

filter_type = "org.openedx.learning.account.settings.read_only_fields.requested.v1"

@classmethod
def run_filter(cls, readonly_fields: set, user: Any) -> set:
"""
Process the readonly_fields set using the configured pipeline steps.

Arguments:
readonly_fields (set): the set of field names the caller considers read-only.
Pipeline steps add field names to this set to mark additional fields as
read-only.
user (User): the Django User whose account settings are being updated.

Returns:
set: the (possibly expanded) set of read-only field names.
"""
data = super().run_pipeline(readonly_fields=readonly_fields, user=user)
return data.get("readonly_fields", set())
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It is customary for filters to return the complete list of inputs that where passed to the filter.

I understand the intent, it would be a strange use case to have a filter such as AccountSettingsReadOnlyFieldsRequested return a different user object, but this is breaking the stablished pattern.

The filter should return something like:

        return (
            data.get("readonly_fields"),
            data.get("user"),
        )

On the other hand, you don't need to return an empty set() as the fallback. The only way for this to happen is for your implemented step to remove the key "readonly_fields" but that would only happen if the step is broken or if by design it is trying to break the pipeline and trigger some try-catch block. Returning a default here would interfere with the accepted mechanism.

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.

31 changes: 31 additions & 0 deletions openedx_filters/learning/tests/test_filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from django.test import TestCase

from openedx_filters.learning.filters import (
AccountSettingsReadOnlyFieldsRequested,
AccountSettingsRenderStarted,
CertificateCreationRequested,
CertificateRenderStarted,
Expand Down Expand Up @@ -801,3 +802,33 @@ def test_schedule_requested(self):
result = ScheduleQuerySetRequested.run_filter(schedules)

self.assertEqual(schedules, result)


class TestAccountSettingsReadOnlyFieldsRequestedFilter(TestCase):
"""
Tests for the AccountSettingsReadOnlyFieldsRequested filter.
"""

def test_run_filter_returns_empty_set_unchanged_when_no_pipeline(self):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Most tests for filters are really easy because they only test that whatever you pass in them is returned back when the step pipeline is empty. That should be enough when you take the filter definition changes into account.

"""
When no pipeline steps are configured, run_filter returns the original readonly_fields.
"""
readonly_fields = set()
user = Mock()

with patch.object(
AccountSettingsReadOnlyFieldsRequested,
"run_pipeline",
return_value={"readonly_fields": readonly_fields},
):
result = AccountSettingsReadOnlyFieldsRequested.run_filter(
readonly_fields=readonly_fields, user=user
)

self.assertEqual(result, readonly_fields)

def test_filter_type(self):
self.assertEqual(
AccountSettingsReadOnlyFieldsRequested.filter_type,
"org.openedx.learning.account.settings.read_only_fields.requested.v1",
)