Register moderation urls for all plugins #1166
Conversation
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1166 +/- ##
==========================================
+ Coverage 92.23% 92.28% +0.04%
==========================================
Files 331 331
Lines 10087 10092 +5
Branches 927 928 +1
==========================================
+ Hits 9304 9313 +9
+ Misses 657 653 -4
Partials 126 126 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a moderation URL extension point: BasePlugin gains get_moderation_urls(), PluginRegistry aggregates plugins' moderation URLs, and moderation URLConf appends those patterns. Adds a unit test for BasePlugin default getters and updates a python-packages submodule reference. Changes
Sequence Diagram(s)sequenceDiagram
participant URLConf as Moderation URLConf
participant Registry as PluginRegistry
participant Plugin as Plugin[i]
participant Router as Django URL router
URLConf->>Registry: get_moderation_urls()
loop for each plugin
Registry->>Plugin: plugin.get_moderation_urls()
Plugin-->>Registry: [URLPattern...]
end
Registry-->>URLConf: combined [URLPattern...]
URLConf->>Router: moderation_urls += combined patterns
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Register moderation urls for all plugins
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
python-packages (1)
1-1: Verify python-packages submodule bump, tag provenance, and CI initializationI wasn’t able to fetch the submodule automatically (access returned 404), so please manually confirm:
- The new SHA b3f80dc6 corresponds to a known tag or release in the python-packages repo (pinning to a tag is preferred for reproducibility).
- There are no breaking or behavioral changes between 5197ec47… and b3f80dc6… that would affect the new moderation URL integration.
- Your CI pipelines include a
git submodule update --init --recursivestep before build/test so the submodule is properly initialized.If helpful, consider pinning the submodule URL to HTTPS or switching to a tagged ref, and adding an explicit CI step to initialize submodules.
django/thunderstore/plugins/base.py (1)
25-28: Add a short docstring and consider future-proofing the typeA brief docstring will help plugin authors discover intent. Also, if plugins might ever return URLResolver (e.g., via include()), the current List[URLPattern] annotation could be too narrow for static type checkers (runtime is fine). Not a blocker, just a note.
Apply this minimal docstring:
@classmethod def get_moderation_urls(cls) -> List[URLPattern]: - return [] + """Plugin hook for moderation-related URL patterns. + + Override in plugins to extend the moderation URLConf. + """ + return []If you expect plugins to return URLResolver too, we could widen the type in a follow-up:
- Base: List[Union[URLPattern, URLResolver]]
- Registry methods accordingly
Would you like me to prep that follow-up change?
django/thunderstore/plugins/tests/test_plugins.py (1)
4-9: Consider adding coverage for the registry aggregatorA focused test that stubs a simple plugin returning a sentinel URL and asserting that PluginRegistry.get_moderation_urls() aggregates it would guard against regressions in the plumbing.
I can add a test plugin class within tests to validate aggregation behavior without touching real plugins. Want me to draft that test?
django/thunderstore/plugins/registry.py (1)
39-41: Ordering is non-deterministic due to Set iteration — verify it won’t matter for URL resolutionSince plugins is a Set, route order depends on hash order. If different plugins register overlapping paths, resolution priority may vary between runs. This follows existing patterns, but moderation routes might be more sensitive to order.
If you want deterministic ordering without changing PluginRegistry.plugins, consider sorting just for URL aggregation. Example change (optional; apply across all URL aggregations for consistency in a follow-up):
- def get_moderation_urls(self) -> List[URLPattern]: - return list(itertools.chain(*(x.get_moderation_urls() for x in self.plugins))) + def get_moderation_urls(self) -> List[URLPattern]: + # Preserve deterministic ordering by class name to avoid URL resolution surprises + plugins = sorted(self.plugins, key=lambda p: p.__name__) + return list(itertools.chain.from_iterable(x.get_moderation_urls() for x in plugins))Would you like me to propose a separate PR aligning ordering across all similar getters?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (5)
django/thunderstore/moderation/urls.py(2 hunks)django/thunderstore/plugins/base.py(1 hunks)django/thunderstore/plugins/registry.py(1 hunks)django/thunderstore/plugins/tests/test_plugins.py(1 hunks)python-packages(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
django/thunderstore/plugins/tests/test_plugins.py (1)
django/thunderstore/plugins/base.py (1)
BasePlugin(14-45)
django/thunderstore/plugins/registry.py (1)
django/thunderstore/plugins/base.py (1)
get_moderation_urls(26-27)
django/thunderstore/moderation/urls.py (2)
django/thunderstore/plugins/base.py (1)
get_moderation_urls(26-27)django/thunderstore/plugins/registry.py (1)
get_moderation_urls(39-40)
django/thunderstore/plugins/base.py (1)
django/thunderstore/plugins/registry.py (1)
get_moderation_urls(39-40)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build docker image
🔇 Additional comments (5)
django/thunderstore/plugins/base.py (1)
25-28: Moderation URL hook added — looks good and aligns with existing plugin APIThe new classmethod mirrors the existing getters’ shape and defaults, keeping backward compatibility and providing a clean extension point.
django/thunderstore/plugins/tests/test_plugins.py (1)
4-9: Solid sanity check for BasePlugin defaultsCovers the new getter and keeps behavior consistent across the existing ones. Good minimal test.
django/thunderstore/moderation/urls.py (2)
3-12: Appending plugin-provided moderation URLs is correctImporting plugin_registry and extending the list keeps the existing URL intact while enabling plugin contributions.
3-12: Watch for circular‐import risks when discovering plugins
Loadingplugin_registryat import time indjango/thunderstore/moderation/urls.py(viaautodiscover→load_ts_plugins()) will eagerly import allts_*.ts_pluginsmodules. Meanwhile,django/thunderstore/core/urls.pybrings inmoderation_urls(line 19), so if any plugin’sts_pluginssubmodule ever imports back intothunderstore.moderation.urls(or eventhunderstore.core.urls), you’ll end up with an import cycle.Please verify that none of your installed
ts_*plugin packages perform such imports at module scope. If this pattern bites you later, consider deferring plugin loading (e.g. in an AppConfig.ready hook) or wrappingget_moderation_urls()behind a function instead of doing it at import time.django/thunderstore/plugins/registry.py (1)
39-41: Aggregator mirrors existing patterns — OKConsistent with get_settings_urls/get_legacy_package_urls. Produces a flat list suitable to extend moderation URLs.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
django/thunderstore/plugins/base.py (1)
25-28: Add a short docstring for plugin authorsA tiny docstring clarifies purpose and expectations for implementers.
Apply this diff:
@classmethod def get_moderation_urls(cls) -> List[URLPattern]: - return [] + """Hook for plugin-provided moderation URL patterns. + + Override in plugins to return a list of URLPattern instances + related to moderation functionality. + """ + return []
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (5)
django/thunderstore/moderation/urls.py(2 hunks)django/thunderstore/plugins/base.py(1 hunks)django/thunderstore/plugins/registry.py(1 hunks)django/thunderstore/plugins/tests/test_plugins.py(1 hunks)python-packages(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- python-packages
🚧 Files skipped from review as they are similar to previous changes (2)
- django/thunderstore/plugins/tests/test_plugins.py
- django/thunderstore/plugins/registry.py
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build docker image
- GitHub Check: Build docker image
🔇 Additional comments (2)
django/thunderstore/plugins/base.py (1)
25-28: Introduce get_moderation_urls extension point — LGTMConsistent with existing plugin hooks, safe default, and clear type signature.
django/thunderstore/moderation/urls.py (1)
12-12: Appending plugin-provided moderation URLs — LGTMOrdering preserves the core route precedence while enabling plugin extensions.
2 participants
Register moderation urls for all plugins
Summary by CodeRabbit
New Features
Tests
Chores