Reference index - rebase of #9170 #9279
Conversation
MySQL doesn't allow long fields to be used in unique indexes.
Co-authored-by: Matt Westcott <matthew@torchbox.com>
…belonging to a more-specific one (Mostly) fixes the issue described in wagtail#9170 (comment). When deleting old references within `ReferenceIndex.create_or_update_for_object`, skip over any with a source content_type that does not match the currently-indexed object or any of its superclasses - these can be assumed to come from a more specific version of the object, with relations that we don't know about from inspecting the less-specific one. This does have the side effect that once an object has been indexed in its more specific form, any invocations of create_or_update_for_object on the less-specific model will be over-cautious, and fail to delete records even if they legitimately refer to relations that exist on the base model - i.e. those references will stick around until the more specific model is indexed. This is a lesser bug than the original, though, and running `rebuild_references_index` will make the index consistent with the database state.
Manage this branch in SquashTest this branch here: https://gasmankaedroho-reference-index-vkxp2.squash.io |
| edit_link_title = None | ||
| else: | ||
| label = str(object) | ||
| edit_link_title = _("Edit this %s") % object._meta.verbose_name |
There was a problem hiding this comment.
Wondering if it's possible to word this in a way to make it easier to translate for languages with genders? (we've probably got a lot of cases like this so maybe this should be solved separately)
There was a problem hiding this comment.
Yep, this is already a frequent issue in the Wagtail codebase, particularly within modeladmin and snippets. #7117 raises this and a couple of similar issues.
|
|
||
| @classmethod | ||
| def extract_references(cls, attrs): | ||
| yield cls.get_model(), attrs["id"], "", "" |
There was a problem hiding this comment.
was mentioning this earlier, but this would be a great to add type annotations to this, and perhaps use a TypedDict as it takes some cognitive load get this
There was a problem hiding this comment.
Have added some supporting comments in 0c2dee3 (but the definitive description of the data format is still on the docstring for ReferenceIndex._extract_references_from_object, as I think it's best to avoid repeating this too much)
|
|
||
| # Find existing references in the database so we know what to add/delete | ||
| existing_references = { | ||
| (to_content_type_id, to_object_id, model_path, content_path): ( |
There was a problem hiding this comment.
nitpick from my tired self: this comprehension and the one below (deleted_reference_ids) while smart, they also make the code more difficult to parse
There was a problem hiding this comment.
Added extra comments in 0c2dee3 and turned the second comprehension into a plain loop - hopefully that makes it easier to follow.
|
Merged in 4c7e1ea |
#9170 rebased with additional fixes:
rebuild_references_indexand elsewhere) no longer wrongly removes references belonging to child modelsextract_references)