Skip to content

[fix] Preserved text -> jsonb migration for JSONField fields #722#724

Merged
nemesifier merged 1 commit into
masterfrom
issues/722-jsonfield-migration
May 20, 2026
Merged

[fix] Preserved text -> jsonb migration for JSONField fields #722#724
nemesifier merged 1 commit into
masterfrom
issues/722-jsonfield-migration

Conversation

@pandafy
Copy link
Copy Markdown
Member

@pandafy pandafy commented May 20, 2026

Checklist

  • I have read the OpenWISP Contributing Guidelines.
  • I have manually tested the changes proposed in this pull request.
  • N/A I have written new test cases for new code and/or updated existing tests for changes to existing code.
  • N/A I have updated the documentation.

Reference to Existing Issue

Closes #722

Please open a new issue if there isn't an existing issue yet.

Description of Changes

Historical migrations were updated to use models.TextField instead of models.JSONField for fields originally backed by the third-party jsonfield library.

The original jsonfield.fields.JSONField inherited from TextField and stored values as PostgreSQL text. Replacing historical migration definitions with models.JSONField caused Django to interpret the migration transition as JSONField -> JSONField, resulting in a no-op migration and preventing PostgreSQL columns from being converted to jsonb.

By preserving the historical field semantics as TextField, Django can correctly detect the transition from text -> jsonb and generate the required ALTER COLUMN ... TYPE jsonb migration SQL during upgrades.

Historical migrations were updated to use models.TextField instead
of models.JSONField for fields originally backed by the third-party
jsonfield library.

The original jsonfield.fields.JSONField inherited from TextField and
stored values as PostgreSQL text. Replacing historical migration
definitions with models.JSONField caused Django to interpret the
migration transition as JSONField -> JSONField, resulting in a no-op
migration and preventing PostgreSQL columns from being converted to
jsonb.

By preserving the historical field semantics as TextField, Django can
correctly detect the transition from text -> jsonb and generate the
required ALTER COLUMN ... TYPE jsonb migration SQL during upgrades.

Closes #722
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 20, 2026

Review Change Stack

📝 Walkthrough

Walkthrough

This PR reverts JSONField references back to TextField in four migration file locations. The changes address PostgreSQL schema migration detection by preserving the historical TextField type in migration files, allowing Django to correctly generate ALTER COLUMN ... TYPE jsonb statements when the ORM later expects jsonb fields. The conversions span both production migrations (sms_meta_data and user_credentials fields) and corresponding sample test app migrations.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related issues

  • #722 — Directly addresses the missing PostgreSQL text→jsonb migration issue and implements the suggested solution of preserving TextField in historical migrations instead of using JSONField.

Possibly related PRs

  • openwisp/openwisp-radius#619 — Introduced the JSONField conversions that this PR reverts; both PRs modify the same schema fields (sms_meta_data, user_credentials, PDF) across migrations.

Suggested labels

enhancement

Suggested reviewers

  • nemesifier

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

  • Ignore

❌ Failed checks (1 error)

Check name Status Explanation Resolution
Bug Fixes ❌ Error PR fixes root cause by preserving TextField in historical migrations, but lacks regression test verifying PostgreSQL column type conversion (text→jsonb) occurs. Add regression test: 1) Create DB with text columns (migrations 0007/0009), 2) Run migration 0043, 3) Verify columns are jsonb type (not text), 4) Confirm JSONField queries work.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title uses the correct [fix] prefix and clearly describes the change: preserving text->jsonb migration for JSONField fields, directly addressing issue #722.
Description check ✅ Passed The PR description covers the key sections: checklist completion, issue reference (#722), and detailed explanation of changes. Testing is marked as done and docs updates marked N/A appropriately.
Linked Issues check ✅ Passed The code changes directly address issue #722 by preserving TextField in historical migrations to enable Django's detection of text->jsonb transitions, generating required ALTER COLUMN SQL.
Out of Scope Changes check ✅ Passed All changes are scoped to historical migrations, updating field definitions from JSONField to TextField to fix the PostgreSQL text->jsonb conversion issue described in #722.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch issues/722-jsonfield-migration

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

@coderabbitai coderabbitai Bot added the enhancement New feature or request label May 20, 2026
@kilo-code-bot
Copy link
Copy Markdown

kilo-code-bot Bot commented May 20, 2026

Code Review Summary

Status: No Issues Found | Recommendation: Merge

Overview

This PR correctly fixes issue #722 by preserving the historical semantics of database migrations.

The Problem: The original jsonfield.fields.JSONField (third-party library) inherited from TextField and stored values as PostgreSQL text. When migrations were updated to use Django's native models.JSONField, Django interpreted this as JSONField -> JSONField (no-op), preventing the ALTER COLUMN ... TYPE jsonb migration from being generated.

The Fix: Changing historical migrations to use models.TextField preserves the original field semantics, allowing Django to correctly detect the transition from text -> jsonb during upgrades.

Files Reviewed (3 files):

  • openwisp_radius/migrations/0007_sms_verification.py - Line 49: sms_meta_data field changed from JSONField to TextField
  • openwisp_radius/migrations/0009_radbatch_user_credentials_field.py - Line 19: user_credentials field changed from JSONField to TextField
  • tests/openwisp2/sample_radius/migrations/0002_initial_openwisp_app.py - Lines 545, 697: Both fields updated accordingly

This is the correct approach for handling historical migrations in Django when transitioning from a third-party JSONField to Django's native JSONField.


Reviewed by kimi-k2.5 · 89,445 tokens

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 97.453%. remained the same — issues/722-jsonfield-migration into master

@github-project-automation github-project-automation Bot moved this from To do (general) to In progress in OpenWISP Contributor's Board May 20, 2026
@github-project-automation github-project-automation Bot moved this from In progress to Reviewer approved in OpenWISP Priorities for next releases May 20, 2026
@nemesifier nemesifier merged commit f162b91 into master May 20, 2026
22 checks passed
@nemesifier nemesifier deleted the issues/722-jsonfield-migration branch May 20, 2026 12:31
@github-project-automation github-project-automation Bot moved this from In progress to Done in OpenWISP Contributor's Board May 20, 2026
@github-project-automation github-project-automation Bot moved this from Reviewer approved to Done in OpenWISP Priorities for next releases May 20, 2026
@openwisp-companion
Copy link
Copy Markdown

Proposed change log entry:

[fix] Preserve text -> jsonb migration for JSONField fields #722

Updated historical migrations to use models.TextField instead of models.JSONField for fields originally backed by the third-party jsonfield library. This ensures that Django correctly detects the transition from text to jsonb, allowing PostgreSQL columns to be converted to jsonb during upgrades.

Closes #722

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Development

Successfully merging this pull request may close these issues.

[bug] Missing PostgreSQL text -> jsonb migration for JSONField replacement

3 participants