-
Notifications
You must be signed in to change notification settings - Fork 11
Add 'ignore' to graph_transition command #56
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
app = apps.get_app(field_spec[0]) | ||
models = apps.get_models(app) | ||
for model in models: | ||
app = apps.get_app_config(field_spec[0]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
apps.get_app was renamed decades ago 😅
613432b
to
d553b35
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Adds a new --exclude
(-e
) option to the graph_transitions
management command so users can omit named transitions from the generated graph.
- Introduce
--exclude
flag in argument parser and propagate it togenerate_dot
- Update
generate_dot
to skip transitions listed inexclude
- Refactor tests to loop over models and add a
test_exclude
, plus document the flag in README
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
tests/testapp/tests/test_graph_transitions.py | Refactored existing tests to iterate over multiple models and added test_exclude |
django_fsm/management/commands/graph_transitions.py | Added --exclude argument, updated generate_dot signature and handler call |
README.md | Added example usage for the -e flag to exclude transitions |
Comments suppressed due to low confidence (2)
django_fsm/management/commands/graph_transitions.py:144
- The help text for the
--exclude
flag could be clearer by indicating that multiple transition names should be comma-separated and that whitespace will be trimmed. For example:help="Comma-separated list of transition names to ignore (whitespace will be trimmed)."
parser.add_argument(
tests/testapp/tests/test_graph_transitions.py:26
- The
test_exclude
test currently only invokes the command without verifying its effect. Consider capturing the command output and asserting that the specified transitions are indeed omitted from the generated graph data.
call_command("graph_transitions", "-e", "standard,no_target", model)
fields_data += all_fsm_fields_data(model) | ||
dotdata = generate_dot(fields_data) | ||
|
||
dotdata = generate_dot(fields_data, ignore_transitions=options["exclude"].split(",")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Parsing the exclude option with raw split may include empty strings and leading/trailing whitespace, causing mismatches. Consider using options['exclude'].split(',') if options['exclude'] else []
and stripping each entry (e.g., [name.strip() for name in ... if name.strip()]
) before passing to generate_dot
.
dotdata = generate_dot(fields_data, ignore_transitions=options["exclude"].split(",")) | |
exclude_transitions = [name.strip() for name in options["exclude"].split(",") if name.strip()] if options["exclude"] else [] | |
dotdata = generate_dot(fields_data, ignore_transitions=exclude_transitions) |
Copilot uses AI. Check for mistakes.
d553b35
to
78e1b61
Compare
Fixes #55
Add a new argument to ignore specific transitions from graph