rdmorganiser / rdmo

A tool to support the planning, implementation, and organization of research data management.
https://rdmorganiser.github.io
Apache License 2.0
105 stars 49 forks source link

feat(interview): Value signals [6] #1190

Open jochenklar opened 2 weeks ago

jochenklar commented 2 weeks ago

This PR is a small spin-of from https://github.com/rdmorganiser/rdmo/pull/1188, because I would like to have some feedback on the idea.

The PR adds 3 custom django signals value_created, value_deleted, and value_updated, which are fired when Values are created, updated or deleted through the interface. The front-end provides the API with the page the user in on, and this can be used in a handler to get all attributes on the current page like this:

@receiver(value_created, sender=Value)
def value_created_handler(sender, request, instance, **kwargs):
    page = Page.objects.filter(id=request.GET.get('page')).first()
    print('value_created', instance, page)

@receiver(value_updated, sender=Value)
def value_updated_handler(sender, request, instance, **kwargs):
    page = Page.objects.filter(id=request.GET.get('page')).first()
    print('value_updated', instance, page)

@receiver(value_deleted, sender=Value)
def value_deleted_handler(sender, request, instance, **kwargs):
    page = Page.objects.filter(id=request.GET.get('page')).first()
    print('value_deleted', instance, page)

The main benefit is that plugins/custom django apps could use them to manipulate answers on the same page without complicated queries.

MyPyDavid commented 2 weeks ago

could it give race conditions when I click a lot of checkboxes or another user is simultaneously clicking the same checkboxes? For your example I guess not right, since it only gets the page ??

jochenklar commented 2 weeks ago

The signal is fired when the value is actually saved, so no additional race conditions should emerge (I think). The value itself is in instance in my example. If a value is changed by a handler and somebody else wants to edit the same value, the same ValidationError would happen as if two people would change values at the same time.

m6121 commented 6 days ago

Looks good to me. Interview seems to work. Did not test it with custom signals, yet.

jochenklar commented 4 days ago

An idea by @MarcoReidelbach: the front-end can also provide set_prefix, set_index, collection_index, and set_collection.

jochenklar commented 4 days ago

Maybe we also add those explicetly to the signal so that they don't need to be collected from the request (which should stay nevertheless, I guess).