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.

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

Disallow deleting pages if still attached to menu node #2338

Merged
merged 3 commits into from
Jul 25, 2022
Merged

Disallow deleting pages if still attached to menu node #2338

merged 3 commits into from
Jul 25, 2022

Conversation

afdev82
Copy link
Contributor

@afdev82 afdev82 commented May 14, 2022 •
edited
Loading

What is this pull request for?

It changes the behavior on delete of the node table's page_id foreign key from cascade to restrict.
When the user wants to delete a page, if it still linked to any node, we show a warning and abort the deletion.

Closes #2301

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@afdev82
Copy link
Contributor Author

afdev82 commented May 14, 2022

Unfortunately, I was not able to fix the new tests that I've added.
In the browser, after I confirm the deletion, I'm redirected to the /admin/pages URL, but I don't know why in the tests I get:

Expected response to be a Turbolinks visit to <http://test.host/admin/pages> but was a visit to <http://test.host/admin/pages/2>.
Expected "http://test.host/admin/pages" to be === "http://test.host/admin/pages/2".

@github-actions
Copy link

This pull request has not seen any activiy in a long time.
Probably because of missing tests or a necessary rebase.
This PR will be closed in 7 days if no further activity happens.

@github-actions github-actions bot added the Stale label Jul 14, 2022
@afdev82 afdev82 mentioned this pull request Jul 14, 2022
Closed
@github-actions
Copy link

This pull request has not seen any activiy in a long time.
Probably because of missing tests or a necessary rebase.
Please open a new PR to latest main if you want to continue working on this.
Thanks for the contribution.

@github-actions github-actions bot closed this Jul 22, 2022
@mamhoff mamhoff reopened this Jul 22, 2022
@mamhoff
Copy link
Contributor

mamhoff commented Jul 22, 2022

Still valid

@afdev82
Copy link
Contributor Author

afdev82 commented Jul 22, 2022

I've fixed the tests, but I don't understand why I had to apply that changes.
As I said before, in the browser, after I confirm the deletion, I'm redirected to the /admin/pages URL.

@github-actions github-actions bot removed the Stale label Jul 23, 2022
@tvdeyen tvdeyen changed the title Fix #2301 Do not delete menu node when page gets destroyed Jul 25, 2022
@tvdeyen tvdeyen changed the title Do not delete menu node when page gets destroyed Disallow deleting pages if still attached to menu node Jul 25, 2022
tvdeyen
tvdeyen approved these changes Jul 25, 2022
View reviewed changes
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Thanks!

@tvdeyen tvdeyen merged commit 8a7627b into AlchemyCMS:main Jul 25, 2022
@afdev82 afdev82 deleted the fix_2301 branch July 25, 2022 12:57
tvdeyen pushed a commit that referenced this pull request Jul 25, 2022
@afdev82 @tvdeyen
Disallow deleting pages if still attached to menu node (#2338)
510769e 
Add foreign key on nodes to restrict on delete page_id
and add a warning on delete if a page is still linked to nodes.

Closes #2301
dbwinger pushed a commit to dbwinger/alchemy_cms that referenced this pull request Mar 20, 2023
@afdev82 @dbwinger
Disallow deleting pages if still attached to menu node (AlchemyCMS#2338)
9129305 
Add foreign key on nodes to restrict on delete page_id
and add a warning on delete if a page is still linked to nodes.

Closes AlchemyCMS#2301
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tvdeyen tvdeyen tvdeyen 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.

Unexpected behaviour: deleting a page removes entire menu branch
3 participants
@afdev82 @mamhoff @tvdeyen