8000 fix: X frame options added to page settings form by theShinigami · Pull Request #8041 · django-cms/django-cms · GitHub
[go: up one dir, main page]
Skip to content

fix: X frame options added to page settings form #8041

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 11 commits into from
Oct 25, 2024
Merged

fix: X frame options added to page settings form #8041

merged 11 commits into from
Oct 25, 2024

Conversation

theShinigami
Copy link
Contributor
@theShinigami theShinigami commented Oct 23, 2024 •
edited
Loading

Description

fix: display X-Frame Options settings under Page Settings

The X-Frame Options settings are now under Page Settings -> Headers.

fix: comment typo cms/page_rendering.py

minor typo 'middlewear' changed to 'middleware'.

Related resources

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.

github-actions[bot]
github-actions bot reviewed Oct 23, 2024
View reviewed changes
Copy link
@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello! Thank you for your contribution! 🎉

As it's your first contribution, be sure to check out the contribution docs.

We invite you to join us on our Discord Server!

Welcome aboard ⛵️!

@theShinigami theShinigami changed the title X frame options fix fix: X frame options fix Oct 23, 2024
@fsbraun fsbraun added needs to be backported Commits need to be backported 5.0 labels Oct 23, 2024
@fsbraun
Copy link
Member
fsbraun commented Oct 23, 2024

It would be great to add a simple test that ensures that all page properties moved to the page content model in #6477, actually are present in page settings. IHMO it would be sufficient to check if the moved fields are contained in the fieldsets.

Also, I noticed that the xframe_options field in PageContent does not have a verbose_name:

django-cms/cms/models/contentmodels.py

Lines 131 to 134 in 19ef774

xframe_options = models.IntegerField(
choices=X_FRAME_OPTIONS_CHOICES,
default=get_cms_setting('DEFAULT_X_FRAME_OPTIONS'),
)

This would be the perfect opportunity to add, say verbose_name = _("X Frame Options")

@fsbraun fsbraun changed the title fix: X frame options fix fix: X frame options added to page settings form Oct 23, 2024
@theShinigami
feat: add verbose_name to xframe_options
bc9dd83 
Adds a verbose_name attribute to the xframe_options field in PageContent model
@theShinigami
test: add tests for moving attributes from Page to PageContent
c4717d6 
Implemented tests to verify attributes moved from the Page model to the PageContent model.
@theShinigami
fix(migration): add verbose_name to xframe_options field
ba2de1a 
Updated migration file to include verbose_name for the xframe_option
fsbraun
fsbraun reviewed Oct 24, 2024
View reviewed changes
@fsbraun
Copy link
Member
fsbraun commented Oct 24, 2024

@theShinigami LGTM! Will test drive later today!

@fsbraun
Copy link
Member
fsbraun commented Oct 25, 2024 •
edited
Loading

@theShinigami I've done the test run. This looks good. One last (bookkeeping) request from my side:

Can you move the test from cms/tests/test_class_properties.py to cms/tests/test_admin.py? Before the AdminPageEditContentSizeTests class might be a good place. This way we keep admin form tests together...

@theShinigami
refactor(tests): moved tests
c58b0e4 
- Tests moved from test_class_properties.py to test_admin.py
@fsbraun fsbraun merged commit 1acb816 into django-cms:develop-4 Oct 25, 2024
51 checks passed
fsbraun added a commit that referenced this pull request Oct 25, 2024
@theShinigami @fsbraun
fix: X frame options added to page settings form (#8041)
4232297 
* fix: correct typo

* feat: add new form options for X-Frame Options

* feat: add verbose_name to xframe_options

Adds a verbose_name attribute to the xframe_options field in PageContent model

* test: add tests for moving attribu
9286
tes from Page to PageContent

Implemented tests to verify attributes moved from the Page model to the PageContent model.

* fix(migration): add verbose_name to xframe_options field

Updated migration file to include verbose_name for the xframe_option

* test: update tests for field presence in PageContent and ChangePageForm

* refactor: remove unused imports

* Revert "refactor: remove unused imports"

This reverts commit 6712895ff05a5ea25efdc9cf46a1c3056ff6c9b2.

* refactor: remove unused imports

* refactor(tests): moved tests

- Tests moved from test_class_properties.py to test_admin.py

---------

Co-authored-by: Fabian Braun <fsbraun@gmx.de>
@fsbraun fsbraun mentioned this pull request Oct 28, 2024
Merged
4 tasks
@fsbraun fsbraun added 4.1 and removed needs to be backported Commits need to be backported labels Nov 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions github-actions[bot] github-actions[bot] left review comments

@fsbraun fsbraun fsbraun approved these changes

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

Successfully merging this pull request may close these issues.

[BUG] X Frame Options Removed from Advanced Form
2 participants
@theShinigami @fsbraun
0