apps.py and urls.py — App Config and URL Registration#48
Conversation
📝 WalkthroughWalkthroughAdds authenticated DRF endpoints for Boost: ChangesBoost Endpoint API Feature
Sequence Diagram(s)sequenceDiagram
participant Client
participant AddOrUpdateView
participant AddOrUpdateRequestSerializer
participant BoostComponentService
Client->>AddOrUpdateView: POST /add-or-update/ (organization, add_or_update, version, extensions?)
AddOrUpdateView->>AddOrUpdateRequestSerializer: validate(request.data)
alt validation succeeds
AddOrUpdateRequestSerializer-->>AddOrUpdateView: validated_data
AddOrUpdateView->>BoostComponentService: __init__(organization, lang_code, version, extensions)
AddOrUpdateView->>BoostComponentService: process_all(submodules, user, request)
alt service raises NotImplementedError
BoostComponentService-->>AddOrUpdateView: NotImplementedError
AddOrUpdateView-->>Client: 501 {detail}
else service raises unexpected exception
BoostComponentService-->>AddOrUpdateView: Exception
AddOrUpdateView-->>Client: 500 {"error":"Internal server error"}
else service returns results
BoostComponentService-->>AddOrUpdateView: results dict
AddOrUpdateView-->>Client: 200 results payload
end
else validation fails
AddOrUpdateRequestSerializer-->>AddOrUpdateView: errors
AddOrUpdateView-->>Client: 400 {errors}
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/endpoint/test_apps.py (1)
40-48: ⚡ Quick winVerify the registered pattern's namespace and structure, not just the count.
Test 3 confirms idempotency by checking that calling
register_plugin_urls()twice results in exactly one entry. However, it doesn't verify what was added. Inspecting the registered pattern would catch bugs like an incorrect namespace, a malformed pattern, or aNonevalue.♻️ Proposed enhancement to verify namespace and pattern type
def test_register_plugin_urls_appends_once(monkeypatch: pytest.MonkeyPatch) -> None: fake = types.ModuleType("weblate.urls") fake.real_patterns = [] monkeypatch.setitem(sys.modules, "weblate.urls", fake) register_plugin_urls() register_plugin_urls() assert len(fake.real_patterns) == 1 + # Verify the registered pattern has the expected namespace + pattern = fake.real_patterns[0] + assert hasattr(pattern, "namespace") + assert pattern.namespace == "boost_endpoint"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/endpoint/test_apps.py` around lines 40 - 48, Update the test_register_plugin_urls_appends_once test to not only assert len(fake.real_patterns) == 1 but also inspect the registered entry in fake.real_patterns: verify it is the expected pattern type (e.g., a django URLResolver/URLPattern) and has the correct namespace/name set by register_plugin_urls(); locate and check the single element from fake.real_patterns, asserting it's not None, has the expected attributes (namespace or name) and matches the expected structure so malformed or wrong-namespace registrations are caught.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/boost_weblate/endpoint/serializers.py`:
- Around line 35-37: The ListField definition for extensions currently allows
empty strings; update the child serializer for the extensions field (the
ListField named extensions in serializers.py) so that the child CharField
disallows blank values (set allow_blank to False or remove allow_blank) to
prevent "" entries from being accepted as valid extensions and causing invalid
filter tokens in requests.
---
Nitpick comments:
In `@tests/endpoint/test_apps.py`:
- Around line 40-48: Update the test_register_plugin_urls_appends_once test to
not only assert len(fake.real_patterns) == 1 but also inspect the registered
entry in fake.real_patterns: verify it is the expected pattern type (e.g., a
django URLResolver/URLPattern) and has the correct namespace/name set by
register_plugin_urls(); locate and check the single element from
fake.real_patterns, asserting it's not None, has the expected attributes
(namespace or name) and matches the expected structure so malformed or
wrong-namespace registrations are caught.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a68c9920-3c1e-438e-9c03-f6399e8e150d
📒 Files selected for processing (11)
src/boost_weblate/endpoint/apps.pysrc/boost_weblate/endpoint/serializers.pysrc/boost_weblate/endpoint/services.pysrc/boost_weblate/endpoint/urls.pysrc/boost_weblate/endpoint/views.pytests/endpoint/__init__.pytests/endpoint/test_apps.pytests/endpoint/test_serializers.pytests/endpoint/test_services.pytests/endpoint/test_views.pytests/test_boost_plugin_urls.py
💤 Files with no reviewable changes (1)
- tests/test_boost_plugin_urls.py
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
tests/endpoint/test_apps.py (1)
34-35: ⚡ Quick winConsider stubbing the parent
weblatepackage for test robustness.While submodule-only stubbing works in the current environment, stubbing the parent
weblatepackage alongsideweblate.urlsfollows defensive programming practices and ensures more deterministic import behavior across different environments. This prevents potential import resolution issues that could arise if tests run in different contexts.Suggested improvement
@@ def test_register_plugin_urls_skips_without_real_patterns( monkeypatch: pytest.MonkeyPatch, ) -> None: + fake_pkg = types.ModuleType("weblate") + fake_pkg.__path__ = [] fake = types.ModuleType("weblate.urls") + fake_pkg.urls = fake + monkeypatch.setitem(sys.modules, "weblate", fake_pkg) monkeypatch.setitem(sys.modules, "weblate.urls", fake) register_plugin_urls() assert not hasattr(fake, "real_patterns") @@ def test_register_plugin_urls_appends_once(monkeypatch: pytest.MonkeyPatch) -> None: + fake_pkg = types.ModuleType("weblate") + fake_pkg.__path__ = [] fake = types.ModuleType("weblate.urls") fake.real_patterns = [] + fake_pkg.urls = fake + monkeypatch.setitem(sys.modules, "weblate", fake_pkg) monkeypatch.setitem(sys.modules, "weblate.urls", fake)Also applies to: 41-43
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/endpoint/test_apps.py` around lines 34 - 35, Stub the parent package as well as the submodule to avoid import resolution issues: create a ModuleType for "weblate" (alongside the existing fake for "weblate.urls"), insert it into sys.modules via monkeypatch.setitem(sys.modules, "weblate", parent_fake), and ensure parent_fake.urls is set to the existing fake module before setting sys.modules["weblate.urls"]; apply the same change to the other stub block around lines 41-43 so both places create and register the parent package module in addition to the submodule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/boost_weblate/endpoint/serializers.py`:
- Around line 50-61: The validation currently checks only values but not
language keys, so update the loop that iterates over value.items() to also
validate lang_code: ensure lang_code is a non-empty string (e.g.,
isinstance(lang_code, str) and lang_code.strip() != ""), and if it fails add an
error entry in errors[str(lang_code)] with a message like "add_or_update: each
key must be a non-empty language code; got <repr(lang_code)>", then skip further
submodules checks for that key; keep the existing submodules validations for
other keys.
In `@src/boost_weblate/endpoint/views.py`:
- Around line 69-95: The loop over add_or_update currently uses one outer
try/except so a later failure masks earlier successes; change it to try/except
around each iteration (per lang_code) when calling
BoostComponentService(...).process_all(...) so you capture and log exceptions
per language (include organization and lang_code in the log), accumulate a
results dict that maps each lang_code to either the success payload or an error
description, and at the end return a structured response that contains
per-language outcomes; if all succeed return the normal success status, if some
fail return a multi-status response (HTTP 207) with the mixed results.
---
Nitpick comments:
In `@tests/endpoint/test_apps.py`:
- Around line 34-35: Stub the parent package as well as the submodule to avoid
import resolution issues: create a ModuleType for "weblate" (alongside the
existing fake for "weblate.urls"), insert it into sys.modules via
monkeypatch.setitem(sys.modules, "weblate", parent_fake), and ensure
parent_fake.urls is set to the existing fake module before setting
sys.modules["weblate.urls"]; apply the same change to the other stub block
around lines 41-43 so both places create and register the parent package module
in addition to the submodule.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b0514f1e-9467-40a0-b966-bbbc513a6ab2
📒 Files selected for processing (11)
src/boost_weblate/endpoint/apps.pysrc/boost_weblate/endpoint/serializers.pysrc/boost_weblate/endpoint/services.pysrc/boost_weblate/endpoint/urls.pysrc/boost_weblate/endpoint/views.pytests/endpoint/__init__.pytests/endpoint/test_apps.pytests/endpoint/test_serializers.pytests/endpoint/test_services.pytests/endpoint/test_views.pytests/test_boost_plugin_urls.py
💤 Files with no reviewable changes (1)
- tests/test_boost_plugin_urls.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/endpoint/test_views.py (1)
89-137: ⚡ Quick winAdd a mixed-outcome test for the 207 response path.
The new per-language branching in
AddOrUpdateViewincludes aHTTP_207_MULTI_STATUScase, but current tests only cover all-error paths (501/500). Please add one test where one language succeeds and another fails to lock in that contract.Proposed test shape
+def test_add_or_update_returns_multi_status_for_mixed_outcomes( + monkeypatch: pytest.MonkeyPatch, +) -> None: + factory = APIRequestFactory() + request = factory.post( + "/add-or-update/", + { + "organization": "o", + "version": "v", + "add_or_update": {"ja": ["json"], "fr": ["unordered"]}, + }, + format="json", + ) + user = User(username="t_user5") + force_authenticate(request, user=user) + + service_ja = MagicMock() + service_ja.process_all.return_value = {"ok": True} + service_fr = MagicMock() + service_fr.process_all.side_effect = RuntimeError("boom") + + def service_factory(*, lang_code, **_kwargs): + return service_ja if lang_code == "ja" else service_fr + + monkeypatch.setattr( + "boost_weblate.endpoint.views.BoostComponentService", + service_factory, + ) + + response = AddOrUpdateView.as_view()(request) + assert response.status_code == status.HTTP_207_MULTI_STATUS + assert response.data["languages"]["ja"]["status"] == "success" + assert response.data["languages"]["fr"]["status"] == "error"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/endpoint/test_views.py` around lines 89 - 137, Add a new test (e.g., test_add_or_update_mixed_outcome_returns_multi_status) alongside the existing tests in tests/endpoint/test_views.py that posts to AddOrUpdateView with two languages (one that will succeed and one that will fail) using APIRequestFactory and force_authenticate; monkeypatch BoostComponentService (the same target used in test_add_or_update_internal_error_is_masked) so it returns a success result for one language and raises for the other, then call AddOrUpdateView.as_view()(request) and assert response.status_code == status.HTTP_207_MULTI_STATUS and that response.data contains the original organization/version plus per-language entries where the successful language has status "ok" (or equivalent success marker) and the failing language has status "error" and includes the masked error shape used elsewhere.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/endpoint/test_views.py`:
- Around line 89-137: Add a new test (e.g.,
test_add_or_update_mixed_outcome_returns_multi_status) alongside the existing
tests in tests/endpoint/test_views.py that posts to AddOrUpdateView with two
languages (one that will succeed and one that will fail) using APIRequestFactory
and force_authenticate; monkeypatch BoostComponentService (the same target used
in test_add_or_update_internal_error_is_masked) so it returns a success result
for one language and raises for the other, then call
AddOrUpdateView.as_view()(request) and assert response.status_code ==
status.HTTP_207_MULTI_STATUS and that response.data contains the original
organization/version plus per-language entries where the successful language has
status "ok" (or equivalent success marker) and the failing language has status
"error" and includes the masked error shape used elsewhere.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ce0aae11-3893-438f-a264-1f55d42c80d9
📒 Files selected for processing (3)
src/boost_weblate/endpoint/serializers.pysrc/boost_weblate/endpoint/views.pytests/endpoint/test_views.py
|
@coderabbitai resume |
✅ Actions performedReviews resumed. |
Close #41
Summary by CodeRabbit
New Features
Tests