Skip to content

[change] Switched third-party JSONField to built-in JSONField #673#687

Open
emohk wants to merge 4 commits intoopenwisp:masterfrom
emohk:issues/673-use-built-in-jsonfield
Open

[change] Switched third-party JSONField to built-in JSONField #673#687
emohk wants to merge 4 commits intoopenwisp:masterfrom
emohk:issues/673-use-built-in-jsonfield

Conversation

@emohk
Copy link
Copy Markdown

@emohk emohk commented Jul 8, 2025

Checklist

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

Reference to Existing Issue

Closes #673 .

Description of Changes

Replaced all uses of third-party with Django’s built-in and added corresponding migrations. Django applies the type conversion automatically using the appropriate SQL command, ensuring that existing data remains safe.

@emohk emohk force-pushed the issues/673-use-built-in-jsonfield branch from c6f5410 to b62f072 Compare July 8, 2025 19:30
@emohk emohk force-pushed the issues/673-use-built-in-jsonfield branch from b62f072 to 3975f34 Compare July 18, 2025 09:55
…p#673

Replaced all uses of third-party  with Django’s built-in  and added corresponding migrations.
Django applies the type conversion automatically using the appropriate SQL command, ensuring that existing data remains safe.

Fixes openwisp#673
@emohk emohk force-pushed the issues/673-use-built-in-jsonfield branch from 3975f34 to 8d7c713 Compare August 13, 2025 04:35
Copy link
Copy Markdown
Member

@nemesifier nemesifier left a comment

Choose a reason for hiding this comment

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

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Jan 31, 2026

Walkthrough

This change replaces the external jsonfield library's JSONField with Django's built-in models.JSONField across the codebase. Specifically, it updates the params field in the Check model and the main_tags and extra_tags fields in the Metric model. Corresponding Django migrations are added to alter these fields in the database schema. The change removes load_kwargs and dump_kwargs configurations from the field definitions, relying on Django's JSONField default behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

🚥 Pre-merge checks | ✅ 4
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: switching from third-party JSONField to Django's built-in JSONField, and references the related issue #673.
Description check ✅ Passed The description covers the main changes and references the linked issue, but the checklist shows that tests and documentation updates were not completed.
Linked Issues check ✅ Passed The PR fulfills the primary requirement from issue #673 by replacing third-party JSONField with Django's built-in JSONField across all models and adding migrations to preserve existing data.
Out of Scope Changes check ✅ Passed All changes are focused on replacing JSONField implementations with Django's built-in version and adding corresponding migrations; no unrelated changes are present.

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

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c9239b4 and f69ba09.

📒 Files selected for processing (6)
  • openwisp_monitoring/check/base/models.py
  • openwisp_monitoring/check/migrations/0012_alter_check_params.py
  • openwisp_monitoring/monitoring/base/models.py
  • openwisp_monitoring/monitoring/migrations/0013_alter_metric_extra_tags_alter_metric_main_tags.py
  • tests/openwisp2/sample_check/migrations/0004_alter_check_params.py
  • tests/openwisp2/sample_monitoring/migrations/0005_alter_metric_extra_tags_alter_metric_main_tags.py
🧰 Additional context used
🧬 Code graph analysis (4)
openwisp_monitoring/check/migrations/0012_alter_check_params.py (2)
tests/openwisp2/sample_check/migrations/0004_alter_check_params.py (1)
  • Migration (6-23)
tests/openwisp2/sample_monitoring/migrations/0005_alter_metric_extra_tags_alter_metric_main_tags.py (1)
  • Migration (6-25)
openwisp_monitoring/monitoring/migrations/0013_alter_metric_extra_tags_alter_metric_main_tags.py (1)
tests/openwisp2/sample_monitoring/migrations/0005_alter_metric_extra_tags_alter_metric_main_tags.py (1)
  • Migration (6-25)
tests/openwisp2/sample_monitoring/migrations/0005_alter_metric_extra_tags_alter_metric_main_tags.py (2)
openwisp_monitoring/monitoring/migrations/0013_alter_metric_extra_tags_alter_metric_main_tags.py (1)
  • Migration (6-25)
tests/openwisp2/sample_check/migrations/0004_alter_check_params.py (1)
  • Migration (6-23)
tests/openwisp2/sample_check/migrations/0004_alter_check_params.py (2)
openwisp_monitoring/check/migrations/0012_alter_check_params.py (1)
  • Migration (6-23)
tests/openwisp2/sample_monitoring/migrations/0005_alter_metric_extra_tags_alter_metric_main_tags.py (1)
  • Migration (6-25)
🪛 Ruff (0.14.14)
openwisp_monitoring/check/migrations/0012_alter_check_params.py

[warning] 8-10: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


[warning] 12-23: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

openwisp_monitoring/monitoring/migrations/0013_alter_metric_extra_tags_alter_metric_main_tags.py

[warning] 8-10: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


[warning] 12-25: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

tests/openwisp2/sample_monitoring/migrations/0005_alter_metric_extra_tags_alter_metric_main_tags.py

[warning] 8-10: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


[warning] 12-25: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

tests/openwisp2/sample_check/migrations/0004_alter_check_params.py

[warning] 8-10: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)


[warning] 12-23: Mutable class attributes should be annotated with typing.ClassVar

(RUF012)

🔇 Additional comments (6)
openwisp_monitoring/monitoring/base/models.py (1)

93-103: LGTM! Clean migration to Django's built-in JSONField.

The field definitions correctly use models.JSONField with appropriate options. Using default=dict (a callable) is the correct approach to avoid mutable default argument issues. The removal of load_kwargs/dump_kwargs is appropriate since Django's built-in JSONField handles JSON serialization automatically.

openwisp_monitoring/check/migrations/0012_alter_check_params.py (1)

1-23: LGTM! Migration correctly alters the params field.

The migration properly converts the params field to Django's built-in JSONField with matching field options from the model definition. The Ruff warnings about ClassVar annotations are false positives for Django migrations—this is the standard, expected pattern for migration class attributes.

openwisp_monitoring/monitoring/migrations/0013_alter_metric_extra_tags_alter_metric_main_tags.py (1)

1-25: LGTM! Migration correctly updates both JSONField columns.

The migration properly alters extra_tags and main_tags fields to use Django's built-in JSONField. Field options (blank, default, verbose_name, db_index) match the model definitions in monitoring/base/models.py. The Ruff warnings are false positives for standard Django migration patterns.

tests/openwisp2/sample_monitoring/migrations/0005_alter_metric_extra_tags_alter_metric_main_tags.py (1)

1-25: LGTM! Test migration mirrors the main migration correctly.

This sample/test app migration correctly mirrors the field changes from openwisp_monitoring/monitoring/migrations/0013_alter_metric_extra_tags_alter_metric_main_tags.py, ensuring consistency for swappable model testing.

openwisp_monitoring/check/base/models.py (1)

37-42: LGTM! Clean migration to Django's built-in JSONField.

The params field correctly uses models.JSONField with the same semantics as before. The field options match the corresponding migration in 0012_alter_check_params.py.

tests/openwisp2/sample_check/migrations/0004_alter_check_params.py (1)

1-23: LGTM! Test migration mirrors the main migration correctly.

This sample/test app migration correctly mirrors the field changes from openwisp_monitoring/check/migrations/0012_alter_check_params.py, ensuring consistency for swappable model testing.

✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.


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

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

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.

[change:monitoring] Replace thirdparty JSONField with Django built in JSONField

2 participants