Skip to content

[16.0][ADD] field_management#391

Open
madara1150 wants to merge 30 commits into16.0from
16.0-field-manager
Open

[16.0][ADD] field_management#391
madara1150 wants to merge 30 commits into16.0from
16.0-field-manager

Conversation

@madara1150
Copy link
Copy Markdown
Collaborator

@madara1150 madara1150 commented Dec 25, 2025

@madara1150 madara1150 changed the title [ADD] readonly_management [16.0][ADD] readonly_management Dec 25, 2025
@madara1150 madara1150 changed the title [16.0][ADD] readonly_management [16.0][ADD] field_management Jan 13, 2026
@madara1150 madara1150 added the status/ready-for-review Status: Issue is up for grabs label Jan 14, 2026
@madara1150
Copy link
Copy Markdown
Collaborator Author

Code Review: field_management module

Bug 🐛

field_management/i18n/th.po:133 — Missing Thai translation for "Force Required"

msgid "Force Required"
msgstr ""   # ← ขาดคำแปล

"Force Readonly" แปลว่า "บังคับให้เป็นแบบอ่านอย่างเดียว" และ "Force Invisible" แปลว่า "บังคับให้ไม่เห็น" แต่ "Force Required" ยังว่างอยู่ ควรแปลเป็น "บังคับให้จำเป็น" หรือคล้ายกัน


Code Quality

1. Code Duplication สูง (field_management/models/)

readonly_management.py, invisible_management.py, required_management.py มีโครงสร้างเหมือนกันเกือบ 100% (fields เหมือนกันหมด: name, active, model_id, field_ids, model_name, apply_on_domain, note) เช่นเดียวกับ *_fields.py อีก 3 ไฟล์

แนะนำให้สร้าง abstract base model เพื่อลด duplication ~60%:

class FieldManagementBase(models.AbstractModel):
    _name = 'field.management.base'
    _description = 'Field Management Base'
    
    name = fields.Char('Name')
    active = fields.Boolean(default=True)
    model_id = fields.Many2one('ir.model', ...)
    model_name = fields.Char(related='model_id.model', store=True)
    apply_on_domain = fields.Char()
    note = fields.Text('Note')

2. Performance — 3 queries ต่อ form view load (base.py:117-119)

readonly_configs = self.env['readonly.management'].sudo().search(model_domain)
invisible_configs = self.env['invisible.management'].sudo().search(model_domain)
required_configs = self.env['required.management'].sudo().search(model_domain)

ทุกครั้งที่ user เปิด form view จะ query DB 3 ครั้ง ถ้า config ไม่เปลี่ยนบ่อย ควรพิจารณา cache ด้วย @tools.ormcache หรือ invalidate เมื่อ config เปลี่ยน

3. Redundant store=True on related field (readonly_management_fields.py:16-21)

model_id = fields.Many2one(
    'ir.model',
    related='management_id.model_id',
    store=True,  # ← redundant — ค่านี้มีอยู่แล้วผ่าน management_id
    readonly=False,
)

store=True สร้าง column ซ้ำใน DB ทั้ง 3 *_fields models ถ้าไม่ได้ใช้ใน search/filter ควรเอาออก


Positive ✅

  • Test coverage ดีมาก — 48+ test cases ครอบคลุม edge cases ได้ดี (invalid domain, inactive config, multi-config OR logic, fields absent from view)
  • Security ดี — ใช้ ast.literal_eval() กับ try-except ป้องกัน injection, ใช้ ORM ไม่มี raw SQL
  • Domain parsing logic (base.py) — เขียนได้ clean และ handle edge cases ดี เช่น force flag, OR/AND composition, invisible field injection

@madara1150 madara1150 force-pushed the 16.0-field-manager branch from 5af87ed to 6401f19 Compare April 7, 2026 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

status/ready-for-review Status: Issue is up for grabs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant