saxix/django-adminactions

Files selected in merge UI are deleted and the opposite ones are saved instead

int-ua opened this issue · 8 comments

Opening this issue to discuss the problem that arose after fixing #95: images selected were not saved and discarded instead.

@saxix Why is API merge selects the other values and not master? Maybe we can flip it? I already did it in #124

AFAIU it needs more, at least a new major version because of API behaviour change. Maybe I missed something and it's possible to fix FileField/ImageField selection without this change?

Further improvement can include taking values from other in case it's null in master

saxix commented

the idea is he master is what you want to keep as PK and move the values from 'other'. After reading I double checked your PR and saw the changes in the tests. I think I'll going to revert the changes because we keep compatibility. Any thoughts on that ?

Yes, compatibility is important. But I don't see another way of fixing the problem with selecting images in HTML UI: whatever the user chooses is deleted and the other file is selected. Do I understand correctly that all selected by user values are stored in the master object? Also, IMHO, selecting other values, replacing all master fields with them and dropping original values works not in all cases. I guess the reasoning was that objects that are created later in time have more information, but in my case it's the opposite: we have a large number of objects that were created later and have much less info in their fields except for some new relations. Also, do I understand correctly that it's reverted to what is happening during merge with HTML UI, when master contains all field values that were explicitly selected by user?

Aside from the file selection problem: what do you think about adding optional argument to api.merge that will switch the behaviour?

saxix commented

master should be the object (pk) preserved and other is the pk deleted.
The final object will have the master's pk where the values depend by the user selection.

Hi, I noticed the same problem: merging two records, A and B. A is selected as a 'master', B is selected as 'other'. After merge I expect A to be preserved and B deleted, but get the opposite: A is getting deleted while B is preserved.
Just adding a comment here, so that I can see further updates on this thread :)

This isn't just for file fields -it is for all fields. There is an inconsistency with the UI and the API. The UI shows the merged object taking all of the Master values by default, and you can select fields from Other to use instead. As long as you select at least one field from other, it will work as expected. However, if you select no fields, the merge API will take all fields from other, instead of all fields from master.

If you select no fields to copy from Other, it returns None

def clean_field_names(self):
      if self.cleaned_data['field_names']:
          return self.cleaned_data['field_names'].split(',')
      else:
          return None

If ``fields`` is None ``master`` will get all the ``other`` values except primary_key.

Regardless of if you think Master or Other should be the default source of values, the UI needs to match the outcome - right now if you do not select any values from other, it will cause data loss. I would vote for taking all values from Master if fields is None.