Apply suggestions from code review

Co-authored-by: Raphael Michel <michel@rami.io>

doc/development/api/datasync.rst

@@ -4,6 +4,8 @@
 Data sync providers
 ===================
 
+.. warning:: This feature is considered **experimental**. It might change at any time without prior notice.
+
 pretix provides connectivity to many external services through plugins. A common requirement
 is unidirectionally sending (order, customer, ticket, ...) data into external systems.
 The transfer is usually triggered by signals provided by pretix core (e.g. ``order_created``),
@@ -35,14 +37,15 @@ within it has to call `MyListSyncProvider.enqueue_order` to enqueue the order fo
 
 
 Furthermore, most of these plugins need to translate data from some pretix objects (e.g. orders)
-into an external systems' data structures. Sometimes, there is only one reasonable way or the
+into an external system's data structures. Sometimes, there is only one reasonable way or the
 plugin author makes an opinionated decision what information from which objects should be
 transferred into which data structures in the external system.
 
 Otherwise, you can use a ``PropertyMappingFormSet`` to let the user set up a mapping from pretix model fields
 to external data fields. You could store the mapping information either in the event settings, or in a separate
 data model. Your implementation of :func:`OutboundSyncProvider.mappings`
-needs to provide a list of mappings, with at least the properties defined in
+needs to provide a list of mappings, which can be e.g. static objects or model instances, as long as they
+have at least the properties defined in
 :class:`pretix.base.datasync.datasync.StaticMapping`.
 
 .. code-block:: python
@@ -59,7 +62,7 @@ Currently, we support ``Order`` and ``OrderPosition`` as data sources, with the
 :func:`pretix.base.datasync.sourcefields.get_data_fields`.
 
 To perform the actual sync, implement ``sync_object_with_properties`` and optionally
-``finalize_sync_order``. The former is called for each object to be created, according to the ``mappings``:
+``finalize_sync_order``. The former is called for each object to be created according to the ``mappings``.
 For each order that was enqueued using ``enqueue_order``:
 
 - each Mapping with ``pretix_model == "Order"`` results in one call to `sync_object_with_properties`,
@@ -70,10 +73,12 @@ For each order that was enqueued using ``enqueue_order``:
 
 For example implementations, see the test cases in :package:``tests.base.test_datasync``.
 In :class:`SimpleOrderSync`, a basic data transfer of order data only is
-shown. Therein, a ``sync_object_with_properties`` method is defined like as follows:
+shown. Therein, a ``sync_object_with_properties`` method is defined as follows:
 
 .. code-block:: python
 
+    from pretix.base.datasync.utils import assign_properties
+
     def sync_object_with_properties(
             self, external_id_field, id_value, properties: list, inputs: dict,
             mapping, mapped_objects: dict, **kwargs,
@@ -90,7 +95,7 @@ shown. Therein, a ``sync_object_with_properties`` method is defined like as foll
         # We use the helper function ``assign_properties`` to update a pre-existing object.
         update_values = assign_properties(
             new_values=properties,
-            old_vlaues=pre_existing_object or {},
+            old_values=pre_existing_object or {},
             is_new=pre_existing_object is None
         )
 
@@ -118,6 +123,16 @@ shown. Therein, a ``sync_object_with_properties`` method is defined like as foll
             "my_result": result,
         }
 
+.. note:: The result dictionaries of earlier invocations of `sync_object_with_properties` are 
+          only provided in subsequent calls of the same sync run, such that a mapping can 
+          refer to e.g. the external id of an object created by a preceding mapping. 
+          However, the result dictionaries are currently not provided across runs. This will 
+          likely change in a future revision of this API, to allow easier integration of external 
+          systems that do not allow retrieving/updating data by a pretix-provided key.
+
+ `mapped_objects` is a dictionary of lists of dictionaries. The keys to the dictionary are 
+ the mapping identifiers (`mapping.id`), the lists contain the result dictionaries returned 
+ by `sync_object_with_properties`.
 
 In :class:`OrderAndTicketAssociationSync`, an example is given where orders, order positions,
 and the association between them are transferred.

src/pretix/base/datasync/datasync.py

@@ -245,6 +245,9 @@ class OutboundSyncProvider:
                     sq.in_flight_since = now()
                     sq.save()
                 except DatabaseError:
+                    # Either select_for_update failed to lock the row, or we couldn't set in_flight
+                    # as this order is already in flight (UNIQUE violation). In either case, we ignore
+                    # this order for now.
                     continue
                 try:
                     mapped_objects = self.sync_order(sq.order)

src/pretix/base/models/datasync.py

@@ -51,7 +51,7 @@ class OrderSyncQueue(models.Model):
     not_before = models.DateTimeField(blank=False, null=False, db_index=True)
     need_manual_retry = models.CharField(blank=True, null=True, max_length=20, choices=[
         ('exceeded', _('Temporary error, auto-retry limit exceeded')),
-        ('permanent', _('Misconfiguration, please check provider settings')),
+        ('permanent', _('Provider reported a permanent error')),
         ('config', _('Misconfiguration, please check provider settings')),
         ('internal', _('System error, needs manual intervention')),
         ('timeout', _('System error, needs manual intervention')),

src/pretix/control/templates/pretixcontrol/datasync/control_order_info.html

@@ -15,12 +15,12 @@
                     {% csrf_token %}
                     {% if pending %}
                         {% if pending.not_before > now or pending.need_manual_retry %}
-                        <button type="submit" name="run_job_now" value="{{ pending.pk }}" class="btn btn-default"><i class="fa fa-refresh"></i> {% trans "Retry now" %}</button>
+                            <button type="submit" name="run_job_now" value="{{ pending.pk }}" class="btn btn-default"><i class="fa fa-refresh"></i> {% trans "Retry now" %}</button>
                         {% endif %}
                         <button type="submit" name="cancel_job" value="{{ pending.pk }}" class="btn btn-danger"><i class="fa fa-times"></i> {% trans "Cancel" %}</button>
                     {% else %}
-                            <button type="submit" class="btn btn-default"><i class="fa fa-refresh"></i> {% trans "Sync now" %}</button>
-                            <input type="hidden" name="queue_sync" value="true">
+                        <button type="submit" class="btn btn-default"><i class="fa fa-refresh"></i> {% trans "Sync now" %}</button>
+                        <input type="hidden" name="queue_sync" value="true">
                     {% endif %}
                 </form>
                 <p><b>{{ display_name }}</b></p>
@@ -33,7 +33,8 @@
                             <i class="fa fa-warning"></i>
                             {% blocktrans trimmed with num=pending.failed_attempts max=pending.max_retry_attempts %}
                             Error. Retry {{ num }} of {{ max }}.
-                            {% endblocktrans %}{% if pending.not_before %}
+                            {% endblocktrans %}
+                            {% if pending.not_before %}
                                 {% blocktrans trimmed with datetime=pending.not_before|date:"SHORT_DATETIME_FORMAT" %}
                                 Waiting until {{ datetime }}
                                 {% endblocktrans %}

src/pretix/control/templates/pretixcontrol/datasync/failed_jobs.html

@@ -65,8 +65,8 @@
             <tfoot>
                 <tr>
                     <td colspan="5">
-                        <button type="submit" name="action" value="retry" class="btn btn-primary"><i class="fa fa-refresh"></i> Retry selected</button>
-                        <button type="submit" name="action" value="cancel" class="btn btn-danger"><i class="fa fa-times"></i> Cancel selected</button>
+                        <button type="submit" name="action" value="retry" class="btn btn-primary"><i class="fa fa-refresh"></i> {% trans "Retry selected" %}</button>
+                        <button type="submit" name="action" value="cancel" class="btn btn-danger"><i class="fa fa-times"></i> {% trans "Cancel selected" %}</button>
                     </td>
                     {% if staff_session %}
                         <td></td><td></td>

src/pretix/control/views/datasync.py

@@ -129,6 +129,8 @@ class GlobalFailedSyncJobsView(AdministratorPermissionRequiredMixin, FailedSyncJ
 
 
 class OrganizerFailedSyncJobsView(OrganizerPermissionRequiredMixin, FailedSyncJobsView):
+    permission = "can_change_organizer_settings"
+
     def get_queryset(self):
         return super().get_queryset().filter(
             event__organizer=self.request.organizer