Community
Participate
Working Groups
Build ID: M20070921-1145 Currently, Delete action (on Abstract Task in the TaskListView) removes the task from the task list. I think, this action can also be propagated to the repository if remote task deletion is supported. Additional api methods (canDeleteTask and deleteTask) on AbstractRepositoryConnector might help to hook into the infrastructure. I would be happy to help you on this.
Created attachment 81309 [details] DeleteAction patch to support repository delete DeleteAction is modified to consult the repository connector to determine if system allows task deletion. If so, task is deleted from the relevant repository. Then, the repositories are synched.
Up till now many users are using local delete to cleanup archive category and for those users this feature will have very bad consequences. All in all mass-delete is a dangerous feature because it is easy to delete something by accident, especially when selection is mixed with other local tasks. So, perhaps this delete action should appear in the Actions section in the Task Editor. Anyways, if others think this is a good idea, local repository connector and probably web repository connector will need to be updated to allow task deletion. Also, I am sure Mik will tell you that, when submitting features like this you need to provide some test cases to cover new functionality and also provide Mylyn context with your changes. It doesn't seem like your code will close an active task and delete local task context if connector does support task deletion.
I delete tasks from the task list all the time, I would not want this to do it in the repository too. If we add something like this, I think a new option would be the way to go.
I understand the complications of implementing this feature into the existing DeleteAction. Making another option in the view popup menu seems like the cleanest answer to "how is this feature going to be exposed on the UI?" question. This feature can easily be implemented by an individual mylyn integrator. The enhancement is requested to provide the mechanism (plumbing) in the mylyn core code, believing that it is going to be useful for the integrators who want to support remote task deletion.
After sitting on this I think proposed api is a bad idea. If you think about it, task repositories allow many operations on any given task and many of them are used much more often then delete. For example, resolve, reopen, reassign, start progress, etc. So, api should provide unified access to arbitrary operations and not just delete. More over, such api already provided by Mylyn. Well, almost... AbstractTaskDataHandler.postTaskData() allows to invoke/post any operation and the only problem is that RepositoryOperation doesn't provide strong mapping for common operations (and it seem been generally crafted around Bugzilla).
Like creation, deletion is a special operation both in the UI and from the point of view of the Task API. It is a meta operation from the point of view of posting data (e.g. it does not necessarily make sense to post data to something that will stop existing once the post is done). I agree with Balboa that it needs to be supported by the connector API, for example, Tasktop needs it for the Microsoft Outlook connector (although that has been low priority so Shawn has not contributed it yet). Balboa: great that you're taking this on! Please don't be discouraged by some of the posts on this bug, there are some technical points behind them but I don't think there is anything sufficinet to prevent this feature from being incorporated. For more information on writing tests see http://wiki.eclipse.org/Mylyn_Contributor_Reference#Writing_Tests This will definitely need good test coverage, probably easiest to implement with a MockRepositoryConnector extension that makes it support deletion. In terms of UI, I think that for tasks that reside on the repository (not on the local machine) the user needs to be informed of what the deletion will do, and be given an option: You can delete the task and context from the Task List only, or from the repository if supported. If you delete from the Task List only the task will reappear if matched by a query. [Delete Locally] [Delete Locally and from Repository] [Cancel] Then for repositories that do not support deletion (might need something like AbstractTaskRepository.canDelete(String) we can gray out the corresponding option. This would be great because it would reduce all the confusion that users have with the current delete action by making the expectation of server-side delete explicit, and disabled for repositories that don't support it. The Local task repository and Web tasks could have special handling. Rob: please review this patch and guide it along.
Mik, Rob, don't you think that instead of adding api to connector interfaces for each operation we should standardize on operations passed to the AbstractTaskDataHandler.postTaskData() ?
Eugene: maybe, but -1 for the meta operations of creation and deletion (see start of comment#6). For others we can consider doing so if there is sufficient consistency across connectors. Rob and Shawn would have a better sense of that, but let's keep this bug focused on deletion.
I still don't get answer why deletion should be special operation (other that it been suggested by two commercial vendors). Existing connectors have other more common operations and delete isn't one of them, and I just don't understand that.
Eugene: please be careful about implying that we are unfairly biasing input based on commercial vendors' interests. Fairness of balancing projects' priorities is extremely important to me, I am very careful about vendor bias, and if there is merit to them I want any such suggestions or accusations discussed in a place that others can see (e.g. the mylyn-integrators mailing list). I also think that it is very important for the growth of our framework to encourage and support vendors like CollabNet contributing to the project, and have always been encouraging to all vendors interested in contributing. I don't want to belabor this debate here, so if you have a big problem with the design you can add it to our next meeting agenda. The current design is: 1) Operations modification of tasks: work via RepositoryTaskData handling 2) Meta-operations for creating and retrieving task data: work via AbstractRepositoryConnector methods Deletion fits into (2).
Mik, you know me, if I'd want to accuse anyone, I'll say that without implying (i.e. count number of patches needed by particular vendor that took priority to the other contributions). Anyways, for the record, I hardly see how "delete" is either task data retrieval or task creation. Also, by the nature, the former assume presence of the task data and would be more appropriate for the task data handler (i.e. postDaskData). I think adding "delete" operation this way will be quite bad decision for connector-neutral features. Note that I am not against standardizing "delete" operation, but strongly believe it should be done trough different api, without cluttering connector class.
I have started work on this support by adding new API (canDeleteTask and deleteTask) in the AbstractRepository connector. The next step is to come up with a good UI to support this. Also, we should consider adding a delete item to the Task Editor toolbar so that it is consistent between connectors as there are already some that have implemented this themselves.
Created attachment 145980 [details] Screenshot Here is one option of a UI for deleting tasks from the repository by hooking into the current delete mechanism. The wording would need to be updated accordingly. The checkbox would only display if there is at least 1 task in the selected tasks that can be deleted from the repository. Some cases that we will need to deal with and need to make sure that wording is concise: * None of the tasks can be deleted from the repository * All of the tasks can be deleted from the repository * Some of the tasks can be deleted from the repository * Local tasks are special? Also, we need to determine the policy for what happens if a parent task is deleted, do all children get deleted as well?
Shawn: Try the following. Also note that you'll want to leave the app icon on the shell instead of giving it a custom one. Title: Delete Tasks Description: Permanently delete the following tasks from the TaskListView.LABEL? [x] Also delete from repository [grayed if not supported and add "(not supported)" to label, invisible if local] If there is a cross-repository selection, consider just not allowing repository copy deletion. Keep the list of tasks as is.
In terms of deletion action location, I think we should go with the following: * Show in popup menu, both on task editor and in Task List, using the red X icon * Do not show in the task editor toolbar. The action should be uncommon enough, and require knowing the location (container) of the task often enough that the user should be better off invoking it from the Task List. As an analogy, Word does not have a Delete action for an open document, its only available from a listing of the document. * For unsubmitted repository tasks, add the "Clear Outgoing Changes" eraser button to the toolbar. In terms of implementation this is the same effect as deleting, but from ther user's point of view nothing other than their unsubmitted changes is being deleted.
Created attachment 146123 [details] new dialog Here is a screenshot of what the dialog would look like. I wonder if we should still have the text about the planning and context information or not
Created attachment 146127 [details] start of the ui patch Here is a start to the UI patch. I still need to do some work with message externalization and ensuring that the messages make sense in all cases.
Created attachment 146128 [details] mylyn/context/zip
Created attachment 146298 [details] updated patch I have updated the patch to run the deletion in a TaskJob so that it does not block the user. Also, I have updated the messages to inform the user that they will be deleting contexts and planning information. With this, I also moved the delegating progress monitor up to the TaskJob so that all TaskJobs can use this if they wish. Steffen, can you do a sanity check of this before I externalize the strings and commit this?
Created attachment 146299 [details] mylyn/context/zip
Looks good. A few minor nits in DeleteTasksJob: * OperationCanceledException should be handled * Consider wrapping the calls to the connector in a SafeRunnable
Thanks for the feedback Steffen. I handle the operation canceled exception now. As for the SafeRunnable, I mimicked it by catching throwable since SafeRunnable is from jface and the DeleteTasksJob is in the core. I catch Exception and LinkageError as the SynchronizeRepositoriesJob does.
Created attachment 146337 [details] mylyn/context/zip
Steffen, does Trac or XPlanner support deleting tasks from the server?
Trac has support for task deletion but that usually requires admin privileges and I am not sure if the remote API correctly implements it. I'll think about adding support for deletion but I might not it as it would fail for most users and I haven't seen any requests for it. I don't know about XPlanner.
UI review comments: * Delete needs to appear in the popup menu of the task editor as "Delete Task".
Reopening to address comment 26.
I have added the context menu "Delete Task " in the editor.
Created attachment 146998 [details] mylyn/context/zip
