We need to be sure that the “allow to skip the recycle bin” configuration option is an option of the refactoring module and not an option of the model. I don’t recall where (I think in a github code review) but I commented about this already but I don’t remember other’s opinions so I’m restarting the discussion here.
I’m ambivalent on my side since the CRUD + move/rename operations on pages are APIs of the model right now and these APIs call the refactoring module internally to perform the action. This means that currently the main APIs for this are inside the model and thus it would make sense to have the config property also in the model configuration. Now, the refactoring module also exposes a Script Service (and indirectly through Jobs for Java, but not a very easy to use API). So basically we have several API locations to do the same things, hence the confusion for me and why it’s hard to decide where the config property should be located. More specifically we have 2 main locations for APIs:
- When inside a script, it’s the refactoring module with the exposed Script Service
- When in Java, it’s the model APIs (accessed through the XWiki and XWikiDocument classes)
I guess we have 2 options:
- Consider that the config option should go in the same module as where the API is. For java API this is in the oldcore and thus CoreConfiguration (However, maybe should be in the model module since this is where any new model would go in the future).
- Consider that the config option should go in the module where it’s used and that’s the refactoring module. This means that we don’t pass the config as a parameter of the various refactoring operations. I’m not sure about that and I think it would make more sense that the refactoring module has all its config passed to the methods as it’s already done for lots of existing configurations.
Thus I don’t believe that locating the config option in the refactoring module is a good idea at all. I much prefer to continue seeing the refactoring module as a utility module.
WDYT?