getkirby / kirby

Kirby's core application folder
https://getkirby.com
Other
1.32k stars 168 forks source link

Folders end up with the same sorting number on error #2512

Closed hdodov closed 3 years ago

hdodov commented 4 years ago

Describe the bug
I have 1_blog in my content and _drafts/test. I tried to make the test page public and got the error:

rename(C:\Projects\my-project\content/1_blog,C:\Projects\my-project/content/2_blog): Access is denied. (code: 5)

I don't know why the error occurred, but the main problem is that after the error, I had 1_blog and 1_test, i.e. two pages with the same sorting number.

Expected behavior
I would expect the draft to remain a draft and the published page to be the only one at position 1.

Kirby Version
3.3.4

afbora commented 4 years ago

I think this is not related to Kirby, but about read and writing permissions in Windows. You can find several solutions by doing a search on Google: Windows Access is denied. (code: 5)

hdodov commented 4 years ago

The problem was that I was running Webpack in watch mode and it was monitoring the content files. However, I disagree about the issue not being related to Kirby. I want to make the test page first and therefore, the blog second. Kirby fails to do that and it should back to the previous state. In the Panel, I'm left with the information that test is still a draft when in fact it's not. That's unexpected behavior for which Kirby is responsible.

distantnative commented 4 years ago

Agreed, Kirby should catch errors during moving folders. I think it does, my guess is that the move from draft to 1_test was successful, but then moving 1_blog to 2_blog failed and it recovered by restoring 1_blog.

Ideally, Kirby would restore the whole chain of actions. But I am wondering: is it maybe that exactly the permissions to restore those were missing? E.g. your file system grants you to move formt he drafts folder to the main one, but inside the main one it doesn't grant you permissions to delete folders. So Kirby can't restore any previous state... Tricky.

lukasbestle commented 4 years ago

Does Kirby try to recover anything in the current implementation? My guess is that moving the blog directory failed completely and Kirby just stopped doing anything at this point, so the broken intermediate state was the result.

distantnative commented 4 years ago

@lukasbestle My bad. Wishful thinking that changed my memory 😅

distantnative commented 3 years ago

What I am wondering: why does moving _drafts/test to 1_test work but then moving 1_blog to 2_blog doesn't?

afbora commented 3 years ago

@hdodov I would try renaming (running single php with rename() method) the folder where I got the error by running it with PHP outside of Kirby to see if the error is on the system or in kirby.

Also, if the file in the folder is used by an application, it does not allow you to change it.

lukasbestle commented 3 years ago

There can always be file system issues, maybe some files are open and therefore locked. So even if it‘s not Kirby‘s fault, Kirby should ideally handle a full rollback. That‘s complex to implement though.

bastianallgeier commented 3 years ago

I think we have to be honest at this point. We won't be able to solve this. If you find a good solution, please let us know.