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

Bug fix where static files are removing if automated static files are turned off #14208

Merged
merged 2 commits into from Jan 24, 2019

Conversation

sdrenth
Copy link
Contributor

@sdrenth sdrenth commented Dec 21, 2018

This fixes the issue where old static files where removed even if the automate static files system setting was turned off for that element type.

What does it do?

I've added a system setting check that checks if static files are automated for the current element type.

Related issue(s)/PR(s)

Issue: #14206
PR that created the issue: #14135

@Jako Jako added bug The issue in the code or project, which should be addressed. area-core pr/review-needed Pull request requires review and testing. labels Dec 21, 2018
@Jako Jako added this to the v2.7.1 milestone Dec 21, 2018
@JoshuaLuckers
Copy link
Contributor

What happens if you select the wrong file by accident (like index.php), save it and change it to the correct file afterwards?

Is it possible to have those files move to a "trash folder" instead of deleting them from the filesystem immediately?

@sdrenth
Copy link
Contributor Author

sdrenth commented Dec 21, 2018

@JoshuaLuckers I don't think a trash folder is necessary. If you set up your static file structure correctly the source does not have access to files such as index.php

break;
}

return (bool) $this->xpdo->getOption('static_elements_automate_' . $type, null, false);
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is type is an empty string? In this case, all times will be called wrong setting. Make sense fetch setting value only when the type is defined.

core/model/modx/modelement.class.php Outdated Show resolved Hide resolved
@tolanych
Copy link
Contributor

I tested this PR (reproduced from #14206 (comment)) for setting static_elements_automate_chunks.

When this settings as Yes - previous static file removed from file system.
When this setting as No - previous static file stay.

Copy link
Contributor

@JoshuaLuckers JoshuaLuckers left a comment

Choose a reason for hiding this comment

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

I tested the following with static_elements_automate_* turned off:

  • If I change the static file for an element the old file is not deleted (good).
  • If I delete an element that uses a static file the static file is deleted (not good).

@Jako
Copy link
Collaborator

Jako commented Jan 12, 2019

@JoshuaLuckers Your issue should be fixed with #14228. But this PR covers another part.

@JoshuaLuckers JoshuaLuckers added pr/ready-for-merging Pull request reviewed and tested and ready for merging. and removed pr/review-needed Pull request requires review and testing. labels Jan 24, 2019
@JoshuaLuckers JoshuaLuckers self-assigned this Jan 24, 2019
@JoshuaLuckers JoshuaLuckers merged commit d1a3b51 into modxcms:2.x Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-core bug The issue in the code or project, which should be addressed. pr/ready-for-merging Pull request reviewed and tested and ready for merging.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants