Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Queries number optimalization + more #13

Closed
wants to merge 2 commits into from
Closed

Conversation

zlorf
Copy link

@zlorf zlorf commented Dec 9, 2013

Hi,

I've tried to find the One Truly Ultimate django app for model sorting, but none suited me completely. Yours was closest to ideal, so I decided to patch it.

I wonder why app should perform so many queries when saving order? Why it should call save() on every object (and save perform typically 2 queries for model)? Why not use update() instead?

Second problem: I don't want order field to be directly editable in admin. It can only confuse users. Instead, they should use drag&drop in changelist to set ordering.

So my proposition: add ORDERABLE_ORDER_EDITABLE setting which control whether sort_order is editable (by default it is, to provide backward compatibility). Moreover, if field is not editable, order saving use update optimization (maybe it can be used even in editable case, but for uneditable it's obviously safe).

Second commit add translation for column header and remove sorting option. Why? Because if someone (accidentally) reverse the sorting order and then do some drag&drop, he can be surprised that the order is just reversed from what he saw. Changelist order should not be alterable.

@maxpeterson
Copy link
Member

Thanks for the contribution, it is always good to know these libraries are of use.

To make the order field read only / not editable did you consider setting readonly_fields or fields on your OrderableAdmin subclass? We would normally use one of these as it gives the ability to set it at the mode / admin level.

The efficiency of the admin reorder_view could definitely be looked at. We have been trying to add tests to this app as we update it so I would like to get this view tested before we make any significant changes.

You have highlighted an issue with the sorting option on the field that had not been considered. Presumably the same issue exist if the users reorders the items based on another field? I am not sure what the best answer to this is, maybe it would be possible to restrict the drag and drop reordering to only be available when the items are ordered by the sort_order.

@zlorf
Copy link
Author

zlorf commented Dec 10, 2013

Ah yes, I forgot about readonly_fields option. I will test if it fits.

@jturnbull
Copy link
Contributor

Looks like this wasn't needed in the end

@jturnbull jturnbull closed this Jun 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants