Deprecate DeleteSpaceAction and related template

[surli]

Hi everyone,

I incidently discovered recently that we have a DeleteSpaceAction and a deletespace.vm template related.
AFAICS this action is here to allow deleting an entire space, and it mainly displays the document about to be deleted, before using the implementation of Delete for performing actual deletion.

It been introduced with Loading...

The thing is that I don’t think we use that action anymore at all in XWiki Standard, the only actual reference I can see is in the DocumentTreeMacros for some data of the space node, but I don’t see it used in the UI.
I quickly tested it, and apparently the template is a bit buggy (doesn’t list all pages of the space) and I don’t think it offers the various protection we put in place for delete UI (e.g. asking questions in case of deletion of a page containing used xclass).

Now note that this action appears to be used in few extensions, see: Sign in to GitHub · GitHub
It is also used a bit for the testing framework of XWiki.

So my proposal is to deprecate the DeleteSpaceAction and the related template and move both of them to legacy: we would need to use another implementation for the testing framework but I think we can just rely on Delete action. (to be confirmed)

wdyt?

[tmortagne]

@surli

So my proposal is to deprecate the DeleteSpaceAction and the related template and move both of them to legacy:

+1

@surli

Now note that this action appears to be used in few extensions, see: Sign in to GitHub · GitHub

Not really a problem as long as it’s still provided in legacy. We might want to log a warning when this action is called, maybe.

[mleduc]

+1 thanks

[MichaelHamann]

+1, thank you!

[vmassol]

+1 globally but we need to decide what we do about the “nestedspaces” mode of the document tree. BTW is this documented somewhere on xwiki.org @mflorea ?

See comment from Marius on matrix:

the Document Tree also supports the “nested spaces” hierarchy mode xwiki-platform/xwiki-platform-core/xwiki-platform-index/xwiki-platform-index-tree/xwiki-platform-index-tree-api/src/main/java/org/xwiki/index/tree/internal at master · xwiki/xwiki-platform · GitHub in which case space nodes are displayed which can have a context menu xwiki-platform/DocumentTreeMacros.xml at master · xwiki/xwiki-platform · GitHub that has an option to remove that space node xwiki-platform/DocumentTreeMacros.xml at master · xwiki/xwiki-platform · GitHub . When that context menu option is clicked it triggers an event xwiki-platform/tree.js at master · xwiki/xwiki-platform · GitHub xtree.contextMenu.remove whose listener deletes the node . When a node that has an associated entity is delete (such as a space node) it triggers xwiki-platform/tree.js at master · xwiki/xwiki-platform · GitHub delete_node.jstree event that is caught and we execute the ‘delete’ operation, which uses the delete URL xwiki-platform/tree.js at master · xwiki/xwiki-platform · GitHub specified in the tree JSON.

[mflorea]

I’m not against deprecating DeleteSpaceAction or deletespace.vm but we should provide an alternative URL to delete a space. It could be a REST URL, or whatever, but since spaces are (still) valid entities in our model there should be an entry point (URL) to delete a space.

@vmassol

we need to decide what we do about the “nestedspaces” mode of the document tree. BTW is this documented somewhere on xwiki.org @mflorea ?

Yes:

• Document Tree Macro parameers

  showSpaces Whether to show the space nodes or not. The documents are grouped by space if the space nodes are displayed.

• Nested spaces tree example

  {{documentTree showSpaces="true" /}}

[vmassol]

@mflorea

Yes:

• Document Tree Macro parameers

showSpaces Whether to show the space nodes or not. The documents are grouped by space if the space nodes are displayed.

• Nested spaces tree example

{{documentTree showSpaces="true" /}}

Thanks. I’ve checked it but I don’t see the mention that when showSpaces is true then there’s a context menu to be able to delete a whole space. Did I miss that or is it missing from the doc?

[mflorea]

@vmassol

Thanks. I’ve checked it but I don’t see the mention that when showSpaces is true then there’s a context menu to be able to delete a whole space. Did I miss that or is it missing from the doc?

Your initial question was about the “nested spaces” mode of the tree, not about the context menu, so my answer was limited that that. I never said that showSpaces implies there is a context menu. These are separate things:

• you may want to see the space nodes without a context menu
• you may want to see the context menu for the nested pages mode, where space nodes are not displayed

If you want to activate the context menu you currently have to use:

{{documentTree showSpaces="true" readOnly="false" links="false" /}}

It’s a hidden feature for now because some of the actions from the context menu are not (fully) implemented (including delete). @surli if you remove the deleteURL entry from JSON the tree will make the request to the tree back-end using action=delete&id=space%3Axwiki%3ASome.Space&form_token=... which will respond with “The specified action is not supported”. This means we probably don’t need a dedicated delete URL for space. The delete space will be handled by xwiki-platform/DocumentTreeMacros.xml at master · xwiki/xwiki-platform · GitHub .

Thanks,
Marius

[surli]

@mflorea

This means we probably don’t need a dedicated delete URL for space.

Actually we probably do need it for our test framework since it’s an action we use in several places. Now as you said it could be a REST Endpoint, and maybe it’s already doable like that.

After second thought I’m wondering if we shouldn’t just remove the overridden render method from the action and delete the template: we’d keep the DeleteSpaceAction with a fallback on delete template for the UI. I don’t think this deletespace.vm template is any kind of API.