Closed jzwood closed 3 years ago
Overall looks good, but I'm not convinced about the formatting in the editor modal. I think we should remove escaping slashes as it gets a bit confusing, other than that I think is a really good improvement.
@comtom yeah, that's a great point -- I'm not a fan of how it looks either. So digging into it a little bit I was unable to craft a different representation that successfully updates.
Hopping down to psql
to illustrate:
CREATE TABLE json_array_table (components json[]);
\d json_array_table
#= Table "public.json_array_table"
Column | Type | Collation | Nullable | Default
------------+--------+-----------+----------+---------
components | json[] |
INSERT INTO json_array_table (components) VALUES ('{"{\"a\": 1}", "{\"b\": 2}"}');
SELECT * FROM json_array_table LIMIT 1;
components
-----------------------------
{"{\"a\": 1}","{\"b\": 2}"}
(1 row)
This indicates to me that the back-slashes are part of how the json[] is being represented so should be included in the modal editor. Let's try some alternatives and see if they work?
INSERT INTO json_array_table (components) VALUES ('{"{"a": 1}", "{"b": 2}"}'::json[]);
malformed array literal: "{"{"a": 1}", "{"b": 2}"}"
INSERT INTO json_array_table (components) VALUES ('{{"a": 1}, {"b": 2}}'::json[]);
malformed array literal: "{{"a": 1}, {"b": 2}}"
INSERT INTO json_array_table (components) VALUES ({{"a": 1}, {"b": 2}}::json[]);
syntax error at or near "{"
INSERT INTO json_array_table (components) VALUES ([{"a": 1}, {"b": 2}]::json[]);
syntax error at or near "{"
In summary, I think that while unsightly, the backslashed json-array is actually what we want here.
@comtom on reflection I think I may have misunderstood your suggestion. However Postgresql stores json[]
is unimportant as long as what the user interacts with makes sense to them. I updated my PR so that the editing modal shows pretty array json (IE without the back-slashes) but adds them in before updating the DB.
TL;DR it's prettier now and updating works as expected.
That was I hoping to achieve! Looks much better now. Will try to test tomorrow so we can merge. Thank you for this awesome contribution
Tested and works properly. @Paxa do you agree to merge this one? I don't see any drawbacks.
Looks good. Thank you guys
Before
According to psql
After
Updating with valid
jsonb[]
also works as expected.