Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datatype configuration refactor #13605

Merged
merged 14 commits into from
Dec 21, 2022
Merged

Conversation

kjac
Copy link
Contributor

@kjac kjac commented Dec 21, 2022

Prerequisites

  • I have added steps to test this contribution in the description below

Description

In the current backoffice, the server is fully in charge of datatype configurations; every configuration field has to be explicitly defined serverside, and only those fields will be persisted to the datatype configuration - no matter what the client might attempt to save.

In the new backoffice, the server won't know anything about datatype configurations except what is necessary for validation purposes. For example, the server must know about the maxChars configuration for a textarea datatype in order to perform editor validation, but it should never know nor care about the rows configuation, because this is purely a clientside presentation configuration.

This PR is a complete rework of how datatype configuration is stored and handled, in order to facilitate the new backoffice requirements listed above.

This is not a clean-up of the current ways of handling datatype configuration. On the contrary, some work has been put in to retain the current datatype configuration handling, so datatype configuration continues to work (almost) within the current backoffice, and so the rest of the backoffice doesn't explode due to this datatype configuration rework - for example, content type configuration and content editing both rely on this backwards compatibility.

This is also not an implementation of datatype controllers for the new management API. However, in order to test the whole rework, rudimentary datatype controllers for the new management API have been added to this PR. These will be mended in a future PR, among other things to include:

  • Validation for configuration editors to validate their own configuration before it's persisted.
  • Splitting up property editors in dedicated serverside and clientside mappings to enable reuse of serverside configuration editors.

On a curious note, the current backoffice uses Newtonsoft.Json for (de)serialization and JSON handling while the new management API uses System.Text.Json. Therefore a temporary datatype configuration serializer has been put in place in an attempt to handle both simultaneously - the ContextualConfigurationEditorJsonSerializer.

Limitations of this PR

This PR performs a general clean-up of all known datatype configurations (see MigrateDataTypeConfigurations) to make up for years and years of inconsistency (since V7) in datatype configuration formatting. A few datatypes are also handled explicitly to either reformat or sanitize known configuration issues, while others remain unhandled. Specifically color pickers and list views are known to have issues at this point. We can't do anything about those datatypes now, because we first need to define their future configuration format in order to perform a configuration migration. This will be handled in a separate task, and the migration will be amended to handle all known datatypes. Thus at some point it will be necessary to re-test this PR in conjunction with the PR for this separate task.

The ContextualConfigurationEditorJsonSerializer almost works. Due to a caching issue, the current backoffice almost consequently breaks when updating a datatype using the new management API. A reboot of the site fixes this caching issue. Since it's a temporary workaround to have two configuration serializers, we won't put more work into trying to solve the caching issue at this point. It should resolve itself once we can get rid of Newtonsoft.Json.

Testing this PR

Get in touch with KJA for a copy of the datatype Postman collection for the new management API.

  1. Make sure you have at least a few "value list" based datatypes configured - i.e. checkbox lists, radiobutton lists or dropdowns.
  2. Make note of all of your current datatype configurations (or at least random samples).
  3. Run the upgrade, which migrates most of the known datatype configurations to a compatible format (see limitations above).
  4. Restart the site to clear out cache issues that might be caused by the new ContextualConfigurationEditorJsonSerializer.
  5. Verify that most of the datatypes are still configured as they were before the upgrade.
  6. Verify that you can edit the current datatypes - i.e. add items to the "value list" based datatypes.
  7. Verify that you can get and update datatypes using the new management API.
  8. Reboot the site and verify that those updates are also reflected in the current backoffice.
  9. Verify that you can update a datatype using the new management API with more configuation data than is currently used by the configuration editor (add extra items to the data collection).
    • Test with both simple values (integer, boolean, string) and with complex values (array, object).
  10. Reboot the site and verify that the updated datatypes from the previous step is still editable within the current backoffice.
    • NOTE: If you save them in the new backoffice, the additional configuration data will be stripped because of how the backwards compatibility works.

Bonus test

As it happens, neither the textbox nor the textarea datatypes perform serverside validation of the maxChars configuration. This is likely an oversight, but we can use it to test this PR.

In TextAreaPropertyEditor, remove the CreateValueEditor() implementation and then add the following to the class:

protected override IDataValueEditor CreateValueEditor()
{
    TextOnlyValueEditor valueEditor = DataValueEditorFactory.Create<TextOnlyValueEditor>(Attribute!);
    valueEditor.Validators.Add(new MaxCharsValidator());
    return valueEditor;
}

private class MaxCharsValidator : IValueValidator
{
    // TODO: localize and stuff
    public IEnumerable<ValidationResult> Validate(object? value, string? valueType, object? dataTypeConfiguration)
    {
        if (value == null)
        {
            return Array.Empty<ValidationResult>();
        }

        if (value is not string stringValue)
        {
            return new[] { new ValidationResult($"Expected a string value, got: {value?.GetType()}") };
        }

        var textAreaConfig = (TextAreaConfiguration?)dataTypeConfiguration;
        var maxChars = textAreaConfig?.MaxChars ?? 0;

        // cleverly cheating the clientside validation :D
        if (maxChars <= 10)
        {
            return Array.Empty<ValidationResult>();
        }
        maxChars -= 10;

        return stringValue.Length > maxChars
            ? new[] { new ValidationResult($"Fail... such fail! Way too much entered - max = {maxChars}, got = {stringValue.Length}") }
            : Array.Empty<ValidationResult>();
    }
}

Now perform the following steps:

  1. Create a textarea datatype with maxChars set to 20
    image
  2. Create a content type with a property based on this datatype.
  3. Create content based on this content type.
  4. Enter more than 10 characters in the textarea, save the content and verify that the serverside validation fails:
    image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants