Allow OR matching in addition to AND matching #29

New issue

Closed

opened 2021-08-09 19:42:39 +02:00 by hansegucker · 7 comments

hansegucker commented

2021-08-09 19:42:39 +02:00

Owner

Currently, the importer only supports matching persons by a combination of fields. If we use email and import_ref_csv, the importer will find only persons with the same email AND the same import reference. But for use at the Katharineum, we need an option to find persons with the same email OR the same import reference.

Here is a simple patch which removes the AND mode for a OR mode, but there are much better options to solve this:

--- ldap_sync.py	2021-08-09 19:26:52.140242913 +0200
+++ production/lib/python3.9/site-packages/aleksis/apps/ldap/util/ldap_sync.py	2021-08-09 19:35:55.465769996 +0200
@@ -6,7 +6,7 @@
 from django.conf import settings
 from django.core.files import File
 from django.db import DataError, IntegrityError, transaction
-from django.db.models import fields
+from django.db.models import fields, Q
 from django.db.models.fields.files import FileField
 from django.utils.text import slugify
 from django.utils.translation import gettext as _
@@ -267,11 +267,17 @@
             if missing_key not in matches:
                 defaults[missing_key] = getattr(user, missing_key)
 
-        if get_site_preferences()["ldap__create_missing_persons"]:
-            person, created = Person.objects.get_or_create(**matches, defaults=defaults)
-        else:
-            person = Person.objects.get(**matches)
+        person = None
+        q = Q()
+        for key, value in matches.items():
+            q = q | Q(**{key: value})
+        try:
+            person = Person.objects.get(q)
             created = False
+        except Person.DoesNotExist:
+            if get_site_preferences()["ldap__create_missing_persons"]:
+                person = Person.objects.create(**matches, **defaults)
+                created = True
 
         person.user = user
         status = "New" if created else "Existing"

This has a high priority for @fph.

Currently, the importer only supports matching persons by a combination of fields. If we use `email` and `import_ref_csv`, the importer will find only persons with the same email **AND** the same import reference. But for use at the Katharineum, we need an option to find persons with the same email **OR** the same import reference. Here is a simple patch which removes the AND mode for a OR mode, but there are much better options to solve this: ```diff --- ldap_sync.py 2021-08-09 19:26:52.140242913 +0200 +++ production/lib/python3.9/site-packages/aleksis/apps/ldap/util/ldap_sync.py 2021-08-09 19:35:55.465769996 +0200 @@ -6,7 +6,7 @@ from django.conf import settings from django.core.files import File from django.db import DataError, IntegrityError, transaction -from django.db.models import fields +from django.db.models import fields, Q from django.db.models.fields.files import FileField from django.utils.text import slugify from django.utils.translation import gettext as _ @@ -267,11 +267,17 @@ if missing_key not in matches: defaults[missing_key] = getattr(user, missing_key) - if get_site_preferences()["ldap__create_missing_persons"]: - person, created = Person.objects.get_or_create(**matches, defaults=defaults) - else: - person = Person.objects.get(**matches) + person = None + q = Q() + for key, value in matches.items(): + q = q | Q(**{key: value}) + try: + person = Person.objects.get(q) created = False + except Person.DoesNotExist: + if get_site_preferences()["ldap__create_missing_persons"]: + person = Person.objects.create(**matches, **defaults) + created = True person.user = user status = "New" if created else "Existing" ``` This has a high priority for @fph.

hansegucker commented

2021-08-09 19:42:39 +02:00

Author

Owner

assigned to @nik

hansegucker commented

2021-09-07 19:22:31 +02:00

Author

Owner

@nik Is there any progress?

nik commented

2021-09-11 10:33:47 +02:00

Owner

No, there is no progress until now.

nik commented

2021-09-11 10:35:29 +02:00

Owner

Moved to a late release, as this is not a bug, but a feature request, and a very complex one. We will need a complete boolean logic parser of some sort to implement this.