V8-archive: Global Settings errors: Attempting to write an array as the value for field "value" in "directus_settings"

[justrealmilk]

Bug Report

• Upon adjusting Thumbnail Quality Tags, Thumbnail Actions, in Global Settings: Attempting to write an array as the value for field "value" in "directus_settings"
• Upon removing a set Logo, This value should not be blank

Steps to Reproduce

• Attempt to adjust Thumbnail Quality Tags, Thumbnail Actions
• Attempt to remove set logo

Technical Details

Cloned master branch of directus/directus: c2a534e8e042a7ad1a1757cb7996b2c9fb12c863 (yesterday, April 23)

[theharshin]

@bjgajjar Same problem as logo field? Can we think of a better way to handle directus_settings? The key-value pair strategy will create a lot of problems for complex settings!

[bjgajjar]

Fixed in #909

[rijkvanzanten]

The key-value pair strategy will create a lot of problems for complex settings!

How so? The alternative in SQL would be to have a column per setting with only one row. That's the only way it can work the same as other collections, but that's not realistic with the large amount of options..

[benhaynes]

Even complex settings like these thumbnail options can be distilled down to simpler forms:

https://github.com/directus/api/issues/813

This would be a _big_ refactor... so let's keep discussing, but I'd say KVPs are what we have for now.

[theharshin]

By complex settings I mean that a single setting may contain sub-setting. Just like Thumbnail Options.
Right now for settings, we're using interfaces which are returning JSON, Array, String & Numbers.
All this format needs to be converted to a String to save into the respective column.
For now, we've handled those by converting them into String or CSV based on their datatype.

[benhaynes]

Right. What do you propose?

[theharshin]

In the current fix, we have applied the following logic for datatypes:

Array:
Convert to CSV while saving into db.
Convert CSV to Array in API response.

JSON:
Convert to String while saving.

I think this conditional conversion should be avoided. One thing we can do is always save the value in JSON format like this:

{
   "value":"<value_from_interface>"
}

We need to handle only JSON and the original value from the interface can be saved as it is. I don't know how practical this is 🤔 but just an idea.

[rijkvanzanten]

The API already has all of these conversions going on.. somewhere right? Regular collections already have this datacasting in place (esp between CSV-Array and TEXT-JSON).

[benhaynes]

If we update min MySQL version to 5.7 don't we remove the JSON->Text conversion?

[rijkvanzanten]

Yes, but that's beside the point. I'm saying that we shouldn't have to re-invent the wheel for these conversions as the API already has and uses these in other places