éditeur de gabarit pour le remplissage de pdf (#74797) #122

Merged
bdauvergne merged 3 commits from wip/74797-connecteur-PDF-gabarit-de-xfdf-p into main 2023-03-02 17:29:58 +01:00
Owner
No description provided.
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from a1a91da76f to c4cec7781c 2023-03-01 15:55:23 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from c4cec7781c to c347e73600 2023-03-01 16:38:46 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from c347e73600 to f2deaa65c6 2023-03-01 17:09:51 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from f2deaa65c6 to 949e222e12 2023-03-01 17:55:12 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 949e222e12 to 6d104ff81d 2023-03-01 17:58:37 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 6d104ff81d to 3e4972c496 2023-03-01 17:59:08 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 3e4972c496 to a42958744b 2023-03-01 18:38:05 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from a42958744b to 61b5a47432 2023-03-01 19:04:00 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from ed55ad2fd0 to 2af12aa100 2023-03-01 21:22:04 +01:00 Compare
Author
Owner

Le build est bloqué sur le bug #73760 qui na pas de rapport.

Le build est bloqué sur le bug #73760 qui na pas de rapport.
bdauvergne changed title from WIP: éditeur de gabarit pour le remplissage de pdf to éditeur de gabarit pour le remplissage de pdf 2023-03-01 21:47:11 +01:00
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from fb8c625640 to 37d82b5b25 2023-03-01 21:51:42 +01:00 Compare
Owner

Le build est bloqué sur le bug #73760 qui na pas de rapport.

Le build échoue aussi sur pylint,

> Le build est bloqué sur le bug #73760 qui na pas de rapport. Le build échoue aussi sur pylint,
Owner

Aussi, il ne faudra pas utiliser ce ticket pour faire passer des changements type pg_virtualenv.

Aussi, il ne faudra pas utiliser ce ticket pour faire passer des changements type pg_virtualenv.
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 0b5e5bb18c to 7c6a7f4b3d 2023-03-02 07:40:03 +01:00 Compare
Author
Owner

Aussi, il ne faudra pas utiliser ce ticket pour faire passer des changements type pg_virtualenv.

Ce nétait pas l'idée mais je voulais me convaincre que je le problème ne venait pas du tout de l'isolation de la DB.

> Aussi, il ne faudra pas utiliser ce ticket pour faire passer des changements type pg_virtualenv. Ce nétait pas l'idée mais je voulais me convaincre que je le problème ne venait pas du tout de l'isolation de la DB.
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 7c6a7f4b3d to 51975356dc 2023-03-02 07:41:10 +01:00 Compare
fpeters requested changes 2023-03-02 07:44:55 +01:00
@ -0,0 +47,4 @@
return super().get_success_url() + '#'
class PageThumbnailView(ResourceView):
Owner
Cette partie n'est absolument pas couverte par les tests : https://jenkins.entrouvert.org/job/gitea/job/passerelle/job/wip%252F74797-connecteur-PDF-gabarit-de-xfdf-p/15/Coverage_20Report_20_28native_29/d_1f676f23cbeaa486_views_py.html
Author
Owner

J'ai rajouté la couverture de ce code dans le test.

J'ai rajouté la couverture de ce code dans le test.
@ -0,0 +40,4 @@
widget_type: str = dataclasses.field(compare=False)
rect: Rect = dataclasses.field(compare=False)
on_value: str = dataclasses.field(compare=False, default=pdfrw.PdfName.On)
annotation: typing.Optional[pdfrw.PdfDict] = dataclasses.field(default=None, repr=False)
Owner
    annotation: typing.Optional[pdfrw.PdfDict] = dataclasses.field(default=None, repr=False) 

Je ne suis vraiment pas à l'aise avec cette ligne (et un peu celles d'avant); le gain (?) ne vaut pour moi certainement pas l'introduction de cette partie qui pourra plus difficilement être maintenue; je préférerais vraiment que ça devienne du Python plus commun.

``` annotation: typing.Optional[pdfrw.PdfDict] = dataclasses.field(default=None, repr=False) ``` Je ne suis vraiment pas à l'aise avec cette ligne (et un peu celles d'avant); le gain (?) ne vaut pour moi certainement pas l'introduction de cette partie qui pourra plus difficilement être maintenue; je préférerais vraiment que ça devienne du Python plus commun.
Author
Owner

Il y a déjà du code équivalent dans chrono et lingo donc non.

Il y a déjà du code équivalent dans chrono et lingo donc non.
Owner

Vraiment 1/ je sais que j'ai déjà vu du code ainsi passer, 2/ j'ai vérifié dans chrono avant d'écrire mon commentaire et le code n'y a pas de ligne aussi incompréhensible.

Vraiment 1/ je sais que j'ai déjà vu du code ainsi passer, 2/ j'ai vérifié dans chrono avant d'écrire mon commentaire et le code n'y a pas de ligne aussi incompréhensible.
Author
Owner
chrono/agendas/models.py:@dataclasses.dataclass(frozen=True)
chrono/agendas/models.py-class SharedCustodySlot:
chrono/agendas/models.py-    guardian: Person = dataclasses.field(compare=False)
chrono/agendas/models.py-    date: datetime.date
chrono/agendas/models.py-    label: str = dataclasses.field(compare=False, default='')

Si tu peux me pointer les lignes de https://docs.python.org/3/library/dataclasses.html qu´on peut utiliser ou pas, ça m'aidera.

``` chrono/agendas/models.py:@dataclasses.dataclass(frozen=True) chrono/agendas/models.py-class SharedCustodySlot: chrono/agendas/models.py- guardian: Person = dataclasses.field(compare=False) chrono/agendas/models.py- date: datetime.date chrono/agendas/models.py- label: str = dataclasses.field(compare=False, default='') ``` Si tu peux me pointer les lignes de https://docs.python.org/3/library/dataclasses.html qu´on peut utiliser ou pas, ça m'aidera.
Owner
    annotation: typing.Optional[pdfrw.PdfDict] = dataclasses.field(default=None, repr=False) 

Tu peux éventuellement réagir en décidant de poser un commentaire explicatif, ou je ne sais quoi d'autre j'ai aucune idée du pourquoi/comment de cette ligne.

Mais perso s'il faut une règle je dirais bien que l'import du module typing serait mon niveau de blocage. (inutile aussi de me pointer le from typing import List dans lingo).

``` annotation: typing.Optional[pdfrw.PdfDict] = dataclasses.field(default=None, repr=False) ``` Tu peux éventuellement réagir en décidant de poser un commentaire explicatif, ou je ne sais quoi d'autre j'ai aucune idée du pourquoi/comment de cette ligne. Mais perso s'il faut une règle je dirais bien que l'import du module typing serait mon niveau de blocage. (inutile aussi de me pointer le `from typing import List` dans lingo).
Author
Owner

Si le probl

Si le probl
Owner

(le message n'est pas passé)

(le message n'est pas passé)
Author
Owner

J'ai viré le typing.Optional.

J'ai viré le typing.Optional.
@ -0,0 +220,4 @@
if not flatten:
pdfrw.PdfWriter().write(file_object, self._pdf_reader)
else:
with io.BytesIO() as fd:
Owner

Cette partie (write avec flatten à True) n'est pas testée du tout. (je préfèrerais qu'on n'introduise pas trop de code inutilisé).

Cette partie (write avec flatten à True) n'est pas testée du tout. (je préfèrerais qu'on n'introduise pas trop de code inutilisé).
Author
Owner

J'ai rajouté la couverture de ce code dans le test.

J'ai rajouté la couverture de ce code dans le test.
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 51975356dc to 4eae2dbe3e 2023-03-02 08:16:22 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 4eae2dbe3e to 23794e1945 2023-03-02 08:33:48 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 23794e1945 to f17a906142 2023-03-02 09:04:34 +01:00 Compare
fpeters changed title from éditeur de gabarit pour le remplissage de pdf to éditeur de gabarit pour le remplissage de pdf (#74797) 2023-03-02 09:14:09 +01:00
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from f17a906142 to f3fae6b416 2023-03-02 09:35:50 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from f3fae6b416 to e110b06ee7 2023-03-02 09:39:11 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from e110b06ee7 to 2510261dce 2023-03-02 10:12:57 +01:00 Compare
Author
Owner

Il me restait un souci avec le pre-commit debian car wrap-and-sort trie différemment sur la machine jenkins et sur la mienne où devscripts était en version unstable.

Il me restait un souci avec le pre-commit debian car wrap-and-sort trie différemment sur la machine jenkins et sur la mienne où devscripts était en version unstable.
bdauvergne requested review from fpeters 2023-03-02 14:44:17 +01:00
tnoel requested changes 2023-03-02 14:48:52 +01:00
@ -259,0 +298,4 @@
),
}
@endpoint(
Owner

Ça m'irait mieux de remplacer complétement fill-form.

Ca veut dire aussi que

  • le nom de fill_form_file "Fill Form default input file" devient "Fill Form input File"
  • et le help_text peut être juste "PDF file"
Ça m'irait mieux de remplacer complétement fill-form. Ca veut dire aussi que * le nom de fill_form_file "Fill Form default input file" devient "Fill Form input File" * et le help_text peut être juste "PDF file"
@ -259,0 +330,4 @@
)
if not self.fill_form_file:
raise APIError('not PDF file configured', http_status=500)
Owner

Même si un code 5xx parait correct ici, ça m'ennuie de l'utiliser à cause de ce que ça signifie habituellement, à savoir "y'a un bogue dans passerelle".

Parce que non, c'est juste un bogue de config...

Je pense qu'on pourrait plutôt renvoyer une 4xx (genre 418, 406 voire 404) ou même juste laisser la 200 (parce qu'à l'usage ça va vite être vu).

(même si je sais qu'on ne devrait pas renvoyer une 4xx parce que c'est pas une erreur du client)

Même si un code 5xx parait correct ici, ça m'ennuie de l'utiliser à cause de ce que ça signifie habituellement, à savoir "y'a un bogue dans passerelle". Parce que non, c'est juste un bogue de config... Je pense qu'on pourrait plutôt renvoyer une 4xx (genre 418, 406 voire 404) ou même juste laisser la 200 (parce qu'à l'usage ça va vite être vu). (même si je sais qu'on ne devrait pas renvoyer une 4xx parce que c'est pas une erreur du client)
@ -3,1 +3,4 @@
{% block actions %}
{% if object|can_edit:request.user %}
<a href="{% url 'pdf-fields-mapping-edit' connector='pdf' slug=object.slug %}">{% trans 'Edit field mapping' %}</a>
Owner

Peut-être nommer le bouton "Fill Form: Edit field mapping" pour préciser que c'est un bouton lié au système de remplissage, on traduira par "Formulaire : configuration des champs"

Peut-être nommer le bouton "Fill Form: Edit field mapping" pour préciser que c'est un bouton lié au système de remplissage, on traduira par "Formulaire : configuration des champs"
@ -14,3 +20,2 @@
<div>
<p>{% blocktrans with file=object.fill_form_file %}PDFtk {{ file }} dump_data_fields_utf8 output{% endblocktrans %}</p>
<p>{{ object.pdftk_dump_data_fields_utf8|safe }}</p>
<p>{{ object.list_fields_mapping|safe }}</p>
Owner

Je pense qu'on peut supprimer cet onglet avec la disparition du fill-form actuel.

Je pense qu'on peut supprimer cet onglet avec la disparition du fill-form actuel.
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 2510261dce to 3d00a1e24a 2023-03-02 15:45:00 +01:00 Compare
bdauvergne removed review request for fpeters 2023-03-02 15:45:13 +01:00
bdauvergne requested review from tnoel 2023-03-02 15:45:17 +01:00
tnoel approved these changes 2023-03-02 16:24:18 +01:00
tnoel left a comment
Owner

Deux pépins de i18n ratés en première lecture (déso).

Deux pépins de i18n ratés en première lecture (déso).
@ -0,0 +41,4 @@
field_class = TemplateField
else:
continue
label = f'field {i + 1} ({help_text})'
Owner

Pépin de i18n ici, il faut qu'on ait au moins la traduction de _('field')

Pépin de i18n ici, il faut qu'on ait au moins la traduction de _('field')
Author
Owner

il faut que ça corresponde à ce qu'il y a dans la miniature du PDF annotée avec pillow donc non.

il faut que ça corresponde à ce qu'il y a dans la miniature du PDF annotée avec pillow donc non.
@ -0,0 +62,4 @@
</div>
{% endif %}
{% for page_number, image_map in pages %}
<h3>Page {{ page_number|add:1 }}</h3>
Owner

Autre petit manque sur l'i18n ici, il faudrait plutôt

<h3>{% blocktrans with number=page_number|add:1 %}Page {{number}}{% endblocktrans %}</h3>

Autre petit manque sur l'i18n ici, il faudrait plutôt `<h3>{% blocktrans with number=page_number|add:1 %}Page {{number}}{% endblocktrans %}</h3>`
Author
Owner

ok c'est fait.

ok c'est fait.
tnoel requested changes 2023-03-02 16:27:18 +01:00
tnoel left a comment
Owner

Oubli d'un petit i18n de plus...

Oubli d'un petit i18n de plus...
@ -0,0 +63,4 @@
y = (area_rect.y1 + area_rect.y2) / 2 - 5
if field.widget_type == 'checkbox':
y -= 10
draw.text((x, y), f'field {i + 1}', anchor='lb', fill=(0, 0, 0, 255))
Owner

i18n ici encore, un _('field') qui manque (idéalement un _(''field %d) mais à toi de voir)

i18n ici encore, un `_('field')` qui manque (idéalement un `_(''field %d)` mais à toi de voir)
Author
Owner

Avec la police non vectorielle par défaut de pillow je ne sais pas si on peut avoir de l'UTF-8 (je vois bien qu'on traduirait par "champ {i + 1}" mais je ne suis pas convaincu de l'utilité ici de traduire, ça reste un écran technique ou personne n'ira à part un admin fonctionnel.

Avec la police non vectorielle par défaut de pillow je ne sais pas si on peut avoir de l'UTF-8 (je vois bien qu'on traduirait par "champ {i + 1}" mais je ne suis pas convaincu de l'utilité ici de traduire, ça reste un écran technique ou personne n'ira à part un admin fonctionnel.
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 3d00a1e24a to 393d7ae975 2023-03-02 16:32:41 +01:00 Compare
bdauvergne requested review from tnoel 2023-03-02 16:33:36 +01:00
tnoel requested changes 2023-03-02 16:34:34 +01:00
tnoel left a comment
Owner

Dernier voeux : on a des boutons « Enregistrer » et « Annuler » tout en bas de la page de gestions des champs. C'est trop bas... Il faudrait idéalement les répéter tout en haut de la page.

(Et ou aussi avoir un truc anti-oubli qui lève un popup quand on veut quitter la page alors qu'on n'a pas validé le formulaire, ça existe dans redmine...)

Dernier voeux : on a des boutons « Enregistrer » et « Annuler » tout en bas de la page de gestions des champs. C'est trop bas... Il faudrait idéalement les répéter tout en haut de la page. (Et ou aussi avoir un truc anti-oubli qui lève un popup quand on veut quitter la page alors qu'on n'a pas validé le formulaire, ça existe dans redmine...)
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 393d7ae975 to 7471956673 2023-03-02 16:39:02 +01:00 Compare
bdauvergne force-pushed wip/74797-connecteur-PDF-gabarit-de-xfdf-p from 7471956673 to 06b13b20db 2023-03-02 17:06:34 +01:00 Compare
bdauvergne requested review from tnoel 2023-03-02 17:21:39 +01:00
tnoel approved these changes 2023-03-02 17:29:20 +01:00
bdauvergne merged commit 0d5db81460 into main 2023-03-02 17:29:58 +01:00
bdauvergne deleted branch wip/74797-connecteur-PDF-gabarit-de-xfdf-p 2023-03-02 17:29:58 +01:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: entrouvert/passerelle#122
No description provided.