feat: add AccountSettingsReadOnlyFieldsRequested filter

[pwnage101]

ENT-11510

---

Blocks:

• feat: add AccountSettingsReadOnlyFieldsStep pipeline step edx-enterprise#2544
• feat: replace enterprise_support import with AccountSettingsReadOnlyFieldsRequested filter openedx-platform#38098

[felipemontoya]

@pwnage101 I will try to review this afternoon. I know the big goal is to get rid of this import in the platform from openedx.features.enterprise_support.utils import get_enterprise_readonly_account_fields. Is there anything else about ENT-11510 that I should know?

[felipemontoya]

There are some non conventional behaviours that should be easy to correct.

[felipemontoya]

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.

[pwnage101]

I commented on the draft PRs pertaining to this specific convention.

• https://github.com/openedx/openedx-filters/pull/339/changes#r2983650901
• https://github.com/openedx/openedx-filters/pull/338/changes#r2983621303
• https://github.com/openedx/openedx-filters/pull/337/changes#r2983618427
• https://github.com/openedx/openedx-filters/pull/333/changes#r2983613910

[felipemontoya]

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.

[felipemontoya]

Minor fix worth addressing in the docstring.

I'll merge once this is udpated