+1 on this
My proposition was to have this feature activated by default. So, to display the “skip” checkbox by default. I’ll make it deactivated by default.
I’ll take into account the other observations and make a pass of the wording soon and update by initial post.
+1
After considering both options, introducing an XClass for the configuration of whether or not advanced users are allowed to choose to send documents to the recycle bin or delete them permanently is the best option.
- Allows to change the setting without restarting the server
- Easier to test
You will find below a screenshot of the new configuration screen.
The corresponding code will be placed in a new xwiki-platform-refactoring-ui module.
Do you agree to place this new menu in the “Others” section?
Do you find the title and hint of the property clear enough?
Thanks
Is it a real need? It’s quite a specific feature so personally I wouldn’t be chocked to have it only in xwiki.properties.
I don’t really understand this argument: for unit tests it should be a config API easy to mock, and for integration tests AFAIR we can specify custom properties, so it shouldn’t be an issue.
Not really, I would have put it in Editing I think.
The hint is a bit confusing IMO since you don’t have a clear information about what Yes stands for when reading it.
For the docker tests it is important, it allows us to perform all the tests related to the deletion of documents in a row.
It is also nicer from an UX point of view.
Agreed
What do you think of the sentences below?
When set to Yes, allows advanced users to choose whether they want to send documents to the recycle bin or delete them permanently when deleting a document. When set to No, documents are always sent to the Recycle Bin if it is available, otherwise they are always permanently deleted.
“Preferences” (plural) 
Do we have other refactoring options?
Seems ok.
Thanks!
I think we should always be able to make config changes without restarting the wiki, except when it’s just too technically complex (for example when it needs to init at startup time, like changing the document store or stuff like that).
This brings added value.
I don’t think it clutters the Admin if it’s well organized in the right categories.
Tests are users of XWiki so it’s interesting to listen to them. In this case they’re saying: you cannot modify the config at runtime. And indeed it’s true and it’s a need. Note that restarting a wiki requires an infra person and in lots of companies the wiki admin doesn’t have access to the infra.
I don’t think Editing is right. Maybe Content.
Personally I like a Refactoring section because I think there could be more options and the Content section is already quite full. Also it’s not the same level as other Content options which could be more important. That’s why I was asking whether we have other options already or not.
Seems we don’t, only one which is supposed to be removed ASAP (refactoring.rename.useAtomicRename = true).
I can imagine other options in the future related to the generic domain of Refactoring in a wiki.
Now the option to be able to send docs or not to the recycle bin doesn’t feel very “refactoring”. I wonder if the refactoring module was the right place for that… The fact that Simon and Marius both suggested other sections makes me feel that the config property might not be in the right place.
BTW I asked Manuel already on the PR (no answer at this point) but we need to decide if that option applies also to attachments or not. I’m mentioning this here since it impacts the hint description.
BTW we don’t use the “documents” terminology in the UI. Only “pages”
IIUC the original intent is to be able to remove both the document and its attachments permanently under some conditions:
- having the right to do so (ie, being an advanced user and having the proper configuration activated)
- explicitly choosing to do so when removing the document.
Good to know, I’ll update the hint accordingly.
To summarize, we have 2 settings:
-
xwiki.recyclebinindicates whether a recycle bin is available -
canSkipRecycleBinindicates whether a user can choose to skip the recycle bin
Behavior 1
When xwiki.recyclebin is false, all the documents are always removed permanently.
Behavior 2
When xwiki.recyclebin is true and canSkipRecycleBin is false, the behavior is the same as before 12.8RC1, documents are always send to the recycle bin (including their attachments).
The same is true of the children if the user choose to remove them too.
When xwiki.recyclebin is true and canSkipRecycleBin is true, the behavior 2 applies if the user choose the first option (sending the document to the recycle bin), and the behavior 1 applies if the user choose to remove the document permanently/.
If the logic presented above is agreed upon, I will update the javadoc and hints to make this as clear are possible to devs and users.
Regarding the topic of the configuration, I agree that it is counter intuitive to place a document removal into a refactoring section.
However, afaicw no existing section fully match the scope of this newly introduced settings.
On possible approach could be to introduce a xwiki-platform-page-management module dedicated to the management of the configurations of operations related to the pages.
In this case we could introduced a Page management sub-section in the Editing section.
WDYT?
From xwiki.cfg AFAIR and with an old naming style 
You mean behavior 1 I guess (for the first reference).
It seems correct.
I think it’s fine in the refactoring module if we consider that removing something is called a refactoring. However it has consequences and it means that anything related to removing pages or attachments must be eventually moved to the refactoring module (UIX for the content menu, delete templates, etc).
Even the xwiki.recyclebin property needs to be deprecated and replaced by a refactoring.recycleBin.enable config property and RefactoringConfiguration needs to have an API for that. Maybe we need to list all places where delete is done and check if it makes sense to eventually move them to the refactoring module. Ofc the doc/attachment stores shouldn’t be moved. I’m only referring to APIs (java and scripting) and UIs here.
Do we agree about this?
Not sure if it really means this but we need to ask ourselves the question. I’m on the fence here. But I think it makes sense that both xwiki.recyclebin and refactoring.canSkipRecycleBin are put in the same module (eventually).
The hard part is that delete is currently both a model feature and a refactoring feature. Maybe the refactoring module should only concern itself with performing the refactoring and not the deletion, and thus have the model do the deletion and call the refactoring module to perform additional operations. In this case it would mean that the recycle bin configs are model configs and not refactoring ones.
If we consider that removing something is called a refactoring, I agree.
+1
Another point to consider, in addition to the backend design of the page deletion operation, is how it is perceived by the end users.
I don’t thinks I would look for a page deletion related setting in a refactoring section.
Some alternatives could be:
- Content -> Pages
- Editing -> Pages
- Pages -> Pages
This is why I’m on the fence and far from sure that the refactoring module is the right place for this config property.