Repository navigation
refactor(views): split core/views/__init__.py into 11 domain modules (H6) - #423
Merged
Merged
Conversation
First move of the core/views/__init__.py split (4315 lines, 78 symbols). Per the plan (#417), permissions.py goes first: it has no Django view surface and every later domain module that needs RBAC checks imports from it. Moves AuthenticatedUser, _get_property_role, is_owner_or_shared, and _is_client_only_user verbatim. None of the four has an external core.views.<name> reference (checked urls.py, all test patches), so __init__.py imports them for internal use only where a not-yet-moved view still calls them by bare name, and does not add them to __all__. Verified: manage.py check clean, ruff clean, 2186 passed / 4 failed (unit+integration) — the 4 failures are the pre-existing ATTOM live tests, unrelated (see #413), requiring a real API key this environment doesn't carry. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves home, health_check, system_status, refresh_all_sources, and health_json verbatim. All five are routed in core/urls.py, so all five are re-exported. Verified: manage.py check clean, ruff clean, 165 unit tests pass, 37 targeted health/system integration tests pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves investment_targets_edit, MarketRefreshView, markets_list, brrrr_calculator, and sell_index verbatim. All five routed in core/urls.py. Widens mypy.ini's [mypy-core.views] ignore_errors suppression with a matching [mypy-core.views.*]. mypy config sections match the exact module path only, not submodules, so the suppression that has covered the whole views layer (core/views/__init__.py never had its Django-ORM lookup types fixed) stopped applying the moment core.views.markets became a real submodule -- surfacing 3 pre-existing type errors that have nothing to do with this refactor. This preserves today's exact suppression scope against the new file layout; fixing those errors is separate work, not H6's. Note: my first attempt to commit this step failed at the mypy pre-commit hook, and I moved on to step 4 without registering that -- both step 3 and step 4 sat staged-but-uncommitted together until `git log` caught the gap. Split back apart here; no code was lost, but flagging the process failure. Verified: manage.py check clean, ruff clean, mypy clean (all 5 changed files), full pre-commit suite green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves _get_financing_value, _format_financing_value, _generate_pdf, and export_pdf verbatim. Only export_pdf (routed) and _generate_pdf (has a test patch target) are re-exported; the two formatting helpers are exports.py-internal only. Two fixes this step needed to be correct, both surfaced by running the full suite rather than trusting ruff-clean as sufficient: 1. US_STATES was a stray module-level constant that happened to sit between export_pdf and vrm_properties_list in the original file. My extraction absorbed it into exports.py's line range along with export_pdf, even though export_pdf never uses it -- four OTHER functions do (onboard, growth_explorer, pipeline_screening_settings, vrm_properties_list), none of which have moved yet. Extracted it to core/views/constants.py, the shared home all four will import from as they move. 2. test_pdf_export_returns_500_on_generation_failure patches "core.views._generate_pdf" to make export_pdf observe a forced failure. Before this split both functions lived in the same module, so patching the core.views alias worked. After the split, export_pdf resolves _generate_pdf via its own module globals (core.views.exports), not through the core.views re-export alias -- patching the alias no longer touches what export_pdf actually calls. Updated the test to patch core.views.exports._generate_pdf, matching where the call actually resolves now. Production behavior is unchanged; this is a white-box test's patch coordinate catching up to real module structure. This same trap will hit core.views.fetch_place_growth_metrics/ fetch_housing_demand_index/discover_places_in_state/FREDAdapter (43 patch sites, three test files) once growth.py moves in step 11 -- noted for that step rather than fixed blind now. Verified: manage.py check clean, ruff/mypy clean, and (before this step's re-commit split) the full unit+integration suite: 2186 passed / 4 failed -- same 4 pre-existing ATTOM failures as step 1's baseline, unrelated (see #413). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves the ten leasing_* views verbatim: list, kanban, add, detail, showing, application, screening, lease, move_in, stabilize. All routed in core/urls.py. Verified: manage.py check clean, ruff/mypy clean, 165 unit tests pass, 13 targeted leasing integration tests pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves the 15 pipeline stage-machine views verbatim: list, kanban, review_queue, review_csv, advance_stage, advance, kill, hold, reactivate, detail, add_from_source, offer_create, dd_checklist, renovation, closing_create. All routed in core/urls.py. At 838 lines this exceeds the plan's 800-line target -- flagging rather than silently missing it. Splitting further would mean cutting a single deal-stage state machine apart along an arbitrary line count rather than a real domain seam; screening.py (the actual second half of the original pipeline_* group) was already split out as its own module in the plan for exactly this reason. Leaving it as one cohesive unit. Verified: manage.py check clean, ruff/mypy clean, 165 unit tests pass, 110 targeted pipeline integration tests pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves pipeline_screener, screener_filter, pipeline_screening_settings, and screening_preview verbatim. All routed in core/urls.py. pipeline_screening_settings uses US_STATES from constants.py. Verified: manage.py check clean, ruff/mypy clean, 165 unit tests pass, 103 targeted screening integration tests pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves property_list, _parse_compare_ids, property_compare, property_detail, financing_comparison, property_add, property_edit, capex_item_edit, property_delete, property_add_income, property_add_expense, and property_share verbatim. All routed in core/urls.py except _parse_compare_ids (internal to property_compare only). Imports is_owner_or_shared, _get_property_role, _is_client_only_user from .permissions -- the heaviest cross-module RBAC dependency of any domain module, per the plan's reasoning for moving permissions.py first. Also switched regen.py's re-export strategy for good this step: ruff refuses to auto-delete unused imports in a literal __init__.py file (package re-export convention -- confirmed via its JSON diagnostics, which return fix: null there, not just an unapplied fix), so my earlier __all__-based approach made ruff hedge on every import in the file instead of pruning genuinely dead ones. Re-export lines now use explicit self-aliasing (`X as X`, ruff's own suggested convention), and I run the fixer against a temp copy under a non-"__init__.py" name to get ruff's real (not my regex) multi-line-import-safe fix, then copy the result back. Second real trap found running the full suite: test_compare_computes_analysis_when_missing did `monkeypatch.setattr(views, "compute_analysis_for_property", fake)` where `views` is `from core import views` -- an object reference, not a string, so my earlier core.views.X string-grep never would have caught it. Same root cause as the _generate_pdf trap in step 4: property_compare now resolves compute_analysis_for_property through properties.py's own module globals, not through the core.views alias. Fixed by patching views.properties (submodule auto-registered as an attribute of the core.views package on import) instead. Swept the whole test suite for the same object-reference pattern; this was the only occurrence. Verified: manage.py check clean, ruff/mypy clean, full unit+integration suite: 2186 passed / 4 failed -- same 4 pre-existing ATTOM failures as every prior step's baseline (see #413). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves dashboard, onboard, pipeline_dashboard, portfolio_dashboard, market_dashboard, and update_market_indicators verbatim. All routed in core/urls.py. Imports _is_client_only_user from .permissions and US_STATES from .constants. Verified: manage.py check clean, ruff/mypy clean, 165 unit tests pass, 28 targeted dashboard/onboard integration tests pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Moves search_listings, analyze_property, report_listing, report_property, vrm_properties_list, property_discovery, hud_property_list, hud_property_detail, usda_property_list, and usda_property_detail verbatim. All routed in core/urls.py. Imports logger and User for local use, and US_STATES from .constants. Only __init__.py's growth_areas, growth_areas_export_csv, and growth_explorer remain -- step 11 (growth.py) empties it down to a pure re-export shim. Verified: manage.py check clean, ruff/mypy clean, 165 unit tests pass, 127 targeted discovery/search/hud/usda/vrm integration tests pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Final step. Moves growth_areas, growth_areas_export_csv, and growth_explorer verbatim. core/views/__init__.py is now a pure 79-line re-export shim -- no logic, imports only, exactly the plan's Definition of Done. Removed the original header's `from core.integrations.market.census import (...)` and `from core.integrations.sources.fred_adapter import FREDAdapter` once growth.py owns them: keeping both the header's copy and growth.py's own copy, both re-exported into __init__.py, is a literal F811 redefinition of the same four names from two different import statements. growth.py is now the sole importer; __init__.py gets them via the explicit `.growth` re-export. Third and largest instance of the same-module-caller-callee patch trap flagged back in step 4 (#413's _generate_pdf,step 8's compute_analysis_for_property): 46 occurrences across three test files patched discover_places_in_state / fetch_housing_demand_index / fetch_place_growth_metrics / FREDAdapter.fetch_state_employment_growth via "core.views.X" -- all four are used exclusively by growth_explorer, and all four are IMPORTED (not defined) in the original file, so once growth_explorer's module changed, patching the core.views alias stopped reaching what growth_explorer's own module globals actually call. Updated every occurrence in test_growth_explorer_view.py, test_pipeline_bridge.py, and test_growth_explorer.py to "core.views.growth.X". Swept the whole test suite first to confirm no other object-reference-style patches (the compute_analysis_for_property pattern) existed for these names -- none did, all 46 were the string-literal form this sweep updates directly. Zero unpatched occurrences remain. H6 is complete: core/views/__init__.py went from 4315 lines / 78 top-level symbols to 79 lines of re-exports across 11 domain modules (permissions, system, markets, exports, leasing, pipeline, screening, properties, dashboards, discovery, growth). All 36 original core.views.* test patch targets still resolve -- some directly via re-export, some (this step, step 4, step 8) by being repointed to the submodule that now actually defines the name, which is the correct fix in each case, not a workaround. Verified: manage.py check clean, ruff/mypy clean, full pre-commit suite, and the definitive final gate -- the complete unit+integration suite: 2186 passed / 4 failed, the same 4 pre-existing ATTOM failures every checkpoint in this refactor has shown (see #413), unchanged from step 1's original baseline. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| selected_sources = request.POST.getlist("sources") | ||
| if not selected_sources: | ||
| messages.warning(request, "Please select at least one source.") | ||
| return redirect(f"{request.path}?growth_area_id={growth_area.pk}") |
| advance_stage(prop) | ||
| return JsonResponse({"status": "ok", "stage": prop.stage}) | ||
| except ValueError as exc: | ||
| return JsonResponse({"error": str(exc)}, status=400) |
|
|
||
| if not source_type or not source_id: | ||
| messages.error(request, "Missing source_type or source_id") | ||
| return redirect(next_url) |
| vrm = VrmProperty.objects.get(vrm_property_id=int(source_id)) | ||
| except VrmProperty.DoesNotExist, ValueError, TypeError: | ||
| messages.error(request, "VRM property not found") | ||
| return redirect(next_url) |
| hud = HudProperty.objects.get(hud_case_number=source_id) | ||
| except HudProperty.DoesNotExist: | ||
| messages.error(request, "HUD property not found") | ||
| return redirect(next_url) |
| cn = CountyForeclosureNotice.objects.get(pk=int(source_id)) | ||
| except CountyForeclosureNotice.DoesNotExist, ValueError, TypeError: | ||
| messages.error(request, "County notice not found") | ||
| return redirect(next_url) |
| return redirect("pipeline_detail", pk=pp.pk) | ||
|
|
||
| messages.error(request, f"Unknown source type: {source_type}") | ||
| return redirect(next_url) |
| request, | ||
| "Too many re-screens — please wait a few minutes and try again.", | ||
| ) | ||
| return redirect(request.get_full_path()) |
| messages.success(request, f"{pp.address[:40]} moved to Underwriting.") | ||
| except PipelineProperty.DoesNotExist: | ||
| pass | ||
| return redirect(request.get_full_path()) |
| pp.save(update_fields=["status", "kill_reason", "updated_at"]) | ||
| except PipelineProperty.DoesNotExist: | ||
| pass | ||
| return redirect(request.get_full_path()) |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Implements H6 from the production audit, per the plan in #417.
What changed
core/views/__init__.py— 4,315 lines, 78 top-level symbols — is now a 77-line pure re-export shim over 11 domain modules:permissions.pyconstants.pymarkets.pyexports.pysystem.pydashboards.pyscreening.pyleasing.pygrowth.pyproperties.pydiscovery.pypipeline.pyOne commit per module, in the order the plan specified (permissions first — no view surface, every RBAC-dependent module needs it; growth last — 35 of 36 test patch targets live there, so it moves only once the pattern is proven by ten prior commits). Each commit is independently
manage.py checkclean, ruff/mypy clean, and test-verified.pipeline.pyat 838 lines exceeds the plan's 800-line target — flagged rather than silently missed. Splitting it further would cut a single deal-stage state machine along an arbitrary line count rather than a real domain seam;screening.py(the actual second half of the originalpipeline_*group) was already split out as its own module for exactly this reason.Three real bugs the process caught — not hypothetical
Each was found by running the actual suite, not by ruff/mypy being clean (which they were, every time, when these were still broken):
A stray shared constant.
US_STATESphysically sat betweenexport_pdfandvrm_properties_listin the original file and got silently absorbed into whichever symbol's extracted range preceded it. It's needed by four different functions across four different new modules. Extracted tocore/views/constants.py.Mypy suppression scope gap.
mypy.inihad[mypy-core.views] ignore_errors = True— matches the exact module path only, not submodules. The moment real submodules existed, 3 pre-existing (unrelated to this refactor) type errors surfaced. Widened to[mypy-core.views.*], preserving today's exact suppression scope rather than fixing the underlying errors, which is separate work.The same-module patch trap, three times.
mock.patch("core.views.X")only patches the re-export alias in__init__.py. When the caller of X moved into the same new module as X itself, the caller's bare-name lookup resolves through the submodule's own globals — the alias patch stops reaching it. Found via_generate_pdf/export_pdf(step 4, 1 site),compute_analysis_for_property/property_compare(step 8, 1 site — a different pattern: an object-referencemonkeypatch.setattr(views, "X", ...)that no string grep would catch), and the four Census/FRED integration functions/growth_explorer(step 11, 46 sites across 3 test files). Every occurrence updated to patch the actual defining submodule; swept the whole suite each time to confirm no sibling occurrences were missed.Also fixed along the way
git logcaught the divergence. Split back into two clean commits; no code was lost, but noting the failure mode.ruffrefuses to auto-delete unused imports in a file literally named__init__.py(confirmed via its own JSON diagnostics returningfix: null, not just an unapplied fix — package re-export convention). Worked around by running the fixer against a temp copy under a different name, then copying the cleaned result back, rather than my own regex-based import surgery (which corrupted multi-line import blocks and needed abandoning mid-refactor).Verified
manage.py checkclean, full pre-commit suite (ruff, ruff format, mypy) cleanexports,properties, and the finalgrowthstep): 2186 passed / 4 failed every time — the same 4 pre-existing ATTOM live-API failures established in fix(census): correct stale return contract and surface live source failures #413's baseline, unrelated to this refactorcore.views.*test patch targets still resolve — some via direct re-export, some (3 documented cases above) repointed to the submodule that actually defines the name, which is the correct fix, not a workaroundcore/urls.py'sfrom . import views/from .views import MarketRefreshViewand everyviews.<name>route target confirmed against the real re-export set (69 route references + 5 direct patch targets, cross-checked against what each module actually needs to re-export)Test plan
🤖 Generated with Claude Code