Skip to content

refactor: consolidate GitHub integration into dojo/github/ package#14766

Merged
mtesauro merged 1 commit into
DefectDojo:devfrom
Maffooch:github-cleanup
Apr 30, 2026
Merged

refactor: consolidate GitHub integration into dojo/github/ package#14766
mtesauro merged 1 commit into
DefectDojo:devfrom
Maffooch:github-cleanup

Conversation

@Maffooch
Copy link
Copy Markdown
Contributor

Summary

  • Consolidate scattered GitHub integration code (dojo/github.py, dojo/github_issue_link/, GitHub forms in dojo/forms.py, GitHub models in dojo/models.py, GitHub templates) into a single self-contained dojo/github/ package.
  • Match the canonical dojo/url/ reference layout from CLAUDE.md: __init__.py + admin.py + models.py + services.py + ui/{__init__,forms,urls,views}.py + templates/dojo/.
  • Extract the inline Github(api_key).get_user() credential check from new_github view into services.validate_github_credentials() per CLAUDE.md Phase 2 ("external integration calls belong in services.py").
  • Backward-compat re-exports left in dojo/models.py (5 GITHUB models) and dojo/forms.py (6 GITHUB forms) per the playbook. dojo/utils.py and dojo/urls.py updated to use the new canonical paths.
  • GitHub Enterprise OAuth env vars in settings.dist.py left in place (out of scope per task; conceptually belong with SSO).
  • No behavior change. No DB migrations (single dojo app preserves app_label).

Test plan

  • python manage.py check — System check identified no issues
  • python manage.py makemigrations --check --dry-run — No changes detected
  • python manage.py test unittests -k github — 29/29 tests pass
  • Backward-compat re-exports resolve from dojo.models and dojo.forms (model/form identity matches new canonical paths)
  • New canonical paths resolve: dojo.github.{models,services}, dojo.github.ui.{forms,urls}
  • Templates resolve from dojo/github/templates/dojo/{github,new_github,delete_github}.html
  • Admin registrations active for GITHUB_Conf, GITHUB_Issue, GITHUB_PKey, GITHUB_Clone, GITHUB_Details_Cache
  • ruff check clean on all touched files
  • Smoke-test in dev container with enable_github=True: /github, /github/add, /github/<id>/delete; create finding linked to GITHUB_PKey-configured product, push to GitHub, close, reopen

🤖 Generated with Claude Code

@Maffooch Maffooch requested a review from mtesauro as a code owner April 27, 2026 22:09
@Maffooch Maffooch changed the title Consolidate GitHub integration into dojo/github/ package refactor: consolidate GitHub integration into dojo/github/ package Apr 27, 2026
@github-actions github-actions Bot added the settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR label Apr 27, 2026
Collapse dojo/github.py and dojo/github_issue_link/ into a single
dojo/github/ package matching the canonical dojo/url/ reference layout
described in CLAUDE.md (models, admin, services, ui/forms, ui/views,
ui/urls, templates). Backward-compat re-exports left in dojo/models.py
and dojo/forms.py. No behavior change, no migrations.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@Maffooch Maffooch added this to the 2.58.0 milestone Apr 27, 2026
Copy link
Copy Markdown
Contributor

@mtesauro mtesauro left a comment

Choose a reason for hiding this comment

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

Approved

@mtesauro mtesauro merged commit d0fcc0d into DefectDojo:dev Apr 30, 2026
157 checks passed
Maffooch pushed a commit to devGregA/django-DefectDojo that referenced this pull request Apr 30, 2026
Pulls in 15 upstream commits including:

* dojo/sso/ consolidation (DefectDojo#14765) — SSO settings/urls/views/templates/
  remote-user moved into a self-contained package.
* dojo/notifications/ consolidation (DefectDojo#14767) — notification helper +
  templates moved into the package, with a new context_processors split.
* dojo/github/ consolidation (DefectDojo#14766) — github_issue_link package
  renamed and reshaped under dojo/github/{models,services,ui,...}.
* test_tag_inheritance.py extension (DefectDojo#14771).
* Bulk-delete findings extension hook (DefectDojo#14740).
* Planned-remediation-version column alignment fix (DefectDojo#14773).
* Dependency bumps (datatables.net, gitpython, python-gitlab, pyopenssl,
  vulners, ruff, postcss).

Conflict resolutions worth flagging:

* dojo/forms.py — kept dev's reshuffled imports (GITHUB_* models now
  re-exported via dojo.github.ui.forms; Global_Role moved to
  dojo.models). Dropped the duplicate Global_Role import; the legacy
  authorization rewrite still imports from
  dojo.authorization.models for the rest.
* dojo/settings/settings.dist.py — kept tailwind's UIPreferenceLoader
  chain and APP_DIRS=False, but added a shared
  _DOJO_EXTRA_TEMPLATE_DIRS list referenced by both TEMPLATES[0]["DIRS"]
  and the FilesystemLoader so that dojo/sso/settings.py:apply_sso_settings
  can append the SSO template dir at startup and have it resolved at
  render time.
* dojo/templates/dojo/login.html — Tailwind tree, kept the inline
  Tailwind-styled SSO buttons rather than dev's
  {% include "dojo/sso_login_buttons.html" %} (which is Bootstrap-classic
  flavored and mounted by the SSO consolidation against the classic tree
  only).
* unittests/test_remote_user.py — adopted dev's import path
  (dojo.sso.remote_user, dojo.models.Dojo_Group_Member).
* dojo/api_v2/permissions.py — added a backward-compat shim
  re-exporting from dojo.authorization.api_permissions because the
  legacy authorization consolidation deleted the old module but
  dojo/notifications/api/views.py (new from dev) still imports from the
  old path.

Verified: ruff clean on touched files; manage.py check passes;
unittests.test_authorized_users_ui + unittests.authorization +
unittests.test_user_ui_timestamps + unittests.test_rest_framework +
unittests.test_remote_user all green (1144 tests, 542 skipped).
Maffooch pushed a commit to devGregA/django-DefectDojo that referenced this pull request May 8, 2026
Pulls in 15 upstream commits including:

* dojo/sso/ consolidation (DefectDojo#14765) — SSO settings/urls/views/templates/
  remote-user moved into a self-contained package.
* dojo/notifications/ consolidation (DefectDojo#14767) — notification helper +
  templates moved into the package, with a new context_processors split.
* dojo/github/ consolidation (DefectDojo#14766) — github_issue_link package
  renamed and reshaped under dojo/github/{models,services,ui,...}.
* test_tag_inheritance.py extension (DefectDojo#14771).
* Bulk-delete findings extension hook (DefectDojo#14740).
* Planned-remediation-version column alignment fix (DefectDojo#14773).
* Dependency bumps (datatables.net, gitpython, python-gitlab, pyopenssl,
  vulners, ruff, postcss).

Conflict resolutions worth flagging:

* dojo/forms.py — kept dev's reshuffled imports (GITHUB_* models now
  re-exported via dojo.github.ui.forms; Global_Role moved to
  dojo.models). Dropped the duplicate Global_Role import; the legacy
  authorization rewrite still imports from
  dojo.authorization.models for the rest.
* dojo/settings/settings.dist.py — kept tailwind's UIPreferenceLoader
  chain and APP_DIRS=False, but added a shared
  _DOJO_EXTRA_TEMPLATE_DIRS list referenced by both TEMPLATES[0]["DIRS"]
  and the FilesystemLoader so that dojo/sso/settings.py:apply_sso_settings
  can append the SSO template dir at startup and have it resolved at
  render time.
* dojo/templates/dojo/login.html — Tailwind tree, kept the inline
  Tailwind-styled SSO buttons rather than dev's
  {% include "dojo/sso_login_buttons.html" %} (which is Bootstrap-classic
  flavored and mounted by the SSO consolidation against the classic tree
  only).
* unittests/test_remote_user.py — adopted dev's import path
  (dojo.sso.remote_user, dojo.models.Dojo_Group_Member).
* dojo/api_v2/permissions.py — added a backward-compat shim
  re-exporting from dojo.authorization.api_permissions because the
  legacy authorization consolidation deleted the old module but
  dojo/notifications/api/views.py (new from dev) still imports from the
  old path.

Verified: ruff clean on touched files; manage.py check passes;
unittests.test_authorized_users_ui + unittests.authorization +
unittests.test_user_ui_timestamps + unittests.test_rest_framework +
unittests.test_remote_user all green (1144 tests, 542 skipped).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

settings_changes Needs changes to settings.py based on changes in settings.dist.py included in this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants