fix: Show internal links in the language of the plugin #250
Conversation
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
Reviewer's GuideAdds language awareness to internal link widgets by passing the plugin’s language through widget initialization, URL generation, form fields, and plugin form rendering. Sequence diagram for plugin form rendering with language-aware internal linkssequenceDiagram
participant Admin as actor Admin User
participant PluginAdmin as PluginAdmin
participant PluginForm as PluginForm
participant LinkWidget as LinkWidget
participant InternalLinkWidget as LinkAutoCompleteWidget
Admin->>PluginAdmin: Open plugin add/edit form
PluginAdmin->>PluginForm: get_form(request, obj)
PluginForm->>LinkWidget: Initialize with language=plugin.language
LinkWidget->>InternalLinkWidget: Set language attribute
InternalLinkWidget->>InternalLinkWidget: get_url() includes ?language=plugin.language
PluginForm-->>PluginAdmin: Return form with language-aware widgets
PluginAdmin-->>Admin: Render form with internal links in plugin language
Class diagram for language-aware LinkWidget and related classesclassDiagram
class LinkAutoCompleteWidget {
+language: str | None
+get_url()
}
class LinkWidget {
+language: str | None
+__init__(site_selector: bool | None, language: str | None)
}
class LinkFormField {
+widget: LinkWidget
+__init__(..., language: str | None)
}
class LinkField {
+formfield(...)
}
LinkWidget --> LinkAutoCompleteWidget : uses
LinkFormField --> LinkWidget : has
LinkField --> LinkFormField : formfield()
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @fsbraun - I've reviewed your changes and they look great!
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location> `djangocms_link/cms_plugins.py:63` </location>
<code_context>
+ language = getattr(obj, "language", None)
+ else:
+ language = request.GET.get("plugin_language", None)
+ for widget in self.fields["link"].widget.widgets:
+ widget.language = language
+ return form
</code_context>
<issue_to_address>
Directly mutating widget.language may cause issues if widgets are shared across forms.
Mutating shared widget instances can cause side effects. Ensure each form uses unique widget instances or clone them before modifying the language attribute.
</issue_to_address>
### Comment 2
<location> `djangocms_link/fields.py:375` </location>
<code_context>
super().__init__(*args, **kwargs)
def formfield(self, **kwargs):
+ print("LinkField.formfield", kwargs)
kwargs.setdefault("form_class", LinkFormField)
return super().formfield(**kwargs)
</code_context>
<issue_to_address>
Debug print statement left in production code.
Please remove this print statement before merging to avoid unnecessary console output and potential information exposure.
</issue_to_address>
### Comment 3
<location> `djangocms_link/fields.py:255` </location>
<code_context>
- },
- ),
- ) # Site selector
+ widgets[index].language = language # Pass on language to the internal link widget
+ if site_selector:
+ widgets.insert(
</code_context>
<issue_to_address>
Assigning language directly to widget may not propagate as expected if widgets are reused.
Directly setting the language on shared widget instances can lead to side effects. Ensure each form uses unique widget instances or use a safer way to handle language context.
</issue_to_address>
### Comment 4
<location> `djangocms_link/fields.py:310` </location>
<code_context>
kwargs.setdefault("initial", {})
kwargs.pop("encoder", None) # Passed from LinkField's JSONField parent class
kwargs.pop("decoder", None) # but not needed
+ self.widget = LinkWidget(language=kwargs.pop("language", None))
super().__init__(*args, **kwargs)
if isinstance(self.initial, dict):
</code_context>
<issue_to_address>
Overriding self.widget in __init__ may break expected widget customization via kwargs.
Setting self.widget here ignores any widget provided in kwargs. To preserve customization, check for a widget in kwargs before assigning a default.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #250 +/- ##
==========================================
- Coverage 96.31% 95.91% -0.40%
==========================================
Files 28 28
Lines 706 735 +29
Branches 98 104 +6
==========================================
+ Hits 680 705 +25
- Misses 9 10 +1
- Partials 17 20 +3 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Co-authored-by: sourcery-ai[bot] <58596630+sourcery-ai[bot]@users.noreply.github.com>
|
Thank you, Fabian! We've tested it and it looks great! Adding a link on a German page: Adding a Link to a French page: One thing that we've noticed: 
|
|
@krosefield Fallback languages still active? If no, the gap probably should go away. If yes, the fallback title should be shown. |
…k into fix/target-lang
|
I'm sorry for the delay! Test-setup: A website with three languages (de/en/fr). Fallback language is active and set to English. I've created three pages that are only available in de, en or fr: Creating a link on a German webpage, I see the following pages in the page tree selector: Creating a link on a English webpage, I see the following pages: Creating a link on a French webpage, I see the following pages: When it comes to a page that is actually available in different languages, everything works fine:
I hope this is understandable... and somehow some helpful information . :) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
When adding or editing a plugin, the plugin fields are in the language of the plugin (not the admin language). This also needs to hold for the internal links. This PR adjusts the language of the internal link dropdown. (The dropdown still uses fallback languages if necessary and available.)
Additionally, the link to internal pages shown in the structure board now reflects the language that is edited (i.e., show
/it/pagina/instead of/en/page/as a short description in the structure board when editing Italian content).Dutch, German and French locales are updated.
Fixes #249
Tests are to be added.
@krosefield Please be invited to test.
Related resources
Checklist
masterSlack to find a “pr review buddy” who is going to review my pull request.