tests: Add extensive tests for ArchivedPlugin.restore#54
Merged
fsbraun merged 3 commits intodjango-cms:masterfrom Dec 5, 2025
Merged
tests: Add extensive tests for ArchivedPlugin.restore#54fsbraun merged 3 commits intodjango-cms:masterfrom
fsbraun merged 3 commits intodjango-cms:masterfrom
Conversation
Contributor
Reviewer's GuideAdds targeted regression tests for ArchivedPlugin.restore, introduces a minimal test app with plugin models to exercise FK and M2M relations, and switches internal plugin model resolution to use django-cms’s built-in helper instead of local utilities. Sequence diagram for ArchivedPlugin.restore using django CMS get_plugin_modelsequenceDiagram
actor TestCase
participant ArchivedPlugin
participant CmsPluginsUtils as cms_utils_plugins
participant PluginModel
participant Database
TestCase->>ArchivedPlugin: restore(target_placeholder_id, parent_plugin_id)
activate ArchivedPlugin
ArchivedPlugin->>CmsPluginsUtils: get_plugin_model(plugin_type)
activate CmsPluginsUtils
CmsPluginsUtils-->>ArchivedPlugin: PluginModel
deactivate CmsPluginsUtils
ArchivedPlugin->>PluginModel: construct_from_archived_data(serialized_data)
activate PluginModel
PluginModel->>Database: INSERT plugin row
Database-->>PluginModel: plugin_pk
PluginModel->>Database: RESTORE foreign_key_relations()
PluginModel->>Database: RESTORE many_to_many_relations()
PluginModel-->>ArchivedPlugin: restored_plugin_instance
deactivate PluginModel
ArchivedPlugin-->>TestCase: restored_plugin_instance
deactivate ArchivedPlugin
Entity relationship diagram for test app plugin models with FK and M2MerDiagram
CMSPlugin {
int id
int placeholder_id
int parent_id
string language
int position
}
FKPluginModel {
int id
int cmsplugin_ptr_id
int related_model_id
}
RelatedModel {
int id
}
M2MPluginModel {
int id
int cmsplugin_ptr_id
}
TagModel {
int id
}
M2MPluginModel_Tags {
int id
int m2m_plugin_model_id
int tag_model_id
}
CMSPlugin ||--o| FKPluginModel : one_to_one_inheritance
CMSPlugin ||--o| M2MPluginModel : one_to_one_inheritance
FKPluginModel }o--|| RelatedModel : foreign_key
M2MPluginModel ||--o{ M2MPluginModel_Tags : join_table
TagModel ||--o{ M2MPluginModel_Tags : join_table
Updated class diagram for ArchivedPlugin and plugin model resolutionclassDiagram
class UtilsModule {
+get_related_fields(model)
+get_plugin_fields(plugin_type)
}
class CmsPluginsUtils {
+get_plugin_model(plugin_type)
}
class CMSPlugin {
+id
+placeholder_id
+parent_id
+language
+position
}
class ArchivedPlugin {
+plugin_type
+serialized_data
+language
+position
+placeholder_id
+parent_id
+restore(target_placeholder_id, parent_plugin_id)
}
class FKPluginModel {
+foreign_object
}
class M2MPluginModel {
+related_objects
}
class RelatedModel {
+id
}
class TagModel {
+id
}
FKPluginModel --|> CMSPlugin
M2MPluginModel --|> CMSPlugin
FKPluginModel --> RelatedModel : foreign_key
M2MPluginModel --> TagModel : many_to_many
ArchivedPlugin ..> CmsPluginsUtils : uses
CmsPluginsUtils ..> FKPluginModel : returns
CmsPluginsUtils ..> M2MPluginModel : returns
UtilsModule ..> CMSPlugin : introspects
UtilsModule ..> FKPluginModel : introspects
UtilsModule ..> M2MPluginModel : introspects
File-Level Changes
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}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- The tests that assert hard-coded primary keys (e.g., expecting restored_plugin.pk == 2) are brittle and may become flaky depending on database state; consider asserting relative behavior (such as existence, equality with original fields, or count of plugins) instead of specific PK values.
- In the skipped test for non-existing related fields you expect section to be None, but the Article.section field is non-nullable (no null=True), so either the test expectation or the model definition should be adjusted to match the intended behavior before enabling this test.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The tests that assert hard-coded primary keys (e.g., expecting restored_plugin.pk == 2) are brittle and may become flaky depending on database state; consider asserting relative behavior (such as existence, equality with original fields, or count of plugins) instead of specific PK values.
- In the skipped test for non-existing related fields you expect section to be None, but the Article.section field is non-nullable (no null=True), so either the test expectation or the model definition should be adjusted to match the intended behavior before enabling this test.
## Individual Comments
### Comment 1
<location> `tests/test_import.py:29-32` </location>
<code_context>
+ placeholder = self.page_content.placeholders.create(slot="test")
+ restored_plugin = archived_plugin.restore(placeholder, "en")
+ self.assertEqual(restored_plugin.title, "Test")
+ # second ArticlePluginModel instance in a new placeholder
+ self.assertEqual(restored_plugin.pk, 2)
+ self.assertEqual(restored_plugin.position, 1)
+ self.assertEqual(
</code_context>
<issue_to_address>
**suggestion (testing):** Avoid asserting a hard-coded primary key value in tests
Asserting `restored_plugin.pk == 2` makes this test brittle because it depends on database state and plugin creation order. Instead, consider asserting that the restored plugin has a different pk than the original (e.g. `assertNotEqual(restored_plugin.pk, plugin.pk)`) or that it appears in the expected queryset/placeholder. This will keep the test stable across environments and future fixture changes.
```suggestion
self.assertEqual(restored_plugin.title, "Test")
# restored plugin should be a new instance in the new placeholder
self.assertNotEqual(restored_plugin.pk, plugin.pk)
self.assertEqual(restored_plugin.placeholder, placeholder)
self.assertEqual(restored_plugin.position, 1)
```
</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✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #54 +/- ##
==========================================
+ Coverage 84.92% 91.70% +6.78%
==========================================
Files 10 10
Lines 398 398
Branches 56 56
==========================================
+ Hits 338 365 +27
+ Misses 49 22 -27
Partials 11 11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
fsbraun
reviewed
Nov 26, 2025
| return [field.name for field in fields if field.is_relation] | ||
|
|
||
|
|
||
| @lru_cache() |
Member
There was a problem hiding this comment.
This is actually the "official" way to get a plugin class. I would keep it instead of using the utils which might change in the future...
Contributor
Author
There was a problem hiding this comment.
Alright then.
@fsbraun Even for the get_plugin_model too right?
fsbraun
approved these changes
Nov 26, 2025
Contributor
Author
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.
Description
masterDiscord to find a “pr review buddy” who is going to review my pull request.
Summary by Sourcery
Strengthen ArchivedPlugin restore behavior and align plugin model lookup with django CMS helpers.
Enhancements:
Tests: