fix: preserve custom field attributes during JSON import

[huongcodecb1992]

No description provided.

[rilwis]

Có cơ chế nào để kiểm tra được danh sách các key này là chính xác? Ko bị thừa, thiếu?

[huongcodecb1992]

List này được xây dựng bằng cách đọc code và trả lời câu hỏi: key nào trong $settings mà không có unparse_* method nào đọc qua $this->key?

Những key đó sẽ không bao giờ được auto-track → phải khai báo tường minh.

Ví dụ như:

fields, tab, validation, multiple, callback, columns... — không có method nào isset($this->fields) hay $this->tab → không được track → cần list
type, std, options — được access trong unparse_choice* → đã auto-track → thừa trong list nhưng không hại

Trong codebase: src/Parsers/Field.php — Parser là chiều ngược lại của Unparser, nó đọc và xử lý tất cả native keys. Nếu key nào Parser có handler nhưng Unparser chưa có → key đó chưa được auto-track → cần vào STRUCTURAL_KEYS. Hiện e đã lấy hết các key từ parser sang.

[rilwis]

E đã check phần code xử lý 1 số field trong phần Extensions chưa?

[huongcodecb1992]

E check rồi. a thử kiểm tra xem có còn thiếu field nào nữa không ạ

[rilwis]

VD key desc, input_desc chẳng hạn thì thế nào e?

[opencode-agent]

Based on my review of the PR, here's my analysis:

Code Review

Issues Found

1. Missing Keys in STRUCTURAL_KEYS (Medium Priority)

Location: src/Unparsers/Field.php:21-27

The Parser (src/Parsers/Field.php) uses several keys not present in STRUCTURAL_KEYS:

• desc, label_description - Set in unparse_default_values()
• datalist_choices - Set in unparse_datalist() at line 71
• inline - Used in parse_object_field() (line 128)
• flatten, timestamp, force_delete, raw, alpha_channel - Used in parser but not unparser
• collapsible, select_all_none, field_type - Used in field type parsers

Without these keys, they'll be incorrectly moved to custom_settings.

2. No Test Coverage (High Priority)

No tests exist for this functionality. This makes verification and regression detection impossible.

3. Unused Variable (Low Priority)

Location: src/Unparsers/Field.php:239

$custom = $this->custom_settings ?? [];

This variable is declared but only used for null coalescing check. It appears leftover from earlier implementation.

4. Performance - O(n) Array Merge on Each Access (Low Priority)

Location: src/Unparsers/Field.php:238

$known = array_merge( $this->get_accessed_keys(), self::STRUCTURAL_KEYS );

This runs on every unparse call, but since both arrays are small, it's acceptable.

Positive Observations

1. Elegant auto-tracking - Using __get, __set, __isset magic methods to track accessed keys is a clean approach
2. Correct ordering - Field-type handler runs before unparse_custom_settings() ensures proper tracking
3. Type safety - Proper PHPDoc and array type declarations added in SettingsTrait.php

Recommendations

1. Add missing keys to STRUCTURAL_KEYS:

private const STRUCTURAL_KEYS = [
    // existing keys...
    'desc', 'label_description', 'datalist_choices', 
    'inline', 'collapsible', 'select_all_none', 'field_type',
];

2. Add integration tests to verify custom settings are preserved across parse/unparse cycles

3. Remove or clarify the unused $custom variable at line 239

4. Document why limit and limit_type are explicitly tracked despite being unset in unparse_text_limiter()

opencode session  |  github run