Needs work
Project:
Translation Management Tool
Version:
7.x-1.x-dev
Component:
Core
Priority:
Minor
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
30 Jan 2012 at 11:10 UTC
Updated:
9 Nov 2013 at 23:38 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
berdirI don't think we should delete automatically. Maybe a way to notify the user of running jobs when trying to delete a source through the UI (kinda hard to detect for e.g. i18n_string stuff) or just some kind of availability check for sources when listing items.. we already discussed this before, for various reasons.
Comment #2
vkosenko commentedThe patch adds some validation to the node deletion process and checks if the node to be deleted has any translation job items.
If the node has active translation job items, then an error message will appear saying that it is not possible to delete the node because it has some active translation jobs. A list of the active jobs (with links to the jobs) will be displayed as well.
In case if the node has no active job items, but still has some jobs with other status types (Needs Review, Accepted), then a warning message will appear informing the user about the fact and offering an option (a checkbox) to delete the items as well.
If the node has no job items of any kind, the usual node deletion process will take place.
Comment #3
berdirComment #4
berdirLooks like you added a space here.
This should return an array and then we should probably use a list theme function to display it.
Same here, no need to implode, API functions should return an array.
Comment #5
vkosenko commentedAll issues have been fixed. Please have a look at the patch now.
Comment #6
miro_dietikerA job that left the client system, has some transactions with real payments pending.
Thus, the items should no more be removed... Instead we should mark them as abandoned.
Comment #7
vkosenko commentedI'd like to point out once again that if a node has active jobs, it will not be possible to delete that node at all (as well as the jobs). A node (and the jobs that refer to the node) can only be deleted if the node has no active jobs.
Comment #8
miro_dietikerI think that resulting behavior (banning deletes of nodes) is wrong. While warning with required confirmation seems right.
I'm fine to allow people to delete nodes anytime.
Comment #9
vkosenko commentedWould it be ok, if I (instead of blocking the deletion of a nodes) add a new job item state named "Abandoned" (the way you recommend), so that all active job items get the Abandoned state and the node can be deleted without any problem, displaying just a warning message that the state change will be performed?
Comment #10
berdirWe have the job status "Cancelled" already, we could add the same for job items?
Cancelled in theory also works with active jobs that are being translated, but it depends on the translation provider if they support it and nobody does that yet AFAIK.
Comment #11
vkosenko commentedSo, I guess we have to select one of the two options:
Option A: We disallow to delete a node that has active job items (the way it is now in the patch). The problem lies in the fact that if we do allow a node to be deleted having some active job items, then it will not be possible to finish the job - ever!
Option B: We add the "Cancelled" job item status, so that all active job items must be set cancelled. And only after all active items have been cancelled successfully, the node can be deleted. In case if any of the active items fails to be cancelled, the node is not deleted. We then could either roll back the cancelled items to the previous state or leave them cancelled.
Comment #12
miro_dietikerI would expect that, deleting a node with active job items just leads to "cancelled" job items.
Regarding the completion of a job: Cancelled items should be excluded from the overall stats and for sure, if a job contains 10 items, 8 are accepted and 2 cancelled, it is perfectly complete.
Comment #13
vkosenko commentedSo, now the patch works this way:
if the node we are trying to delete has active job items, then a warning message will be shown saying that the node has some active items and that their status will be set to 'Cancelled' if the node gets deleted. A list of the corresponding job items will be displayed as well. A checkbox with the information on what items (including their quantity) belong to the node will also be displayed on the screen. If the user checks it, all items will be deleted (the active items will be marked as 'Cancelled' and then deleted).
if the node has no active job items, then a warning message will tell the user that the node has some job items (non-active). The same checkbox allowing to delete the items, will be here as well.
Comment #14
berdirThat is a bit tricky. Having two methods could be complicated to implement for translators. Especially given that cancelTranslationItem() is called on all items if a job is cancelled so there are two different situations. Not sure what we should do with that.
Maybe not call within cancelTranslation() and only set the job item status directly?
Not sure we need those theme functions.
The chance that someone really wants to customize those seems to be close to zero to me :) And even if, we could do something like theme('item_list__tmgmt_node') which should work out of the box. But even that I'm not convinced we need :)
You can use $job_item->getSourceLabel() and $job_item->getSourceUri() (returns $uri array of which you need $uri['path'])
Comment #15
vkosenko commented@Berdir: thanks a lot for your review and valuable comments!
1) my idea was that there could be some translators that could use the cancelTranslationItem()
2) removed custom theming for the items output - using the standart theme_item_list instead
3) updated the way the links to the items are formed (using defaultUri() because I need the path to the job item, getSourceUri() would give me the path to the node itself):
$uri = $job_item->defaultUri();
l($job_item->getSourceLabel(), $uri['path']);
Comment #16
miro_dietikerDeleting is not OK. We should be able to mark it as deleted - at least after submission.
A job is similar to a commerce order. Even if the product disappears, the order still happened. Including the product in it.
Comment #17
miro_dietiker