wip/72542-drafts-stats (#72542) #1100

Merged
ecazenave merged 1 commits from wip/72542-drafts-stats into main 2024-03-29 09:10:52 +01:00
4 changed files with 250 additions and 1 deletions

View File

@ -8,6 +8,7 @@ import xml.etree.ElementTree as ET
import pytest
import responses
from django.utils.timezone import localtime
from pyquery import PyQuery
from webtest import Upload
@ -4801,6 +4802,141 @@ def test_admin_form_inspect_validation(pub):
assert not resp.pyquery('[data-field-id="4"] .parameter-validation').length
def test_admin_form_inspect_drafts(pub):
create_superuser(pub)
FormDef.wipe()
formdef = FormDef()
formdef.name = 'form title'
formdef.enable_tracking_codes = True
formdef.fields = [
fields.PageField(id='0', label='1st page'),
fields.StringField(id='1', label='string 1'),
fields.PageField(id='2', label='2nd page'),
fields.StringField(id='3', label='string 2'),
fields.PageField(id='4', label='3rd page'),
fields.StringField(id='5', label='string 3'),
]
formdef.store()
formdef.data_class().wipe()
app = login(get_app(pub))
resp = app.get('/backoffice/forms/%s/inspect' % formdef.id)
assert resp.pyquery('#inspect-drafts p').text() == 'There are currently no drafts for this form.'
data_class = formdef.data_class()
formdata = data_class()
formdata.status = 'draft'
formdata.page_id = '0'
formdata.receipt_time = localtime()
formdata.store()
formdata = data_class()
formdata.status = 'draft'
formdata.page_id = '2'
formdata.receipt_time = localtime()
formdata.store()
formdata = data_class()
formdata.status = 'draft'
formdata.page_id = '4'
formdata.receipt_time = localtime()
formdata.store()
formdata = data_class()
formdata.status = 'draft'
formdata.page_id = '_confirmation_page'
formdata.receipt_time = localtime()
formdata.store()
formdata = data_class()
formdata.status = 'draft'
formdata.page_id = 'xxxx' # unkown page id
formdata.receipt_time = localtime()
formdata.store()
resp = app.get('/backoffice/forms/%s/inspect' % formdef.id)
assert resp.pyquery('#inspect-drafts p')[0].text == 'Statistics on drafts by page.'
assert (
resp.pyquery('#inspect-drafts p')[1].text
== 'Lifespan of drafts (in days): %s.' % formdef.get_drafts_lifespan()
)
assert resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="0"]').length == 1
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="0"] td.label').text()
== '1st page'
)
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="0"] td.percent').text()
== '20%'
)
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="0"] td.total').text()
== '(1/5)'
)
assert resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="2"]').length == 1
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="2"] td.label').text()
== '2nd page'
)
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="2"] td.percent').text()
== '20%'
)
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="2"] td.total').text()
== '(1/5)'
)
assert resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="4"]').length == 1
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="4"] td.label').text()
== '3rd page'
)
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="4"] td.percent').text()
== '20%'
)
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="4"] td.total').text()
== '(1/5)'
)
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="_confirmation_page"]').length
== 1
)
assert (
resp.pyquery(
'table[data-table-id="rate-among-drafts"] tr[data-page-id="_confirmation_page"] td.label'
).text()
== 'Confirmation page'
)
assert (
resp.pyquery(
'table[data-table-id="rate-among-drafts"] tr[data-page-id="_confirmation_page"] td.percent'
).text()
== '20%'
)
assert (
resp.pyquery(
'table[data-table-id="rate-among-drafts"] tr[data-page-id="_confirmation_page"] td.total'
).text()
== '(1/5)'
)
assert resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="_unknown"]').length == 1
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="_unknown"] td.label').text()
== 'Unknown'
)
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="_unknown"] td.percent').text()
== '20%'
)
assert (
resp.pyquery('table[data-table-id="rate-among-drafts"] tr[data-page-id="_unknown"] td.total').text()
== '(1/5)'
)
def test_form_import_fields(pub):
create_superuser(pub)
create_role(pub)

View File

@ -14,6 +14,7 @@
# You should have received a copy of the GNU General Public License
# along with this program; if not, see <http://www.gnu.org/licenses/>.
import datetime
import difflib
import io
import xml.etree.ElementTree as ET
@ -28,6 +29,7 @@ from wcs.backoffice.deprecations import DeprecationsDirectory
from wcs.backoffice.snapshots import SnapshotsDirectory
from wcs.carddef import CardDef
from wcs.categories import Category
from wcs.fields import PageField
from wcs.formdef import (
DRAFTS_DEFAULT_LIFESPAN,
FormDef,
@ -59,7 +61,7 @@ from wcs.qommon.form import (
)
from wcs.qommon.misc import localstrftime
from wcs.roles import get_user_roles, logged_users_role
from wcs.sql_criterias import Equal, Null, StrictNotEqual
from wcs.sql_criterias import Equal, GreaterOrEqual, Null, StrictNotEqual
from wcs.workflows import Workflow
from . import utils
@ -1793,6 +1795,62 @@ class FormDefPage(Directory, TempfileDirectoryMixin):
f'{self.formdef.xml_root_node}:{self.formdef.id}'
)
context['deprecation_titles'] = deprecations.titles
temp_drafts = defaultdict(int)
for formdata in self.formdef.data_class().select_iterator(

Il me semble qu'il serait plus lisible d'avoir les pages dans leur ordre d'apparition, le rendu graphique avec la barre permettra facilement de visualiser la taille.

Il me semble qu'il serait plus lisible d'avoir les pages dans leur ordre d'apparition, le rendu graphique avec la barre permettra facilement de visualiser la taille.
clause=[Equal('status', 'draft')], itersize=200
):
page_id = formdata.page_id if formdata.page_id is not None else '_unknown'
temp_drafts[page_id] += 1
total_drafts = sum(temp_drafts.values()) if temp_drafts else 0
total_formdata = 0
drafts = {}
special_page_index_mapping = {
'_first_page': -1000, # first
'_unknown': 1000, # last
'_confirmation_page': 999, # second to last
}
if total_drafts:
for page_id, page_index in special_page_index_mapping.items():
try:
page_total = temp_drafts.pop(page_id)
except KeyError:
page_total = 0

Je ne me baserais pas du tout sur page_no, mais sur l'ordre actuel des pages. (avec page_no avec les pages conditionnelles on peut avoir les pages qui in fine ne sont pas dans l'ordre).

Je ne me baserais pas du tout sur page_no, mais sur l'ordre actuel des pages. (avec page_no avec les pages conditionnelles on peut avoir les pages qui in fine ne sont pas dans l'ordre).
drafts[page_id] = {'total': page_total, 'field': None, 'page_index': page_index}
for page_id, page_total in temp_drafts.items():
for index, field in enumerate(self.formdef.iter_fields(with_backoffice_fields=False)):
if page_id == field.id and isinstance(field, PageField):
drafts[page_id] = {
'total': page_total,
'field': field,
'page_index': index,
}
break
else:
drafts['_unknown']['total'] += page_total
total_formdata = self.formdef.data_class().count(
[
GreaterOrEqual(
'receipt_time',
datetime.datetime.now() - datetime.timedelta(days=self.formdef.get_drafts_lifespan()),
)
]
)
for draft_data in drafts.values():
draft_percent = 100 * draft_data['total'] / total_drafts
draft_data['percent'] = draft_percent
draft_data['percent_str'] = '%.1f' % draft_percent
to_formdata_percent = 100 * draft_data['total'] / total_formdata
draft_data['to_formdata_percent'] = to_formdata_percent
draft_data['to_formdata_percent_str'] = '%.1f' % to_formdata_percent
context['drafts'] = sorted(drafts.items(), key=lambda x: x[1]['page_index'])
context['total_formdata'] = total_formdata
context['total_drafts'] = total_drafts
context['is_carddef'] = isinstance(self.formdef, CardDef)
return template.QommonTemplateResponse(
templates=[self.inspect_template_name],
context=context,

View File

@ -11,6 +11,9 @@
<button role="tab" aria-selected="false" aria-controls="inspect-workflow" id="tab-workflow" tabindex="-1">{% trans "Workflow" %}</button>
<button role="tab" aria-selected="false" aria-controls="inspect-options" id="tab-options" tabindex="-1">{% trans "Options" %}</button>
<button role="tab" aria-selected="false" aria-controls="inspect-fields" id="tab-fields" tabindex="-1">{% trans "Fields" %}</button>
{% if not snapshots_diff and not is_carddef %}
<button role="tab" aria-selected="false" aria-controls="inspect-drafts" id="tab-drafts" tabindex="-1">{% trans "Drafts" %}</button>

Attention cette page d'inspect est également utilisée pour les modèles de fiches, il faudrait sans doute ne pas reprendre cet onglet dans ce cas.

Attention cette page d'inspect est également utilisée pour les modèles de fiches, il faudrait sans doute ne pas reprendre cet onglet dans ce cas.
{% endif %}
{% if custom_views %}
<button role="tab" aria-selected="false" aria-controls="inspect-customviews" id="tab-customviews" tabindex="-1">{% trans "Custom views" %}</button>
{% endif %}
@ -92,6 +95,35 @@
{% endfor %}
</div>

À la place de juste "Page" il pourrait y avoir une introduction (paragraphe dans un div infonotice) pour expliciter un peu ce qui est sur cette page, éventuellement sous forme de warning (warningnotice) dans le cas de durée de vie des brouillons courte, type moins de 5 jours, pour dire que ça ne représente sans doute pas grand chose.

À la place de juste "Page" il pourrait y avoir une introduction (paragraphe dans un div infonotice) pour expliciter un peu ce qui est sur cette page, éventuellement sous forme de warning (warningnotice) dans le cas de durée de vie des brouillons courte, type moins de 5 jours, pour dire que ça ne représente sans doute pas grand chose.

J'ai mis "Statistics on drafts by page." pour la description et je me contente ensuite de rappeler la durée de vie des brouillons.

J'ai mis "Statistics on drafts by page." pour la description et je me contente ensuite de rappeler la durée de vie des brouillons.
{% if not snapshots_diff and not is_carddef %}

On peut éviter le jeu de piste et fournir la valeur plutôt qu'écrire "valeur par défaut" ?

On peut éviter le jeu de piste et fournir la valeur plutôt qu'écrire "valeur par défaut" ?
<div id="inspect-drafts" role="tabpanel" tabindex="0" aria-labelledby="tab-drafts" hidden>
{% if drafts %}
<div class="infonotice">
<p>{% trans "Statistics on drafts by page." %}</p>
<p>{% trans "Lifespan of drafts (in days)" %}{% trans ":" %} {{ formdef.get_drafts_lifespan }}.</p>
</div>
<h3>{% trans "Rate among drafts" %}</h2>

Formellement c'est désormais autorisé mais en pratique ça peut poser des problèmes donc je verrais bien un préfixe à l'attribut id (ou l'utilisation d'un autre attribut, type data-page-id, qui n'aura pas de signification particulière).

Formellement c'est désormais autorisé mais en pratique ça peut poser des problèmes donc je verrais bien un préfixe à l'attribut id (ou l'utilisation d'un autre attribut, type data-page-id, qui n'aura pas de signification particulière).
<table class="stats" data-table-id="rate-among-drafts">
<tbody>
{% for page_drafts in drafts %}
{% include "wcs/backoffice/includes/inspect-draft-by-page.html" with page_id=page_drafts.0 field=page_drafts.1.field percent=page_drafts.1.percent percent_str=page_drafts.1.percent_str num=page_drafts.1.total den=total_drafts %}
{% endfor %}
</tbody>
</table>
<h3>{% trans "Rate among total forms" %}</h2>
<table class="stats" data-table-id="rate-among-total-forms">

cf commentaire sur les tests, ça devrait passer par gettext pour avoir l'espace insécable en français, pas en anglais.

Ça permettrait aussi de formatter le nombre pour limiter les décimales.

cf commentaire sur les tests, ça devrait passer par gettext pour avoir l'espace insécable en français, pas en anglais. Ça permettrait aussi de formatter le nombre pour limiter les décimales.
<tbody>
{% for page_drafts in drafts %}
{% include "wcs/backoffice/includes/inspect-draft-by-page.html" with page_id=page_drafts.0 field=page_drafts.1.field percent=page_drafts.1.to_formdata_percent percent_str=page_drafts.1.to_formdata_percent_str num=page_drafts.1.total den=total_formdata %}
{% endfor %}
</tbody>

Pour la largeur par contre pas de problème à laisser toutes les décimales.

Pour la largeur par contre pas de problème à laisser toutes les décimales.

Non ça ne fonctionne pas avec les décimales.

Non ça ne fonctionne pas avec les décimales.

Non ça ne fonctionne pas avec les décimales.

Vraisemblablement parce que ça sort avec la virgule comme séparateur de décimal.

> Non ça ne fonctionne pas avec les décimales. Vraisemblablement parce que ça sort avec la virgule comme séparateur de décimal.
</table>
{% else %}
<p>{% trans "There are currently no drafts for this form." %}</p>
{% endif %}
</div>
{% endif %}
<div id="inspect-customviews" role="tabpanel" tabindex="0" aria-labelledby="tab-customviews" hidden>
<div>

Terminer la phrase par un point, et peut-être reformuler façon "There are currently no drafts for this form.".

Terminer la phrase par un point, et peut-être reformuler façon "There are currently no drafts for this form.".
{% for custom_view in custom_views %}

View File

@ -0,0 +1,23 @@
{% load i18n %}
{% if num %}
<tr data-page-id="{{ page_id }}">
<td class="label">
{% if page_id == "_unknown" %}
{% trans "Unknown" %}
{% elif page_id == "_first_page" %}
{% trans "Only page" %}
{% elif page_id == "_confirmation_page" %}
{% trans "Confirmation page" %}
{% else %}
{{ field.ellipsized_label }}
{% endif %}
</td>
<td class="percent">{{ percent|floatformat }}{% trans "%" %}</td>
<td class="total">({{ num }}/{{ den }})</td>
</tr>
<tr>
<td class="bar" colspan="3">
<span style="width: {{ percent_str }}%"></span>
</td>
</tr>
{% endif %}