Bugzilla – Full Text Bug Listing
| Summary: | Extension for the Delete Server Dialog | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | [WebTools] WTP ServerTools | Reporter: | Kaloyan Raev <kaloyan> | ||||||||||
| Component: | wst.server | Assignee: | Elson Yuen <eyuen7> | ||||||||||
| Status: | RESOLVED FIXED | QA Contact: | Elson Yuen <eyuen7> | ||||||||||
| Severity: | enhancement | ||||||||||||
| Priority: | P3 | CC: | alexander.silgidjian, dimitar.giormov, eyuen7, manderse, shr31223, stryker | ||||||||||
| Version: | 3.3 | ||||||||||||
| Target Milestone: | 3.6 M5 | ||||||||||||
| Hardware: | PC | ||||||||||||
| OS: | Windows Vista | ||||||||||||
| See Also: |
https://git.eclipse.org/r/109215 https://git.eclipse.org/r/109216 https://git.eclipse.org/r/109856 https://git.eclipse.org/c/servertools/webtools.servertools.git/commit/?id=e6a8bb987a980d93f3006c889a8c1237f04e9491 |
||||||||||||
| Whiteboard: | |||||||||||||
| Attachments: |
|
||||||||||||
Wouldn't this be better done as a refactoring operation ? Use the refactoring extensibility to react/participate in the Delete operation ? Max, I am not sure this would fit to this use case. The additional vendor specific actions does not change resources in the Eclipse workspace, but invokes actions on the server-side. These actions are optional and it is up to the user to decide if they will be invoked. This is why I think extending the Delete Server Dialog with additional checkboxes fits best. I have created a patch for this and uploaded it in GitHub: https://github.com/asilgidjian/wtp-server-tools Here are the details with regards to the implementation: An extension point should be defined providing the possibility to describe a sequence of additional options that will appear in the delete dialog. The additional delete options will have the following attributes: 1. id – id of the option 2. label – the label that will appear in delete dialog for the option 3. order – optional ordering of this option relative to other options 4. selected - specifies if the option is selected by default 5. operation - optional operation that will be executed if the option is selected 6. serverTypes - The Server Types for which the option will appear 7. parent - optional id of a parent option 8. enablement – optional expression which evaluation will show or hide the option For the purposes of operation attribute, a new interface extending IUndoableOperation will be defined. The new interface will provide possibility to pass the IServer[] for which DeleteServerDialog is called to the operation. The enablement expression will be evaluated with every call of the DeleteServerDialog, thus the options will be reloaded every time. A testcase will be provided in another repository. Here is the repository with the test: https://github.com/asilgidjian/webtools.servertools.tests Looks nice, but just want to point out that refactoring participants does *not* have to be doing raw resource changes. They can change anything and can provide custom diffs for the preview view. Hi Max, Would you explain a little bit more? I am not quite sure that I am using "refactoring participant" and I am performing "raw resource change". Kind Regards, Alex My comment was to Kaloyan's comment that using the refactoring (ltk?) api in Eclipse couldn't be applied here. I claim it can and I think your patch/approach would fit into it too and use the same kind of UI and API for it like other "delete' or "renames" in other part of eclipse. http://www.eclipse.org/articles/Article-LTK/ltk.html From your note to the WTP distribution list, the usage of this new extension is that you want to do action on deleting the last server of a certain server type. Given that you want to do certain action when deleting the server, have you considered using the IServerLifeCycleListener to do the appropriate action on a serverRemoved event? You can pop up your own GUI when needed in that case. Is there anything that you find missing from this approach? We wanted to avoid the double popups, which will appear in that case. The extension point is more robust solution and provides better usability. ok, thanks for the clarification on the usage. Looking at the proposed implementation, it is exposed as boolean delete options to the existing delete dialog. If we are going to allow expansion or customization on that dialog, I wonder if we should expose in a way that will pass the composite to the extension and allow the user to inject their own custom UI in that dialog under the existing check boxes. Doing it that way provides the flexibility for user to add their own customization without limiting to boolean input. Also, as long as we expose a way to access the existing check boxes in the dialog, the adopter can implement listener to events from the existing check boxes to fulfill the dependency addition requirement on the original request. Then, we don't have to burn the parent/child relationship and ordering in the extension point. Hi Elson, this sounds good, allowing the adopter to extend the delete dialog with custom composite will be much more flexible and will allow us and the other adopters to plug additional more complex logic in delete dialog. One way to do this is making similar extensions like in create server wizard and server editor (org.eclipse.wst.server.ui.serverEditorOverviewPageModifier). We should allow the user to completely overwrite the current composite containing the stop and delete configuration checkboxes. But still the adopter can reuse the current composite in his own composite implementation. Another way to achieve this is without using extensions, through the API. But I am not sure where the integration point should be. I mean where the adopter will attach the custom composite to the dialog. Should we create a method that will accept this composite and if yes, where do you think should we add this method? best regards, Dimitar The extension point will look like a combination of org.eclipse.wst.server.ui.serverCreationWizardPageExtension and serverEditorOverviewPageModifier. We tend to be cautious on completely replacing existing GUI. In both the existing extension points serverCreationWizardPageExtension and serverEditorOverviewPageModifier, we tend to provide ways for using to add addition GUI instead of completely replace the existing one. This design ensure all the adapters has similar look and feel on the product and prevent adopters to go off on a tangent to do an operation that is completely different operation. I think an extension will be a cleaner design instead of doing direct API in general. It also allow quick server type filtering. Please consider how this API will work when used with multiple-selection on the delete event. We've had issues in the past with multiple-selection delete being broken. https://bugs.eclipse.org/bugs/show_bug.cgi?id=409475 https://bugs.eclipse.org/bugs/show_bug.cgi?id=345087 I have created an extension as discussed and uploaded it here: https://github.com/asilgidjian/webtools.servertools-344543 A test for the newly created extension is available here: https://github.com/asilgidjian/webtools.servertools.tests-344543 @Rob: With the proposed extension the adopters will receive the array of servers for which deletion is triggered. Based on that they may visualize their custom controls and perform custom cleanup logic upon deletion. Created attachment 238161 [details]
patch
Created attachment 238162 [details]
test for the patch
Created attachment 238790 [details]
v1.0
Thank you for submitting the patch.
A new patch has been created based on the original suggested patch with the following changes:
1. Introduced isEnabled(), performPreDeleteAction() and performPostDeleteAction() to org.eclipse.wst.server.ui.internal.DeleteServerDialogExtension.
2. Moved DeleteServerDialogExtension to external package.
3. Changed the life cycle of DeleteServerDialogExtension.
4. Other code and extension point cleanup
Please try it out to see if it addresses all your requirements.
Created attachment 238791 [details]
v1.0 test
Updated test patch to match the latest patch v1.0 based on the original patch on the tests.
The patch addresses all our requirements. Thanks for amending it. Code released to 3.6. Resolving as code goes to master. New Gerrit change created: https://git.eclipse.org/r/109215 New Gerrit change created: https://git.eclipse.org/r/109216 New Gerrit change created: https://git.eclipse.org/r/109856 Gerrit change https://git.eclipse.org/r/109856 was merged to [master]. Commit: http://git.eclipse.org/c/servertools/webtools.servertools.git/commit/?id=e6a8bb987a980d93f3006c889a8c1237f04e9491 |
The Delete Server Dialog is the one that pops up when users call the Delete action on a server in the Servers view. It requests the user to confirm the deletion of the server instance. The dialog also has some built-in logic for additional actions to be executed together with the deletion: * Delete unused server configuration(s) * Delete running server(s) * Stop server(s) before deleting For the server adapter of our vendor's app server, we need to add more options that, if selected, call additional actions on deletions. It would be nice if such options can be added using an appropriate extension point. It's perfectly OK that these options are displayed like checkboxes. There is no need for more complex options. Additional requirement is to have dependency between the different options like we already have between "Delete running server(s)" and "Stop server(s) before deleting": the second option is only available if the first one is selected. Default value for each options should be also set using the extension point.