Also fix an issue where the workflow tasks report is accessible despite
the user does not have the necessary page permissions, because the fix
in 13048eab79 is missing the view
dispatch() override for the workflow task report view.
This wasn't a security risk as the queryset is filtered down to the data
where the user has the relevant permissions for.
This allows users to also show commenting actions *only*. In addition,
this also fixes an issue where the active filter mechanism thinks that
the hide_commenting_action filter is active with the value False when
the filter is left unchecked.
The test for the inactive filter state is already covered by
test_unfiltered.
Similar to 2173a42345
Without adding an {% if breadcrumbs_items %}, this means the active
filter pills will also be shown even if the listing is still using the
old designs. However, at this point there should be no built-in views in
Wagtail that uses the old design _and_ have filters.
The active filter's clear button works pretty well with the old designs
(when _show_breadcrumbs = False). However, the filters (on the right side)
won't be updated because they are not re-rendered when the AJAX response
comes in.
Rather than adding the if check, we let this happen for now, and
hopefully can set _show_breadcrumbs to True by default in all views,
then advise people on how to adjust to the new templates...
The export buttons, while working correctly on initial load, is still
broken upon filtering on the client-side. This is because the base
listing results template has yet to include the re-rendered header
buttons. As a result, the export links won't be updated on filter.
This will be fixed in the next commit.
Also, the "by task" and "by workflow" links in the workflow reports are
gone because the slim header doesn't support passing arbitrary actions
fragment. This will be fixed in another commit using header_buttons.
This brings us one step closer to the generic BaseListingView and its
template.
Pagination is now rendered via the base listing_results.html template.
Technically we can remove the {% if object_list %} since the check is
already done outside of the results block in that base template.
However, leave it out for now so Git correctly detects that the files
are just renamed for now.
Also, only do this for views that extend the base ReportView and
base_report.html template for now. We'll do the same for PageReportView
and base_page_report.html later, since the template for that one is
a bit different (especially around which blocks are named listing vs
results...)
This report doesn't extend PageReportView or base_page_report.html,
there's no reason for it to follow the structure of
base_page_report.html (having no_results block, the extra wrapper div,
etc.). Simplify this template to follow other non-page reports, so we
can unify them to use a separate results template in the next commit.
- <tbody> only allows <tr> as child elements, so using <p> here is
incorrect.
- The no_results block in _list_* templates will never be rendered,
because the outer template that includes these templates already
do the checking for whether the results are empty or not:
- See in base_page_report.html, the _list_page_report.html is only
included `if pages` – else, the base_page_report.html defines a
no_results block. This block is what actually gets rendered when
there are no results. In short: the no_results block should be
defined in templates that extend base_page_report.html, not
_list_page_report.html (the included template).
- Similarly for _list_page_types_usage.html, the no_results block
should be defined in the view template (page_types_usage.html). This
is already the case, so the block in _list_page_types_usage.html
really shouldn't exist. (Moreover, it does not even extend
_list_page_report.html...)
- Note that as of right now, only the base_page_report.html and
page_types_usage.html templates support the no_results block.
This means the no_results block in aging_pages.html and
site_history.html (both of which extend the base_report.html instead
of base_page_report.html) do not get rendered. This explains why there
are duplicate markups for the no results message in an "else" tag a few
lines above.
Also see the documentation for adding reports:
https://docs.wagtail.org/en/stable/extending/adding_reports.html
The no_results block in the template is only shown for the view
template (one that extends base_page_report.html, not the smaller
include one that extends _list_page_report.html).
`BaseLogEntryManager` leaves this unimplemented, and `ModelLogEntryManager` implements it but `PageLogEntryManager` doesn't. The `LogActionRegistry.get_logs_for_instance` method calls this, which means it fails on page models.
Currently this is only used on generic views, which aren't used by page models (unless they're also registered as a snippet, which isn't really a supported setup) but LogActionRegistry is supposed to work transparently across log models, so this is clearly a bug.
Before the converter was fixed, the possible_sites list contains tuples
of (site_pk, url_to_match) where the site_pk is unique for the whole
list.
With the initial version of the fix, the pk may appear twice as we have
two url_to_match: with and without the wagtail_serve path. As a result,
we would fetch the Site object twice when matching URLs that include the
serve path.
This commit optimizes the fix by using a dictionary using the key as the
site_pk that maps to a list of urls to match, so we can fetch each site
exactly once, to then be used for matching the urls.
