[#1006][FIX] Add type validation in custom attribute parser to preven…

[ParadiesSau]

Description

This PR fixes a critical 500 Internal Server Error when saving or updating Custom Attributes via the UI, as reported in #1006.

Changes

• Added type validation in validate_attribute within app/datamgmt/manage/manage_attribute_db.py.
• The code now ensures that both the "tab" and the "field" are dictionaries before attempting to call .get().
• Improved error logging: Instead of crashing, the function now returns descriptive error messages in the logs list if the JSON structure is invalid.
• Refactored logic to use a private variable _field_content for better readability and compliance with project coding rules.

Verification Results

• Tested with flat JSON structures (no tabs).
• Tested with standard tabbed JSON structures.
• Verified that invalid JSON inputs now return a UI-friendly error message instead of a 500 crash.

Fixes #1006

Summary by CodeRabbit

• New Features

  • Added support for new attribute field types: raw and html.

• Bug Fixes

  • Enhanced validation of attribute data structures with improved error reporting to prevent invalid configurations.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Bug fix in validate_attribute function adding structural type validations and guards. Introduces dictionary type checks for tab and field containers, validates mandatory fields, adds support for new field types ("raw" and "html"), and improves error diagnostics with clearer messages referencing actual data types.

Changes

Cohort / File(s)

Summary

Attribute Validation Type Guards source/app/datamgmt/manage/manage_attribute_db.py

Added type validation to ensure tab and field values are dictionaries before calling .get() methods; prevents AttributeError when malformed structures are encountered. Replaced direct data access with validated _field_content container. Expanded validation to support new field types ("raw", "html") with mandatory "value" checks. Updated error messages to use actual type names via __name__ for clarity.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A carrot of validation, so crisp and so bright,
No more AttributeError in the dead of night!
Dictionaries guarded, with structural care,
Raw fields and HTML now dancing in air. ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title is directly related to the main change: adding type validation to prevent the 'str' object has no attribute 'get' error from issue #1006.
Linked Issues check	✅ Passed	The pull request meets all coding requirements from issue #1006: adds type validation for tab/field dictionaries, handles invalid JSON structures gracefully, and returns descriptive error messages instead of crashing.
Out of Scope Changes check	✅ Passed	All changes are scoped to validate_attribute() in manage_attribute_db.py to fix the AttributeError; no unrelated modifications outside the target function.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

source/app/datamgmt/manage/manage_attribute_db.py (1)

297-303: Duplicate elif block — lines 301-303 are unreachable dead code.

There are two identical elif field_type in ['raw', 'html']: conditions. The second block (lines 301-303) will never execute. Additionally, lines 301-303 still use the old data[tab][field] pattern instead of _field_content, indicating this is likely a copy-paste artifact from the refactoring.

Remove the duplicate block:

🐛 Proposed fix

             elif field_type in ['raw', 'html']:
                 if _field_content.get('value') is None:
                     logs.append(f'{tab} -> {field} of type {field_type} is missing mandatory "value" tag')
 
-            elif field_type in ['raw', 'html']:
-                if data[tab][field].get('value') is None:
-                    logs.append(f'{tab} -> {field} of type {field_type} is missing mandatory "value" tag')
-
             else:
                 logs.append(f'{tab} -> {field}, unknown field type "{field_type}"')

🤖 Fix all issues with AI agents

In @source/app/datamgmt/manage/manage_attribute_db.py:
- Around line 288-290: The error message inside the options type check uses the
wrong variable; when validating if not isinstance(_field_content.get('options'),
list) update the logs.append call in that branch to reference the actual options
value (use type(_field_content.get('options')).__name__) so the message reads
that the field expects a list but got the type of options, keeping the existing
context variables field and field_type.

📜 Review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a4bfeda and ec849bc.

📒 Files selected for processing (1)

• source/app/datamgmt/manage/manage_attribute_db.py

🔇 Additional comments (2)

source/app/datamgmt/manage/manage_attribute_db.py (2)

241-245: LGTM! Good defensive check for tab structure.

This correctly guards against the issue reported in #1006 where .get() was called on a non-dict value.

---

247-253: LGTM! Proper field validation with clear variable naming.

Using _field_content improves readability and the type check prevents the AttributeError that caused the 500 error.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Bug: Wrong variable used in error message for options type check.

Line 290 reports type(_field_content.get("value")) but the check on line 288 is validating _field_content.get('options'). The error message should reference options, not value.

🐛 Proposed fix

                     if not isinstance(_field_content.get('options'), list):
                         logs.append(f'{tab} -> {field} of type {field_type} expects a value of type list, '
-                                    f'but got {type(_field_content.get("value")).__name__}')
+                                    f'but got {type(_field_content.get("options")).__name__}')

🤖 Prompt for AI Agents

In @source/app/datamgmt/manage/manage_attribute_db.py around lines 288 - 290,
The error message inside the options type check uses the wrong variable; when
validating if not isinstance(_field_content.get('options'), list) update the
logs.append call in that branch to reference the actual options value (use
type(_field_content.get('options')).__name__) so the message reads that the
field expects a list but got the type of options, keeping the existing context
variables field and field_type.