8000 refactor: Replace PageAdmin.delete_view by two smaller methods by jrief · Pull Request #7995 · django-cms/django-cms · GitHub
[go: up one dir, main page]
Skip to content

refactor: Replace PageAdmin.delete_view by two smaller methods #7995

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Sep 14, 2024
Merged

refactor: Replace PageAdmin.delete_view by two smaller methods #7995

merged 4 commits into from
Sep 14, 2024

Conversation

jrief
Copy link
Contributor
@jrief jrief commented Sep 11, 2024 •
edited by fsbraun
Loading

Description

This does not change any functionality, it just replaces the copy & pasted & modified method PageAdmin.delete_view by two smaller methods provided by Django to be explicitly overridden for the given purposes.

Related resources

No related issues.

No functional changes, hence all tests run unmodified.

Checklist

  • I have opened this pull request against develop-4
  • I have added or modified the tests when changing logic
  • I have followed the conventional commits guidelines to add meaningful information into the changelog
  • I have read the contribution guidelines and I have joined the channel #pr-reviews on our Discord Server to find a “pr review buddy” who is going to review my pull request.

@jrief jrief requested a review from fsbraun September 11, 2024 08:39
Let pageadmin.has_change_permission(obj=None) return `page_permissi…
79561a3 
…ons.user_can_change_at_least_one_page` instead of `False.
fsbraun
fsbraun reviewed Sep 11, 2024
View reviewed changes
Copy link
Member
@fsbraun fsbraun left a comment

Choose a reason for hiding this comment

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

Great work, @jrief! I wonder if we can simplify this even more by making pageadmin.has_change_permissison a bit smarter. What do you think?

If you permit, I'd also love to add a new delete view template to organize the deleted pages more visually.

fsbraun
fsbraun approved these changes Sep 12, 2024
View reviewed changes
Copy link
Member
@fsbraun fsbraun left a comment

Choose a reason for hiding this comment

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

Looks good to me! @jrief Would you like to check the changes I made?

@fsbraun fsbraun merged commit cca00a5 into develop-4 Sep 14, 2024
34 checks passed
@vinitkumar vinitkumar deleted the refactor-delete_view branch September 14, 2024 12:58
fsbraun pushed a commit to fsbraun/django-cms that referenced this pull request Nov 5, 2024
fix: Remove Page object from admin index (introduced by django-cms#…
df805bc 
…7995)
@fsbraun fsbraun mentioned this pull request Nov 5, 2024
Merged
4 tasks
vinitkumar pushed a commit that referenced this pull request Dec 5, 2024
@fsbraun
fix: Remove Page object from admin index (introduced by #7995) (#8066)
fe54de4 
* fix: Remove `Page` object from admin index (introduced by #7995)

* Update cms/admin/pageadmin.py

* Update cms/admin/pageadmin.py

* Add test

---------

Co-authored-by: Github Release Action <info@django-cms.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fsbraun fsbraun fsbraun approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jrief @fsbraun
0