django-cms / djangocms-moderation

Other
13 stars 20 forks source link

[BUG] Workflow admin view is breaking. SortableAdmin is not working as expected #268

Open vipulnarang95 opened 1 month ago

vipulnarang95 commented 1 month ago

Description

Workflow admin view is breaking. SortableAdmin is not working as expected Name and link to open workflow admin form is empty https://github.com/django-cms/djangocms-moderation/blob/master/djangocms_moderation/admin.py#L1014

Steps to reproduce

Go to django admin --> Moderations --> workflows click on Workflow and it is not displaying the name to open to workflow form

Expected behaviour

Actual behaviour

Screenshots

Additional information (CMS/Python/Django versions)

Do you want to help fix this issue?

joshyu commented 1 month ago

..

vipulnarang95 commented 1 month ago

Also removing the SortableInlineAdminMixin from WorkflowStepInline loads the admin view correctly in line https://github.com/django-cms/djangocms-moderation/blob/master/djangocms_moderation/admin.py#L1003

fsbraun commented 1 month ago

This comment might support your finding @vipulnarang95: https://django-admin-sortable2.readthedocs.io/en/latest/installation.html#upgrading-from-version-1

@joshyu Can you confirm @vipulnarang95's finding and that it solves the problem?

joshyu commented 1 month ago

@fsbraun , I removed both SortableAdminMixin and SortableInlineAdminMixin from inline admin and its parent class, It seems that the list rendered correctly, but I got an error while saving a new workflow.

vipulnarang95 commented 1 month ago

We need to modify forms at https://github.com/django-cms/djangocms-moderation/blob/master/djangocms_moderation/forms.py#L26 It would work fine then

joshyu commented 1 month ago

We need to modify forms at https://github.com/django-cms/djangocms-moderation/blob/master/djangocms_moderation/forms.py#L26 It would work fine then

@vipulnarang95 , how to update this form? Can you share more detail?

joshyu commented 1 month ago

@fsbraun,

I found that the error I mentioned was ever mentioned by Sergi of What.digital and he raised a PR for it.https://github.com/jrief/django-admin-sortable2/pull/371

vipulnarang95 commented 1 month ago

Hi @fsbraun @joshyu I have removed the dependency of django admin sortable2 from djangocms moderation. We have tested this internally and it works fine as expected, just the sort feature in inline is removed which is not used.

joshyu commented 1 month ago

Hi @fsbraun / @vipulnarang95,

From my understanding, the sorting functionality might be in the requirements list, so I think it's not a best solution only to remove the dependencies of django-admin-sortable2.

Whether can we create a new branch without/adminsortable based on master branch, this temporary solution to remove the dependency of adminsortable2 will be put there.

How do you think?

vipulnarang95 commented 1 month ago

From the authoring perspective this functionality of Sort is available just drag and drop feature is not available. If the problem is there in the dependent addon and for our release target we can get a dev release and fix it in later stage as this is not a blocker and the feature is also not used at all. @fsbraun please suggest the best possible way to deliver this.

fsbraun commented 1 month ago

I'd appreciate if we could keep the drag-and-drop sorting capability.

@jrief Can you give https://github.com/jrief/django-admin-sortable2/pull/371 a second look?

@vipulnarang95 @joshyu Is there a way of making a default_order_field not None in djangocms-moderation? And would this also fix the issue?

vipulnarang95 commented 1 month ago

@fsbraun I checked and used this change in django admin sortable2 in my local and it still doesn't resolves the issue. The issue still exists

Also the getattr error josh mentioned was coming after removing the SortableAdmin and SortableAdminMixin class. I am debugging django admin sortable2 now and will share my findings.

FreemanPancake commented 1 month ago

I'd appreciate if we could keep the drag-and-drop sorting capability.

@jrief Can you give jrief/django-admin-sortable2#371 a second look?

@vipulnarang95 @joshyu Is there a way of making a default_order_field not None in djangocms-moderation? And would this also fix the issue?

Hi @fsbraun , This was not a issue for default_order_field, as there is a init method called _get_default_ordering(), this will return and set the default_order_field and default_order_direction 's value as 'name'. The problem was on the get_list_display() method they just replace the name column with _reorder_, which I believe should be append to the original list_display list.

image

And I believe we should add attribute enable_sorting to desired Admin and set its value as True if we want to enable sorting function or False if we don't need this.