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

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

Open
1 of 2 tasks
vipulnarang95 opened this issue May 10, 2024 · 13 comments
Open
1 of 2 tasks

Comments

@vipulnarang95
Copy link

vipulnarang95 commented May 10, 2024

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?

  • Yes, I want to help fix this issue and I will join #workgroup-pr-review on Slack to confirm with the community that a PR is welcome.
  • No, I only want to report the issue.
@joshyu
Copy link
Contributor

joshyu commented May 10, 2024

..

@vipulnarang95
Copy link
Author

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
Copy link
Sponsor Member

fsbraun commented May 10, 2024

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

  • Remove SortableInlineAdminMixin from WorkflowStepInline
  • Ensure it is added to all admin classes using WorkflowStepInline, such as WorkflowAdmin

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

@joshyu
Copy link
Contributor

joshyu commented May 10, 2024

@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
Copy link
Author

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
Copy link
Contributor

joshyu commented May 10, 2024

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
Copy link
Contributor

joshyu commented May 13, 2024

@fsbraun,

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

@vipulnarang95
Copy link
Author

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
Copy link
Contributor

joshyu commented May 14, 2024

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
Copy link
Author

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
Copy link
Sponsor Member

fsbraun commented May 14, 2024

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?

@vipulnarang95
Copy link
Author

@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
Copy link
Contributor

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.

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

No branches or pull requests

4 participants