Skip to content

feat(parser): support TaggableManager relations - #63

Merged
FROWNINGdev merged 1 commit into
FROWNINGdev:mainfrom
Guflly:fix/taggable-manager-relations
Jul 31, 2026
Merged

feat(parser): support TaggableManager relations#63
FROWNINGdev merged 1 commit into
FROWNINGdev:mainfrom
Guflly:fix/taggable-manager-relations

Conversation

@Guflly

@Guflly Guflly commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Summary

  • recognize django-taggit TaggableManager as a many-to-many relation in both parsers
  • use taggit.Tag and taggit.TaggedItem defaults while preserving explicit through=
  • keep external tag targets out of ER output and update the changelog and golden snapshot

Type of change

  • Bug fix (non-breaking, restores expected behaviour)
  • Feature (non-breaking, adds a capability)
  • Breaking change (existing users have to update config or code)
  • Docs / README / comments only (no runtime effect)
  • Internal refactor (no behaviour change, no public API change)
  • Dependency bump
  • CI / build / tooling

Test plan

  • npm test — 111 passed
  • Python test suite — 464 passed, 2 skipped, 41 subtests passed
  • Ruff and Mypy passed

Checklist

  • I ran the full test suite locally (cd cli && pytest -q for Python, npm test for TypeScript) and it is green
  • I added or updated tests that cover the change (bugfixes should get a regression test)
  • If the change is user-facing I updated the CHANGELOG under ## [Unreleased]
  • If the change touches the MCP tool contract (new tool, new arg, new error code) I updated the tool description in mcp_server.py and the relevant tests in test_mcp_server.py
  • If this is a breaking change I called it out under ## Summary above and suggested a migration path

Related issues / discussions

Closes #50.

Summary by CodeRabbit

  • New Features

    • Added support for django-taggit's TaggableManager fields, now recognized and processed as many-to-many relations with automatic related model detection and configurable through model overrides.
  • Tests

    • Comprehensive test coverage added for TaggableManager field parsing, validating both default configurations and custom through model overrides across multiple output formats.

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 358fc409-a0dd-4ca7-9047-166d870824d1

📥 Commits

Reviewing files that changed from the base of the PR and between 748c2dd and ac07b05.

📒 Files selected for processing (7)
  • CHANGELOG.md
  • cli/django_orm_lens/parser.py
  • cli/tests/fixtures/golden/readthedocs.snapshot.json
  • cli/tests/test_aliased_module_and_third_party_fields.py
  • cli/tests/test_er_formats.py
  • src/parser.ts
  • test/taggable-manager.test.js

📝 Walkthrough

Walkthrough

Added TaggableManager support to the CLI and TypeScript parsers. Fields now produce many-to-many metadata targeting taggit.Tag, with default or explicit through models. Tests cover parsing, ER output, and the Read the Docs fixture.

Changes

TaggableManager support

Layer / File(s) Summary
TaggableManager relation parsing
cli/django_orm_lens/parser.py, src/parser.ts
Both parsers recognize TaggableManager as a many-to-many relation. They use taggit.Tag and default to taggit.TaggedItem. Explicit through values override the default.
Parser and ER validation
test/taggable-manager.test.js, cli/tests/test_aliased_module_and_third_party_fields.py, cli/tests/test_er_formats.py, cli/tests/fixtures/golden/readthedocs.snapshot.json, CHANGELOG.md
Tests validate default metadata, explicit through models, and omission of the external taggit.Tag target from ER formats. The golden fixture and changelog describe the new field support.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant DjangoModel
  participant Parser
  participant RelationMetadata
  participant ERFormatBuilder
  DjangoModel->>Parser: Provide TaggableManager field
  Parser->>RelationMetadata: Classify as many-to-many
  RelationMetadata-->>Parser: Set taggit.Tag and through model
  Parser->>ERFormatBuilder: Provide parsed schema
  ERFormatBuilder-->>Parser: Omit external taggit.Tag target
Loading

Possibly related PRs

Suggested reviewers: frowningdev

🚥 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 clearly describes the main change: adding support for TaggableManager relations in the parser. It is concise, specific, and directly related to the changeset.
Linked Issues check ✅ Passed The PR fully addresses all acceptance criteria from issue #50: TaggableManager fields are recognized as M2M relations, default to taggit.Tag and taggit.TaggedItem, explicit through= overrides defaults, tests are added, and ER diagram behavior is preserved.
Out of Scope Changes check ✅ Passed All changes are scoped to supporting TaggableManager relations. Modifications to parser.py, parser.ts, test files, changelog, and golden snapshot fixtures are directly related to the stated objectives. No unrelated changes are present.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@FROWNINGdev
FROWNINGdev merged commit bdcd7d9 into FROWNINGdev:main Jul 31, 2026
18 checks passed
@FROWNINGdev

Copy link
Copy Markdown
Owner

Merged — thank you, this was a pleasure to review.

Three things you did that I want to name, because they are the difference between a patch and a contribution:

You updated both parsers. The issue said nothing about src/parser.ts, and nothing in CI would have caught you leaving it alone — the parity test compares shapes, not field coverage. You went and found that the CLI and the extension have to answer identically, and you brought a JS test with you.

You branched from the current main. That sounds trivial and it is not: the parser changed three times in the twenty-four hours before you opened this, and a PR that sat on an older base would have needed a rebase against exactly the files you touched.

Your snapshot diff is an addition, not a rewrite. That is the tell I look for hardest on a fixture change. tags was missing from the Read the Docs parse entirely, so the golden snapshot gains one entry and alters none — the fixture is now evidence the feature works on real third-party code, rather than cover for something that broke.

One note for whoever reads this later: TaggableManager looks like an ordinary field and is not, and this is the shape of bug that a green test suite cannot see. The relation exists, the parser did not know the type, and the edge was silently dropped — every tagged model quietly showed one relation fewer than it has.

You are credited by name in the CHANGELOG and in the README roadmap. It ships in the next release.

#49 is the same family if you want another — django-mptt models are skipped outright, MPTTModel base plus TreeForeignKey. No obligation, and thanks again.

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.

Parser: support django-taggit TaggableManager as an M2M relation

2 participants