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

remove check for statusSystemID and statusSystem in module ProcessPageClone #1258

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

FdeQuillettes
Copy link

With the proposed change to Pages::___clone(), having statusSystemID or statusSystem on a page should no longer block the cloning of a page/tree, so its check can be removed from ProcessPageClone.

removed check for statusSystemID, clone doesn't affect the id or delete the page
There is no status that technically denies cloning, but statusSystemID and statusSystem prevent cloning in the current implmentation. Therefore during the cloning, status should be 1 for the newly cloned page. When the actual cloning is done, the original status can be restored.
When updating the pages_parents table, all items referencing this page were removed but not necessarily readded, resulting in queries that used has_parent not working correctly
@FdeQuillettes
Copy link
Author

Note: the last commit (7834e49) fixes another issue with clone: When updating the pages_parents table, all items referencing this page were removed but not necessarily readded, resulting in queries that used has_parent not working correctly.

@ryancramerdesign
Copy link
Owner

Thanks @FdeQuillettes, the preventing of system page cloning was intended, though to be honest I'm not sure if the reason is still applicable, so will look into it. Which system page are you needing to clone?

Regarding the pages_parents table issue–good find. We actually fixed this a couple of weeks ago on the dev branch. Are you using the dev branch, and if so, were you still experiencing the pages_parents table issue? Thanks.

@FdeQuillettes
Copy link
Author

Actually we only needed this for statusSystemID (for the multisite plugin we set our site roots to statusSystemID)., but when working on the fix I noticed the same may also apply to statusSystem.

Good to hear the pages_parents is already fixed on the dev branch, I was using the master branch (sorry, pretty new to git/github). We haven't had the problem anymore with the fix I made (which is essentially the same as yours I see, except I moved the check down instead of up) so all should be in order.

edit: actually I noticed that in the dev branch the numChildren check is still after the DELETE and before the INSERT, so which fix do you mean? We have applied the fix suggested in 'has_parent buggy' #1195 (40441f6), but the problem still persisted until I did the above fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants