Skip to content

Add basic JSON UI form field support (PP-4438)#3386

Merged
tdilauro merged 5 commits into
mainfrom
feature/json-formdata
May 26, 2026
Merged

Add basic JSON UI form field support (PP-4438)#3386
tdilauro merged 5 commits into
mainfrom
feature/json-formdata

Conversation

@tdilauro
Copy link
Copy Markdown
Contributor

Description

Adds a json form field type for integration settings. Fields annotated with FormFieldType.JSON display a textarea in the admin UI that accepts a JSON value. Two-way handling is wired up automatically:

  • Form rendering: the field's default value is serialized to a JSON string so the UI displays it correctly.
  • Form submission: the incoming string is parsed back to a Python value before Pydantic validation runs, with a clear error message if the input is not valid JSON.

Note: The ability to edit this field type depends on new support introduced on circulation-admin PR #224.

Motivation and Context

Integration settings that need to store structured data (objects, arrays) previously had no native form type. This adds a first-class JSON field type that makes the round-trip between the admin UI and the settings model transparent.

[Jira PP-4438]

How Has This Been Tested?

  • Manual testing in local development environment admin UI.
  • New/updated tests for the new functionality.
  • All tests and checks pass locally.
  • CI tests and checks pass.

Checklist

  • N/A - I have updated the documentation accordingly.
  • All new and existing tests passed.

@claude
Copy link
Copy Markdown
Contributor

claude Bot commented May 24, 2026

Claude finished @tdilauro's task in 3m 24s —— View job


Code Review

  • Gather context and understand the request
  • Read changed files
  • Analyze the code for bugs and issues
  • Post review findings

Summary

Clean, well-structured implementation. The two-part round-trip (form rendering and form submission parsing) is correctly implemented, and the last commit's fix to pass native Python values instead of calling get_form_value() is the right call — without it, boolean and integer defaults would have been wrapped in JSON strings ("true", "42") rather than emitted as proper JSON values (true, 42). The alias resolution logic in the JSON parsing loop mirrors what the existing empty-string-to-None loop already handles, and the comprehensive parametrized test suite covers all relevant cases. No bugs found.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.34%. Comparing base (5315bfb) to head (8826036).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3386   +/-   ##
=======================================
  Coverage   93.34%   93.34%           
=======================================
  Files         507      507           
  Lines       46434    46451   +17     
  Branches     6336     6341    +5     
=======================================
+ Hits        43345    43361   +16     
- Misses       1999     2000    +1     
  Partials     1090     1090           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tdilauro
Copy link
Copy Markdown
Contributor Author

I improved the coverage and addressed useful AI review feedback. This is ready for human review.

Copy link
Copy Markdown
Member

@jonathangreen jonathangreen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 looks great!

@tdilauro tdilauro merged commit bf1e9db into main May 26, 2026
21 checks passed
@tdilauro tdilauro deleted the feature/json-formdata branch May 26, 2026 22:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants