misc: allow setting a custom id template on cards (#44604) #49

Merged
fpeters merged 1 commits from wip/44604-card-id into main 2023-11-28 08:59:37 +01:00
Owner
No description provided.
fpeters force-pushed wip/44604-card-id from 5359f62e76 to 3e40486683 2023-01-22 18:56:16 +01:00 Compare
fpeters force-pushed wip/44604-card-id from 3e40486683 to fe3102479a 2023-01-22 19:29:56 +01:00 Compare
fpeters force-pushed wip/44604-card-id from fe3102479a to ba705d74a6 2023-05-13 19:47:19 +02:00 Compare
fpeters force-pushed wip/44604-card-id from ba705d74a6 to c078037ac2 2023-06-24 09:23:36 +02:00 Compare
fpeters force-pushed wip/44604-card-id from c078037ac2 to bc3b175428 2023-07-18 16:54:49 +02:00 Compare
fpeters force-pushed wip/44604-card-id from bc3b175428 to d2ca678d60 2023-07-18 17:57:03 +02:00 Compare
fpeters force-pushed wip/44604-card-id from d2ca678d60 to dc5c647da2 2023-10-30 17:05:35 +01:00 Compare
fpeters force-pushed wip/44604-card-id from dc5c647da2 to c0d865fa0a 2023-10-30 17:28:14 +01:00 Compare
fpeters force-pushed wip/44604-card-id from c0d865fa0a to fbcc3b929b 2023-11-25 16:27:23 +01:00 Compare
fpeters force-pushed wip/44604-card-id from fbcc3b929b to b221e34c8b 2023-11-26 10:52:01 +01:00 Compare
fpeters force-pushed wip/44604-card-id from b221e34c8b to 15728f9581 2023-11-26 11:21:26 +01:00 Compare
fpeters reviewed 2023-11-27 14:59:13 +01:00
@ -46,1 +47,4 @@
def get_templates_form(self):
form = super().get_templates_form()
if not get_publisher().has_site_option('enable-card-identifier-template'):
Author
Owner

Tout ça se passe derrière un feature flag.

Tout ça se passe derrière un feature flag.
@ -47,0 +56,4 @@
kwargs = {}
if self.formdef.data_class().exists(criterias):
kwargs['readonly'] = True
kwargs['hint'] = _('Identifier cannot be modified if there are existing cards.')
Author
Owner

S'il y a déjà des données on ne permet pas la modification de ce qui constitue l'identifiant.

S'il y a déjà des données on ne permet pas la modification de ce qui constitue l'identifiant.
@ -47,0 +60,4 @@
form.add(
StringWidget,
'id_template',
title=_('Unique identifier template'),
Author
Owner

Nouvel attribut, id_template, qui contiendra de quoi générer un identifiant.

Nouvel attribut, id_template, qui contiendra de quoi générer un identifiant.
@ -408,3 +408,3 @@
form_page_class = CardFillPage
sidebar_recorded_message = _('The card has been recorded on %(date)s with the number %(number)s.')
sidebar_recorded_message = _('The card has been recorded on %(date)s with the identifier %(identifier)s.')
Author
Owner

À la marge, ajustement des messages.

À la marge, ajustement des messages.
@ -1812,3 +1812,3 @@
def get_formdef_fields(self, include_block_items_fields=False):
yield FakeField('id', 'id', _('Number'))
yield FakeField('id', 'id', _('Identifier') if self.formdef.id_template else _('Number'))
Author
Owner

Et pour les colonnes, quand il y a un identifiant custom on appelle plus ça "numéro".

Et pour les colonnes, quand il y a un identifiant custom on appelle plus ça "numéro".
@ -3266,3 +3266,3 @@
form_page_class = FormBackofficeEditPage
sidebar_recorded_message = _('The form has been recorded on %(date)s with the number %(number)s.')
sidebar_recorded_message = _('The form has been recorded on %(date)s with the number %(identifier)s.')
Author
Owner

Pour les demandes, pas d'identifiant personnalisé, on peut continuer à dire "numéro" dans les messages.

Pour les demandes, pas d'identifiant personnalisé, on peut continuer à dire "numéro" dans les messages.
wcs/carddata.py Outdated
@ -42,0 +42,4 @@
@classmethod
def get_by_id(cls, value):
try:
return cls.select([cls._formdef.get_by_id_criteria(value)], limit=1)[0]
Author
Owner

Sur un carddata, pour aller chercher selon l'id, on va passer par un critère qui sera déterminé au niveau du carddef.

En passant, on n'a pas de contrôle sur l'unicité, si ça se révèle un problème, on ajoutera quelque chose, mais je pense qu'on peut démarrer sans.

Sur un carddata, pour aller chercher selon l'id, on va passer par un critère qui sera déterminé au niveau du carddef. En passant, on n'a pas de contrôle sur l'unicité, si ça se révèle un problème, on ajoutera quelque chose, mais je pense qu'on peut démarrer sans.
wcs/carddef.py Outdated
@ -145,1 +145,4 @@
def get_by_id_criteria(self, value):
if self.id_template:
return Equal('id_display', value)
Author
Owner

S'il y a un gabarit pour l'id, on prend selon la colonne id_display.

S'il y a un gabarit pour l'id, on prend selon la colonne id_display.
wcs/carddef.py Outdated
@ -146,0 +153,4 @@
except ValueError:
# value not an integer, it could be id_display
return Equal('id_display', value)
return Equal('id', value)
Author
Owner

Sinon sur l'id. C'est du code qui existait plus bas, qui est déplacé.

Sinon sur l'id. C'est du code qui existait plus bas, qui est déplacé.
wcs/carddef.py Outdated
@ -248,3 +254,1 @@
criterias.append(Equal('id_display', get_by_id))
else:
criterias.append(Equal('id', get_by_id))
criterias.append(carddef.get_by_id_criteria(get_by_id))
Author
Owner

Voilà, le code était ici, il est maintenant plus haut.

Voilà, le code était ici, il est maintenant plus haut.
@ -385,3 +385,3 @@
return value
try:
carddata = carddef.data_class().get(value_id)
carddata = carddef.data_class().get_by_id(value_id)
Author
Owner

Pour les champs liste, on récupère selon l'identifiant qui peut être custom.

Pour les champs liste, on récupère selon l'identifiant qui peut être custom.
@ -491,1 +494,4 @@
# unless a specific template is defined, only set id_display once
# as it may have been set automatically by interpreting a webservice
# response.
fields['id_display'] = self.formdef.get_display_id_format().strip()
Author
Owner

Ce truc de poser le id_display en fonction d'un retour de webservice, c'est quelque chose de très particulier et normalement plus utilisé (à part à Montpellier, peut-être), bref on ne le fait plus non plus s'il y a un gabarit particulier.

Ce truc de poser le id_display en fonction d'un retour de webservice, c'est quelque chose de très particulier et normalement plus utilisé (à part à Montpellier, peut-être), bref on ne le fait plus non plus s'il y a un gabarit particulier.
wcs/sql.py Outdated
@ -1807,2 +1805,2 @@
carddef_table_alias,
)
if field.carddef.id_template:
carddef_table_decl = 'LEFT JOIN %s AS %s ON (%s.%s = %s.id_display)' % (
Author
Owner

S'il y a un gabarit personnalisé, on fait la jointure ainsi.

S'il y a un gabarit personnalisé, on fait la jointure ainsi.
wcs/sql.py Outdated
@ -1809,0 +1811,4 @@
carddef_table_alias,
)
else:
carddef_table_decl = 'LEFT JOIN %s AS %s ON (CAST(%s.%s AS INTEGER) = %s.id)' % (
Author
Owner

Et sinon on continue à CASTer en entier, pour le match sur l'id natif.

Et sinon on continue à CASTer en entier, pour le match sur l'id natif.
fpeters force-pushed wip/44604-card-id from 15728f9581 to 3169775deb 2023-11-27 14:59:21 +01:00 Compare
fpeters changed title from WIP: misc: allow setting a custom id template on cards (#44604) to misc: allow setting a custom id template on cards (#44604) 2023-11-27 15:15:57 +01:00
fpeters force-pushed wip/44604-card-id from 3169775deb to 632afd546c 2023-11-27 15:21:23 +01:00 Compare
fpeters reviewed 2023-11-27 15:22:32 +01:00
wcs/sql.py Outdated
@ -761,1 +761,3 @@
for attr in ('receipt_time', 'anonymised', 'user_id', 'status'):
attrs = ['receipt_time', 'anonymised', 'user_id', 'status']
if isinstance(formdef, CardDef):
attrs.append('id_display')
Author
Owner

On ajoute un index sur la colonne id_display, pour aider.

On ajoute un index sur la colonne id_display, pour aider.
lguerin reviewed 2023-11-27 17:13:27 +01:00
@ -47,0 +51,4 @@
return form
criterias = [
StrictNotEqual('status', 'draft'),
Null('anonymised'),
Owner

pourquoi ce critère ? On nettoie l'id custom lors de l'anonymisation ?

pourquoi ce critère ? On nettoie l'id custom lors de l'anonymisation ?
Author
Owner

En vrai sur le genre de modèle de fiches concerné ici on n'aura sans doute jamais d'anonymisation, je pense que mon idée là était que ce qui était anonymisé n'était plus utilisé, qu'on pouvait alors permettre de modifier le gabarit.

Je viens d'hésiter à mettre du code spécifique pour l'anonymisation, vider le id_display, mais ça complique de manière moche le .store() général, autant juste ici retirer ce critère. (j'ai mis un commit supplémentaire pour faire ça).

En vrai sur le genre de modèle de fiches concerné ici on n'aura sans doute jamais d'anonymisation, je pense que mon idée là était que ce qui était anonymisé n'était plus utilisé, qu'on pouvait alors permettre de modifier le gabarit. Je viens d'hésiter à mettre du code spécifique pour l'anonymisation, vider le id_display, mais ça complique de manière moche le .store() général, autant juste ici retirer ce critère. (j'ai mis un commit supplémentaire pour faire ça).
lguerin approved these changes 2023-11-27 17:39:47 +01:00
lguerin left a comment
Owner

ok pour moi, top !

ok pour moi, top !
fpeters force-pushed wip/44604-card-id from 9bcc25cc67 to 7d1e092c0f 2023-11-28 08:22:36 +01:00 Compare
fpeters merged commit 0d33387f0f into main 2023-11-28 08:59:37 +01:00
fpeters deleted branch wip/44604-card-id 2023-11-28 08:59:37 +01:00
pducroquet reviewed 2024-01-03 16:02:14 +01:00
@ -5219,6 +5231,7 @@ def migrate():
# 45 & 46: add index on formdata(status)
# 56: add GIN indexes to concerned_roles_array and actions_roles_array
# 74: (late migration) change evolution index to be on (fomdata_id, id)
# 97: add index on carddata/id_display
Owner

Petit "post mortem" : cette migration n'a jamais été exécutée à cause du if sql_level < 74 au dessus :/

Petit "post mortem" : cette migration n'a jamais été exécutée à cause du if sql_level < 74 au dessus :/
Author
Owner

Petit "post mortem" : cette migration n'a jamais été exécutée à cause du if sql_level < 74 au dessus :/

https://dev.entrouvert.org/issues/85348

> Petit "post mortem" : cette migration n'a jamais été exécutée à cause du if sql_level < 74 au dessus :/ → https://dev.entrouvert.org/issues/85348
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/wcs#49
No description provided.