search

bacbos / bolt-menu-editor

a visual menu-editor extension for the awesome bolt cms
Other
21 stars 23 forks source link

Where place the backup folder? #24

Closed gander closed 8 years ago

gander commented 8 years ago

I was try:

Always got error:

Please make sure that there is a menueditor/backups folder or disable the backup-feature in config.yml

bacbos commented 8 years ago

Hi @gander The location of the backup folder is configurable in config.yml, the default is 'app/config/menu-backups'. The path is relative to the root folder of your bolt installation, so it seems that in your case it should be /menueditor/backups

Make sure that this folder exists and is writable by the user running your web-server (or the php-fpm process).

credomane commented 8 years ago

I'm having the same issue. Attempting to save a menu results in this obvious in-your-face overlay popup:

Unable to save backup! Please make sure menueditor/backups is writable by your webserver or disable the feature in the extensions configuration.

Which is NOT the path set in the config file at all. This lead me down a lot of wrong turns and wasted time. While the proper path is a tiny little yellow inline popup that appears during page loading then quickly fades after a few seconds. I didn't even notice it for quite a while because of it fading away.

When I check the real path used by the extension it becomes /absolute/path/to/bolt/.app/config/menu-backups That period before app in the path is "breaking" your extension so no matter what someone does the path is always wrong. This is occurring on a brand new install of bolt 2.2.20 with the first thing I did after completing install was to install bacboslab/menueditor - 2.2.4. This is probably OP's real issue as well.

This isn't a defect in your extension but some issue in bolt itself. Creating the .app/config/menu-backups works for a workaround. I had to literally add die($this->backupDir); to even figure out what was wrong because looking at your code said that /app/config/menu-backups was supposed to be the right place.

credomane commented 8 years ago

I played around some.

It would seem that $this->app['resources']->getPath('web') returns /absolute/path/to/bolt/. While $this->app['resources']->getPath('app') returns /absolute/path/to/bolt/app like what is expected/desired.

//For now I've changed
            $this->backupDir = $this->app['resources']->getPath('web') . $this->config['backupsFolder'];
//to this
            $this->backupDir = $this->app['resources']->getPath('app') . "/" . $this->config['backupsFolder'];
//Which gets everything working in the places they should

It seems as if web is mostly broken or discontinued? On a fresh bolt install with just this extension only your extension uses it.

SvanteRichter commented 8 years ago

@credomane which OS/PHP version are you on? I'm asking since I haven't seen the same issue on CentOS 6 and 7 with php 5.4, 5.6 and 7.0.X.

I'll take a look tomorrow to fix it, you are probably right that the web path is misused here, it was chosen for flexibility and because the install type I usually use.

credomane commented 8 years ago

Debian 8 and php 5.6.

I looked through the bolt source code and found a line that explicitly sets the value for web to blank doing a git-blame turns up this commit https://github.com/bolt/bolt/commit/a9f69e3c389df30d516b08f7cfee1de876ab6725 which appears to be used in bolt releases starting with bolt 2.0.0! The line in question is in the 80-90's. You can see where a lot of the default values for paths were edited in that commit.

My existing install of bolt on another pc (also debian 8 and php5.6) which has been around since bolt 1.6 or something like that currently running 2.2.20 as well doesn't seem to have this issue. I however, noticed that the enableBackups is true but the backupPath is commented out. I never changed the config file on the server. So poking about this extensions code and git-blaming for when the that line was uncommented I come across your commit, @SahAssar https://github.com/bacbos/bolt-menu-editor/commit/c7a85bd43e4b94c4c18058c7db3987ab125d4ca3#diff-5f16dc02790444c81044ef1d6e587930

My conclusion is that bolt 2.0.0 and newer do NOT work with fresh installs of this extension versions 2.2.2, 2.2.3 and 2.2.4. Your commit uncommented the backupsPath which changed the previous "hardcoded" default for the backupPath to go from the working /absolute/path/to/bolt/extensions/vendor/bacboslab/menueditor/backups/ to a now broken /absolute/path/to/bolt/.app/config/menu-backups

Gotta love the bugs that only appear for fresh installs. lol

[edit] If you are going to stick with this new backupPath you should probably make the extension attempt to create the expected folders.

SvanteRichter commented 8 years ago

@credomane Actually, if you don't uncomment backupsFolder it will stay the same as it was earlier, see https://github.com/bacbos/bolt-menu-editor/blob/master/Extension.php#L48-L52 and #17 which introduced it (intendit is my old work account, so it's still me to blame if it's wrong).

The reason for making backupsFolder customizable was that I share the same extensions dir across hundreds of sites. Most other extensions do not store site-specific info in the extension dir, so I thought it would be better to make it customizable while still keeping the default the same. Still, that was a while ago, and I would probably have done it better if I did it now.

I think we should probably just make the backup dir reside inside the app/config/extensions folder, which is where labels and other extensions store their non-config data.

I'll have a PR with a fix tomorrow, so hopefully this will all be resolved within a day :)

SvanteRichter commented 8 years ago

@credomane Shit, I just reread the part about the uncommenting... That is completely wrong (on my part), and I should not have committed that.

I am very sorry for that mistake and will fix it first thing tomorrow (hopefully along with the other fixes mentioned above).

credomane commented 8 years ago

No worries, my mentioning of the uncommenting was separated from the mention of the commit anyways...oops. Just glad I could help out.