entrouvert/wcs
entrouvert
/
wcs
14
1
Fork 0
Code Issues Pull Requests 34 Releases Activity

data_sources: implicitly support users' uuid as valid id value (#83752) #852

Merged
pmarillonnet merged 1 commits from wip/83752-users-data-sources-accept-uuid-as-identifier-value into main 2023-12-04 15:19:22 +01:00
Conversation 2 Commits 1 Files Changed 3 +188 -9
pmarillonnet commented 2023-11-21 12:02:50 +01:00
Owner
Copy Link
No description provided.
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from 4a8ede9d42 to b3ca1a2383 2023-11-21 12:14:14 +01:00 Compare
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from b3ca1a2383 to 3be9937341 2023-11-21 12:25:52 +01:00 Compare
fpeters reviewed 2023-11-21 13:18:00 +01:00
tox.ini Outdated
@ -1,6 +1,6 @@
[tox]
toxworkdir = {env:TMPDIR:/tmp}/tox-{env:USER}/wcs/{env:BRANCH_NAME:main}/{env:EXECUTOR_NUMBER:}
envlist = py3-django32-codestyle-coverage, pylint
envlist = py3-django32{,-codestyle-coverage}, pylint
fpeters commented 2023-11-21 13:11:05 +01:00
Owner
Copy Link

Sans doute pas de rapport.

Sans doute pas de rapport.
pmarillonnet commented 2023-11-21 14:59:17 +01:00
Author
Owner
Copy Link

Yes, une modif locale versionnée par mégarde, c’est retiré.

Yes, une modif locale versionnée par mégarde, c’est retiré.
wcs/data_sources.py Outdated
@ -1106,3 +1109,4 @@
if str(item['id']) == str(option_id):
value = item
break
elif users_source and (uuid := item.get('uuid')) and uuid == option_id:
fpeters commented 2023-11-21 13:17:49 +01:00
Owner
Copy Link

J'aurais plutôt donné son propre elif self.type == 'wcs.users' ici, qui serait directement allé chercher la bonne ligne dans la db,

    value = get_publisher().user_class.get_user_with_roles(option_id, included_roles=..., excluded_roles=...)
    # check par rapport à include_disabled_users
    return value
J'aurais plutôt donné son propre `elif self.type == 'wcs.users'` ici, qui serait directement allé chercher la bonne ligne dans la db, ``` value = get_publisher().user_class.get_user_with_roles(option_id, included_roles=..., excluded_roles=...) # check par rapport à include_disabled_users return value ```
pmarillonnet commented 2023-11-21 14:20:19 +01:00
Author
Owner
Copy Link

J'aurais plutôt donné son propre elif self.type == 'wcs.users' ici, qui serait directement allé chercher la bonne ligne dans la db,

    value = get_publisher().user_class.get_user_with_roles(option_id, included_roles=..., excluded_roles=...)
    # check par rapport à include_disabled_users
    return value

C’est déjà précisément ce que fait, dans le cas self.type == 'wcs:users', get_structured_items qu’on appelle ici, non ?

    if data_source.get('type') == 'wcs:users':
        users = get_publisher().user_class.get_users_with_roles(
            included_roles=data_source.get('included_roles'),
            excluded_roles=data_source.get('excluded_roles'),
            order_by='name',
        )

        include_disabled_users = data_source.get('include_disabled_users')

        # […] def get_dict(u): […]

        return [get_dict(u) for u in users if u.is_active or include_disabled_users]

pas compris ce que ça apporterait de doublonner/déplacer le code directement dans get_structured_value :/

> J'aurais plutôt donné son propre `elif self.type == 'wcs.users'` ici, qui serait directement allé chercher la bonne ligne dans la db, > > ``` > value = get_publisher().user_class.get_user_with_roles(option_id, included_roles=..., excluded_roles=...) > # check par rapport à include_disabled_users > return value > ``` C’est déjà précisément ce que fait, dans le cas `self.type == 'wcs:users'`, get_structured_items qu’on appelle ici, non ? ```python if data_source.get('type') == 'wcs:users': users = get_publisher().user_class.get_users_with_roles( included_roles=data_source.get('included_roles'), excluded_roles=data_source.get('excluded_roles'), order_by='name', ) include_disabled_users = data_source.get('include_disabled_users') # […] def get_dict(u): […] return [get_dict(u) for u in users if u.is_active or include_disabled_users] ``` pas compris ce que ça apporterait de doublonner/déplacer le code directement dans get_structured_value :/
fpeters commented 2023-11-22 11:20:11 +01:00
Owner
Copy Link

Actuellement le code fait la récupération de tous les utilisateurs (filtrés) pour dedans chercher celui avec le bon id/uuid; dans ma proposition j'introduis "get_user_with_roles(option_id, ...)" (user au singulier, paramètre option_id) qui passerait dans la requête SQL également l'id/uuid recherché, pour obtenir un seul résultat.

Actuellement le code fait la récupération de tous les utilisateurs (filtrés) pour dedans chercher celui avec le bon id/uuid; dans ma proposition j'introduis "get_user_with_roles(option_id, ...)" (user au singulier, paramètre option_id) qui passerait dans la requête SQL également l'id/uuid recherché, pour obtenir un seul résultat.
pmarillonnet commented 2023-11-22 17:02:07 +01:00
Author
Owner
Copy Link

Actuellement le code fait la récupération de tous les utilisateurs (filtrés) pour dedans chercher celui avec le bon id/uuid; dans ma proposition j'introduis "get_user_with_roles(option_id, ...)" (user au singulier, paramètre option_id) qui passerait dans la requête SQL également l'id/uuid recherché, pour obtenir un seul résultat.

Ok, mes excuses, je pensais qu’il s’agissait d’une référence à une fonction existante (en l’occurrence get_users_with_roles avec les rôles inclus et exclus) à laquelle il aurait fallu recourir ici. Quiproquo.
Je revois ma copie.

> Actuellement le code fait la récupération de tous les utilisateurs (filtrés) pour dedans chercher celui avec le bon id/uuid; dans ma proposition j'introduis "get_user_with_roles(option_id, ...)" (user au singulier, paramètre option_id) qui passerait dans la requête SQL également l'id/uuid recherché, pour obtenir un seul résultat. Ok, mes excuses, je pensais qu’il s’agissait d’une référence à une fonction existante (en l’occurrence `get_users_with_roles` avec les rôles inclus et exclus) à laquelle il aurait fallu recourir ici. Quiproquo. Je revois ma copie.
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from 3be9937341 to 3634237de2 2023-11-21 13:55:40 +01:00 Compare
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from 3634237de2 to 6ef563c333 2023-11-21 14:16:53 +01:00 Compare
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from 6ef563c333 to 8b06f3cf05 2023-11-21 15:02:31 +01:00 Compare
pmarillonnet changed title from WIP: data_sources: implicitly support users' uuid as valid id value (#83752) to data_sources: implicitly support users' uuid as valid id value (#83752) 2023-11-21 15:02:43 +01:00
fpeters requested changes 2023-11-22 11:20:31 +01:00
fpeters left a comment
Owner
Copy Link

(cf commentaire)

(cf commentaire)
pmarillonnet changed title from data_sources: implicitly support users' uuid as valid id value (#83752) to WIP: data_sources: implicitly support users' uuid as valid id value (#83752) 2023-11-22 16:59:17 +01:00
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from 8b06f3cf05 to 429e5eda88 2023-11-30 16:18:44 +01:00 Compare
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from 76b2e812db to e1da5c80dc 2023-11-30 17:21:30 +01:00 Compare
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from e1da5c80dc to a4216cd50a 2023-11-30 17:24:31 +01:00 Compare
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from a4216cd50a to 681227c945 2023-11-30 18:55:40 +01:00 Compare
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from 681227c945 to 2f179bf547 2023-12-01 01:11:09 +01:00 Compare
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from 2f179bf547 to b9c67015e9 2023-12-01 01:19:26 +01:00 Compare
pmarillonnet changed title from WIP: data_sources: implicitly support users' uuid as valid id value (#83752) to data_sources: implicitly support users' uuid as valid id value (#83752) 2023-12-01 01:19:41 +01:00
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from b9c67015e9 to 7a51fd97f7 2023-12-01 02:00:58 +01:00 Compare
pmarillonnet requested review from fpeters 2023-12-01 08:39:54 +01:00
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from 7a51fd97f7 to b90ec28003 2023-12-01 09:57:08 +01:00 Compare
fpeters reviewed 2023-12-03 19:12:00 +01:00
fpeters left a comment
Owner
Copy Link

Deux petites modifications et ça sera ok.

Deux petites modifications et ça sera ok.
wcs/data_sources.py Outdated
@ -62,6 +62,14 @@ def register_data_source_function(function, function_name=None):
data_source_functions[function_name] = function
def get_dict(user):
fpeters commented 2023-12-03 19:04:11 +01:00
Owner
Copy Link

Je pense que ça serait mieux de la renommer, get_data_source_entry_from_user par exemple.

Je pense que ça serait mieux de la renommer, `get_data_source_entry_from_user` par exemple.
wcs/users.py Outdated
@ -192,0 +200,4 @@
else:
# what are the odds of a user name id being also a synctatically valid integer?
# let's include name ids here too:
criterias = [Or([Equal('id', int(user_id)), Intersects('name_identifiers', [user_id])])]
fpeters commented 2023-12-03 19:11:30 +01:00
Owner
Copy Link

Si jamais on se trouvait sur un entier, avec les nameid générés par authentic, il serait > 2**31 et ça ne passerait pas pour postgresql, je serais plutôt pour quelque chose de ce genre :

criterias = [Intersects('name_identifiers', [user_id])]  # user_id considered as name_id
try:
    if 0 < int(user_id) < 2**31:
        # user_id may refer to the id column
        criterias = [Or([Equal('id', int(user_id)), Intersects('name_identifiers', [user_id])])]
except ValueError:
    pass
Si jamais on se trouvait sur un entier, avec les nameid générés par authentic, il serait > 2**31 et ça ne passerait pas pour postgresql, je serais plutôt pour quelque chose de ce genre : ``` criterias = [Intersects('name_identifiers', [user_id])] # user_id considered as name_id try: if 0 < int(user_id) < 2**31: # user_id may refer to the id column criterias = [Or([Equal('id', int(user_id)), Intersects('name_identifiers', [user_id])])] except ValueError: pass ```
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from b90ec28003 to f070166521 2023-12-04 08:04:25 +01:00 Compare
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from f070166521 to 69e2dfc59a 2023-12-04 08:18:39 +01:00 Compare
pmarillonnet commented 2023-12-04 08:18:56 +01:00
Author
Owner
Copy Link

Deux petites modifications et ça sera ok.

Merci, c’est modifié.

> Deux petites modifications et ça sera ok. Merci, c’est modifié.
fpeters approved these changes 2023-12-04 14:31:26 +01:00
fpeters left a comment
Owner
Copy Link

J'ai relancé un build parce qu'erreur intermittente, mais à la fin d'un build victorieux, ça sera bon.

J'ai relancé un build parce qu'erreur intermittente, mais à la fin d'un build victorieux, ça sera bon.
pmarillonnet commented 2023-12-04 14:36:17 +01:00
Author
Owner
Copy Link

J'ai relancé un build parce qu'erreur intermittente, mais à la fin d'un build victorieux, ça sera bon.

Oui en effet erreur intermittente d’un truc qui n’a rien à voir, je n’avais pas vu que le dernier build était concerné. Merci.

> J'ai relancé un build parce qu'erreur intermittente, mais à la fin d'un build victorieux, ça sera bon. Oui en effet erreur intermittente d’un truc qui n’a rien à voir, je n’avais pas vu que le dernier build était concerné. Merci.
pmarillonnet force-pushed wip/83752-users-data-sources-accept-uuid-as-identifier-value from edda394eed to c3224721c4 2023-12-04 14:48:42 +01:00 Compare
pmarillonnet merged commit c3224721c4 into main 2023-12-04 15:19:22 +01:00
pmarillonnet deleted branch wip/83752-users-data-sources-accept-uuid-as-identifier-value 2023-12-04 15:19:22 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
fpeters
Labels
Clear labels
No items
No Label
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 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#852
No description provided.
Delete Branch "wip/83752-users-data-sources-accept-uuid-as-identifier-value"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?