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

Feat/django 42 compatible #162

Conversation

vipulnarang95
Copy link
Member

Added Django support 4.2 and Python 3.10
Dropped support for django<3.2 and python<3.8

@vipulnarang95 vipulnarang95 requested a review from Aiky30 March 13, 2024 09:57
@vipulnarang95
Copy link
Member Author

Hi @Aiky30 @kinkerl
Can you please help me review the test failures? Any help is appreciated

Copy link
Member

@Aiky30 Aiky30 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @vipulnarang95,

This is a FIL specific package and not a DCA supported package so I can only offer a favour personally here.

The test failures look to be something django admin related. To debug this I would recommend adding breakpoints in the test and tracing the code through to the point of failure to see what is actually happening underneath.

.github/workflows/test.yml Show resolved Hide resolved
CHANGELOG.rst Show resolved Hide resolved
tests/test_admin.py Outdated Show resolved Hide resolved
@vipulnarang95
Copy link
Member Author

vipulnarang95 commented Mar 13, 2024

Hi @Aiky30, Thankyou for the favor. Much appreciated.. I fixed major of them.

Only one test is failing now.
Can you please suggest something on this?
FAIL: test_menuitem_changelist_custom_js (tests.test_admin.MenuItemAdminChangeListViewTestCase)
Check that custom javascript for overriding treebeard js is present.

Traceback (most recent call last):
File "/Users/vipul/Desktop/django/djangocms-navigation/tests/test_admin.py", line 1162, in test_menuitem_changelist_custom_js
self.assertContains(
File "/Users/vipul/Desktop/django/djangocms-navigation/venv/lib/python3.10/site-packages/django/test/testcases.py", line 660, in assertContains
self.assertTrue(
AssertionError: False is not true : Couldn't find 'djangocms_navigation/js/navigation-tree-admin.js' in response

@FreemanPancake
Copy link

Hi @vipulnarang95 , this test request does not response the html with your desired js file: navigation-tree-admin.js reference indeed. the change_list.html seems not using the correct template tag to load the template tag into the html.

@vipulnarang95
Copy link
Member Author

Hi @Aiky30 , Need your favor in this. The test which I mentioned above is failing as the treebeard template tag is not working as expected and the custom js is not find in the response. Would you be able to suggest something?

@joshyu
Copy link
Member

joshyu commented Mar 17, 2024

@vipulnarang95 , I have fixed your errors, please check my PR : vipulnarang95#2

@Aiky30 , please also help to review my solution.

https://github.com/divio/djangocms-text-ckeditor/tarball/master#egg=djangocms-text-ckeditor
https://github.com/django-cms/djangocms-versioning/tarball/support/django-cms-4.0.x#egg=djangocms-versioning
https://github.com/joshyu/djangocms-version-locking/tarball/feat/django-42-compatible#egg=djangocms-version-locking
https://github.com/FidelityInternational/djangocms-moderation/tarball/feature/django-42-compat#egg=djangocms-moderation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vipulnarang95 vipulnarang95 merged commit ecca22d into FidelityInternational:master Mar 22, 2024
8 checks passed
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.

4 participants