Show on delete information in usage views to be linked from delete/unpublish confirmation views #10072
Conversation
Manage this branch in SquashTest this branch here: https://laymonageusage-before-delete-u-lqo95.squash.io |
375393e to
9030c8f
Compare
aaf38cd to
991f5cf
Compare
991f5cf to
f05fa2f
Compare
| label = f"{capfirst(field.verbose_name)}" | ||
| block = field.stream_block | ||
| block_idx = 1 | ||
| while isinstance(block, StreamBlock): |
There was a problem hiding this comment.
As a note to pre-empt feature creep more than anything...
If we're intending to support recursing into blocks to any depth, then we probably ought to recognise StructBlock / ListBlock, and at that point the concept of 'following path components' should be made into a method on the Block API rather than matching individual block types here.
However, given that:
- it's probably uncommon for people to nest StreamBlocks directly in StreamBlocks
- if we explicitly write this code to not go more than one level, we'd probably want an
if isinstance(block, StreamBlock)here anyhow to cover the possibility that we might support other top-level blocks besides StreamBlock in the near future (see "normalize" mechanism for StreamField blocks #9738)
...this code in its current form will in practice only descend one level most of the time, and changing it to actually enforce only going one level deep would remove a maybe-occasionally-useful feature for no real benefit. So for that reason, I'm happy to stick with this :-)
There was a problem hiding this comment.
Ahh yep, I just realised nesting StreamBlock in a StreamBlock isn't very useful!
if we explicitly write this code to not go more than one level, we'd probably want an if isinstance(block, StreamBlock) here anyhow to cover the possibility that we might support other top-level blocks besides StreamBlock in the near future (see #9738)
That's exactly what I had initially, then I thought people might have a nested StreamBlock and the code to handle it wasn't that different apart from changing the if to while, so I made the call. Would definitely be great if we can normalise this across all "container" blocks, though!
|
Sorry, just found another niggle - if an |
f05fa2f to
d75b89c
Compare
|
|
||
| return TemplateResponse( | ||
| request, | ||
| "wagtaildocs/documents/confirm_delete.html", |
There was a problem hiding this comment.
Do you think we should override get_templates() and look for this template before self.template_name in case people override this template?
I think it's unlikely people override this template though...
We don't have to do this for images and pages because we are still using their templates instead of the generic one. As for snippets, the work in #10206 should cover template customisation in a more robust way.
|
In laymonage@20f3b28 the confirmation message needs to be inside the |
Make use of cached properties and ContentType's manager cache
Show the block path and label to better understand which block has the reference
This allows us to make use of self.object in dispatch() for permission checks
…e in generic DeleteView
f51a873 to
568aa64
Compare
|
This is amazing stuff. Thanks @laymonage and @gasman |
|
Great to see this making it into core! |
Fixes #10008, may also fix #1602 albeit with a different approach. Needs #10099. Likely closes #4702.
This PR ensures there's a link to the usage view from the delete and unpublish confirmation views. For the delete view, the link to the usage view has an extra
?describe_on_delete=1parameter, which will make the usage view also describe what happens to the references of the object that's to be deleted.The anchor linked in the specific fields currently still doesn't work as mentioned in #10007 (review). I'll try to make a separate PR if I find a way to fix it.
Performance optimisations
The first commit adds some performance optimisations when accessing the usage view. This is mostly done by using the
ContentTypemodel's default manager so that we use its cache instead of accessing theForeignKeydirectly, which will query the database. Additional optimisation is also done for the count by making it a cached property.Before
After
Please check the following:
make lintfrom the Wagtail root.Please describe additional details for testing this change.
Footnotes
Development Testing ↩
Browser and device support ↩
Accessibility Target ↩