Page MenuHomePhabricator

SuggestedEditsSchema uses unsupported nested defaults
Closed, ResolvedPublic

Description

JsonSchema::DEFAULT is documented to be "only supported on the root-level. Defaults for objects need to be defined at their root level". This decision was made, because nested defaults are trickier to support, and can be easily self-contradictory (nested and root-level default might differ and provide arbitrarily many different default values, and such a conflict would need to be resolved in a predictable way).

However, Growth's SuggestedEditsSchema violates this rule, and attempts to use defaults in referenced items. For this reason, the defaults do not work as intended, and are not applied at all. This should be fixed by making SuggestedEditsSchema use defaults at the root level, as documented by CommunityConfiguration.

Event Timeline

Change #1035380 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] SuggestedEditsSchema: Define defaults at the root level

https://gerrit.wikimedia.org/r/1035380

Quoting from the commit message of JsonSchemaBuilder: Accept arrays as defaults for TYPE_OBJECT by @Urbanecm_WMF

This is intentionally not accounting for "objects in objects", or "objects in arrays". For that, we might want to reconsider the "defaults only at the top level" approach, and instead use something like JsonSchemaTrait::getDefaultFromJsonSchema is doing for objects in objects. Since we do not need objects in objects now, this is left as a future improvement, to give enough time for the discussion.

The approach of JsonSchemaTrait::getDefaultFromJsonSchema is looking good enough to me, if it works. Implementing this functionality seems essential to me for properly preventing the underlying issue that triggered T365653: saving a schema with partial values for objects.

Change #1043774 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/CommunityConfiguration@master] JsonSchemaBuilder: Process nested defaults

https://gerrit.wikimedia.org/r/1043774

Change #1035380 abandoned by Urbanecm:

[mediawiki/extensions/GrowthExperiments@master] SuggestedEditsSchema: Define defaults at the root level

Reason:

in favour of I44131013b53d16627175f18410e18fcf6c272015

https://gerrit.wikimedia.org/r/1035380

Change #1046638 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] SuggestedEditsSchema: Add defaults for missing fields

https://gerrit.wikimedia.org/r/1046638

Change #1046638 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] SuggestedEditsSchema: Add defaults for missing fields

https://gerrit.wikimedia.org/r/1046638

Change #1043774 merged by jenkins-bot:

[mediawiki/extensions/CommunityConfiguration@master] JsonSchemaBuilder: Process nested defaults

https://gerrit.wikimedia.org/r/1043774