From 7dd51f4c462309fc761a10c03eac4cc4c9297f3e Mon Sep 17 00:00:00 2001 From: Jens Pelzetter Date: Sat, 11 Dec 2021 18:57:53 +0100 Subject: [PATCH] Bugfixes for workflow handling. --- .../documents/DocumentWorkflowController.java | 29 ++- .../documents/SelectedDocumentModel.java | 46 +++++ .../documents/authoringstep.xhtml | 70 ++++++- .../article/article-basic-properties.xhtml | 2 +- .../org/librecms/CmsAdminMessages.properties | 4 + .../librecms/CmsAdminMessages_de.properties | 4 + .../libreccm/workflow/WorkflowManager.java | 187 +++++++++++------- .../libreccm/workflow/WorkflowRepository.java | 2 + 8 files changed, 268 insertions(+), 76 deletions(-) diff --git a/ccm-cms/src/main/java/org/librecms/ui/contentsections/documents/DocumentWorkflowController.java b/ccm-cms/src/main/java/org/librecms/ui/contentsections/documents/DocumentWorkflowController.java index e972976d7..64f99b219 100644 --- a/ccm-cms/src/main/java/org/librecms/ui/contentsections/documents/DocumentWorkflowController.java +++ b/ccm-cms/src/main/java/org/librecms/ui/contentsections/documents/DocumentWorkflowController.java @@ -301,19 +301,19 @@ public class DocumentWorkflowController { * @param documentPath The path of the currentd document. * @param newWorkflowUuid The UUID of the the workflow definition form * which the new workflow is created. - * @param returnUrl The URL to return to. + * @param returnUrlParam The URL to return to. * * @return A redirect to the {@code returnUrl}. */ @POST - @Path("@workflow/@applyAlternative/{workflowIdentifier}") + @Path("/apply-alternative") @AuthorizationRequired @Transactional(Transactional.TxType.REQUIRED) public String applyAlternateWorkflow( @PathParam("sectionIdentifier") final String sectionIdentifier, @PathParam("documentPath") final String documentPath, @FormParam("newWorkflowUuid") final String newWorkflowUuid, - @FormParam("returnUrl") final String returnUrl + @FormParam("returnUrl") final String returnUrlParam ) { final Optional sectionResult = sectionsUi .findContentSection(sectionIdentifier); @@ -347,18 +347,35 @@ public class DocumentWorkflowController { ); } - final Optional workflowResult = section + final Optional workflowTemplateResult = section .getWorkflowTemplates() .stream() .filter(template -> template.getUuid().equals(newWorkflowUuid)) .findAny(); - if (!workflowResult.isPresent()) { + if (!workflowTemplateResult.isPresent()) { models.put("section", section.getLabel()); models.put("workflowUuid", newWorkflowUuid); return "org/librecms/ui/contentsection/documents/workflow-not-found.xhtml"; } - workflowManager.createWorkflow(workflowResult.get(), item); + final String returnUrl; + if (returnUrlParam.startsWith("/@contentsections")) { + returnUrl = returnUrlParam.substring("/@contentsections".length()); + } else if(returnUrlParam.startsWith("@contentsections")) { + returnUrl = returnUrlParam.substring("@contentsections".length()); + } else { + returnUrl = returnUrlParam; + } + + final Workflow oldWorkflow = item.getWorkflow(); + if (oldWorkflow != null) { + workflowManager.removeWorkflowFromObject(oldWorkflow, item); + } + + final Workflow workflow = workflowManager.createWorkflow( + workflowTemplateResult.get(), item + ); + item.setWorkflow(workflow); return String.format("redirect:%s", returnUrl); } diff --git a/ccm-cms/src/main/java/org/librecms/ui/contentsections/documents/SelectedDocumentModel.java b/ccm-cms/src/main/java/org/librecms/ui/contentsections/documents/SelectedDocumentModel.java index 1989b21e7..6d5ebf3e0 100644 --- a/ccm-cms/src/main/java/org/librecms/ui/contentsections/documents/SelectedDocumentModel.java +++ b/ccm-cms/src/main/java/org/librecms/ui/contentsections/documents/SelectedDocumentModel.java @@ -32,6 +32,7 @@ import org.librecms.contentsection.Folder; import org.librecms.contentsection.FolderManager; import org.librecms.contentsection.privileges.ItemPrivileges; import org.librecms.ui.contentsections.FolderBreadcrumbsModel; +import org.librecms.ui.contentsections.WorkflowTemplateListModel; import java.util.Arrays; import java.util.Collections; @@ -45,6 +46,7 @@ import javax.enterprise.context.RequestScoped; import javax.inject.Inject; import javax.inject.Named; import javax.servlet.http.HttpServletRequest; +import javax.transaction.Transactional; import javax.ws.rs.core.UriBuilder; /** @@ -165,6 +167,11 @@ public class SelectedDocumentModel { * The tasks of the workflow assigned to the current item. */ private List allTasks; + + /** + * Available workflows that can be assigned to the item. + */ + private List availableWorkflows; public String getItemName() { return itemName; @@ -216,12 +223,17 @@ public class SelectedDocumentModel { ); } + public List getAvailableWorkflows() { + return Collections.unmodifiableList(availableWorkflows); + } + /** * Sets the current content item/document and sets the properties of this * model based on the item. * * @param item */ + @Transactional(Transactional.TxType.REQUIRED) void setContentItem(final ContentItem item) { this.item = Objects.requireNonNull(item); itemName = item.getDisplayName(); @@ -261,6 +273,14 @@ public class SelectedDocumentModel { currentTask.setCurrentTask(true); } } + + availableWorkflows = item + .getContentType() + .getContentSection() + .getWorkflowTemplates() + .stream() + .map(this::buildWorkflowTemplateListModel) + .collect(Collectors.toList()); } /** @@ -394,5 +414,31 @@ public class SelectedDocumentModel { .buildFromMap(values, false) .toString(); } + + /** + * Helper method for building a {@link WorkflowTemplateListModel} for a + * {@link Workflow}. + * + * @param workflow The workflow. + * + * @return A {@link WorkflowTemplateListModel} for the {@code workflow}. + */ + private WorkflowTemplateListModel buildWorkflowTemplateListModel( + final Workflow workflow + ) { + final WorkflowTemplateListModel model = new WorkflowTemplateListModel(); + model.setDescription( + globalizationHelper.getValueFromLocalizedString( + workflow.getDescription() + ) + ); + model.setHasTasks(!workflow.getTasks().isEmpty()); + model.setName( + globalizationHelper.getValueFromLocalizedString(workflow.getName()) + ); + model.setUuid(workflow.getUuid()); + model.setWorkflowId(workflow.getWorkflowId()); + return model; + } } diff --git a/ccm-cms/src/main/resources/WEB-INF/views/org/librecms/ui/contentsection/documents/authoringstep.xhtml b/ccm-cms/src/main/resources/WEB-INF/views/org/librecms/ui/contentsection/documents/authoringstep.xhtml index 0a26f046d..0614c4cc5 100644 --- a/ccm-cms/src/main/resources/WEB-INF/views/org/librecms/ui/contentsection/documents/authoringstep.xhtml +++ b/ccm-cms/src/main/resources/WEB-INF/views/org/librecms/ui/contentsection/documents/authoringstep.xhtml @@ -1,5 +1,6 @@ ]> @@ -29,10 +30,72 @@ #{CmsAdminMessages['contentsection.document.authoring.workflow.active_workflow_label']}: #{CmsSelectedDocumentModel.workflowName} + + +

@@ -40,7 +103,7 @@ #{CmsAdminMessages['contentsection.document.authoring.workflow.active_task']} #{CmsSelectedDocumentModel.currentTask.label} - + #{CmsAdminMessages['contentsection.document.authoring.workflow.active_task.none']}

@@ -53,6 +116,9 @@ #{CmsAdminMessages['contentsection.document.authoring.workflow.start']}
+ + + #{CmsAdminMessages['contentsection.document.authoring.workflow.none']} @@ -115,7 +181,7 @@

#{CmsAdminMessages['contentsection.document.authoring.steps.title']}

- +
    diff --git a/ccm-cms/src/main/resources/WEB-INF/views/org/librecms/ui/contenttypes/article/article-basic-properties.xhtml b/ccm-cms/src/main/resources/WEB-INF/views/org/librecms/ui/contenttypes/article/article-basic-properties.xhtml index f01444731..c199aeb43 100644 --- a/ccm-cms/src/main/resources/WEB-INF/views/org/librecms/ui/contenttypes/article/article-basic-properties.xhtml +++ b/ccm-cms/src/main/resources/WEB-INF/views/org/librecms/ui/contenttypes/article/article-basic-properties.xhtml @@ -7,7 +7,7 @@ + value="#{mvc.basePath}/#{ContentSectionModel.sectionName}/documents/#{CmsSelectedDocumentModel.itemPath}/@article-basicproperties" />

    #{CmsArticleMessageBundle.getMessage('basicproperties.header', [CmsArticlePropertiesStep.name])}

    diff --git a/ccm-cms/src/main/resources/org/librecms/CmsAdminMessages.properties b/ccm-cms/src/main/resources/org/librecms/CmsAdminMessages.properties index 826671dd6..20e1dfdef 100644 --- a/ccm-cms/src/main/resources/org/librecms/CmsAdminMessages.properties +++ b/ccm-cms/src/main/resources/org/librecms/CmsAdminMessages.properties @@ -1010,3 +1010,7 @@ pages.page.details.dialog.category.label=Category pages.page.details.dialog.properties.label=Properties pages.page.details.dialog.properties.key=Name pages.page.details.dialog.properties.value=Value +contentsection.document.authoring.workflow.none=No workflow assigned +contentsection.document.authoring.workflow.change_workflow.title=Assign workflow +contentsection.document.authoring.workflow.change_workflow.submit=Assign workflow +contentsection.document.authoring.workflow.change_workflow.close=Cancel diff --git a/ccm-cms/src/main/resources/org/librecms/CmsAdminMessages_de.properties b/ccm-cms/src/main/resources/org/librecms/CmsAdminMessages_de.properties index 11274f4af..cfb748a31 100644 --- a/ccm-cms/src/main/resources/org/librecms/CmsAdminMessages_de.properties +++ b/ccm-cms/src/main/resources/org/librecms/CmsAdminMessages_de.properties @@ -1011,3 +1011,7 @@ pages.page.details.dialog.category.label=Kategorie pages.page.details.dialog.properties.label=Eigenschaften pages.page.details.dialog.properties.key=Name pages.page.details.dialog.properties.value=Wert +contentsection.document.authoring.workflow.none=Kein Arbeitsablauf zugewiesen +contentsection.document.authoring.workflow.change_workflow.title=Arbeitsablauf zuweisen +contentsection.document.authoring.workflow.change_workflow.submit=Arbeitsablauf zuweisen +contentsection.document.authoring.workflow.change_workflow.close=Abbrechen diff --git a/ccm-core/src/main/java/org/libreccm/workflow/WorkflowManager.java b/ccm-core/src/main/java/org/libreccm/workflow/WorkflowManager.java index 890f76ebf..96ca169ef 100644 --- a/ccm-core/src/main/java/org/libreccm/workflow/WorkflowManager.java +++ b/ccm-core/src/main/java/org/libreccm/workflow/WorkflowManager.java @@ -71,10 +71,12 @@ import java.util.Optional; */ @RequestScoped public class WorkflowManager implements Serializable { + private static final long serialVersionUID = -6939804120313699606L; private final static Logger LOGGER = LogManager.getLogger( - WorkflowManager.class); + WorkflowManager.class + ); @Inject private EntityManager entityManager; @@ -105,7 +107,8 @@ public class WorkflowManager implements Serializable { @PostConstruct private void init() { final KernelConfig kernelConfig = confManager.findConfiguration( - KernelConfig.class); + KernelConfig.class + ); defaultLocale = kernelConfig.getDefaultLocale(); } @@ -121,23 +124,29 @@ public class WorkflowManager implements Serializable { @AuthorizationRequired @RequiresPrivilege(CoreConstants.PRIVILEGE_ADMIN) @Transactional(Transactional.TxType.REQUIRED) - public Workflow createWorkflow(final Workflow template, - final CcmObject object) { - Objects.requireNonNull(template, - "Can't create a workflow without a template."); + public Workflow createWorkflow( + final Workflow template, final CcmObject object + ) { + Objects.requireNonNull( + template, "Can't create a workflow without a template." + ); if (!template.isAbstractWorkflow()) { throw new IllegalArgumentException( - "The provided template is not an abstract workflow"); + "The provided template is not an abstract workflow" + ); } - Objects.requireNonNull(object, - "Can't create a workflow without an object."); + Objects.requireNonNull( + object, "Can't create a workflow without an object." + ); final Workflow workflow = new Workflow(); final LocalizedString name = new LocalizedString(); - template.getName().getValues().forEach( - (locale, str) -> name.addValue(locale, str)); + template + .getName() + .getValues() + .forEach((locale, str) -> name.addValue(locale, str)); workflow.setName(name); final LocalizedString description = new LocalizedString(); @@ -156,16 +165,19 @@ public class WorkflowManager implements Serializable { .forEach(taskTemplate -> createTask(workflow, taskTemplate, tasks)); template .getTasks() - .forEach(taskTemplate -> { - fixTaskDependencies(taskTemplate, - tasks.get(taskTemplate.getTaskId()), - tasks); + .forEach( + taskTemplate -> { + fixTaskDependencies( + taskTemplate, + tasks.get(taskTemplate.getTaskId()), + tasks + ); }); - for(final Map.Entry task : tasks.entrySet()) { + for (final Map.Entry task : tasks.entrySet()) { task.getValue().setTaskState(TaskState.DISABLED); } - + workflow.setObject(object); workflow.setState(WorkflowState.INIT); @@ -175,6 +187,20 @@ public class WorkflowManager implements Serializable { return workflow; } + @AuthorizationRequired + @RequiresPrivilege(CoreConstants.PRIVILEGE_ADMIN) + @Transactional(Transactional.TxType.REQUIRED) + public void removeWorkflowFromObject( + final Workflow workflow, final CcmObject object) { + Objects.requireNonNull(workflow, "Can't delete null."); + Objects.requireNonNull(object); + + for(final Task task : workflow.getTasks()) { + taskRepo.delete(task); + } + workflowRepo.delete(workflow); + } + /** * Helper method for * {@link #createWorkflow(org.libreccm.workflow.WorkflowTemplate, org.libreccm.core.CcmObject)} @@ -184,15 +210,19 @@ public class WorkflowManager implements Serializable { * @param template The template for the task from the workflow template. * @param tasks A map for storing the new tasks. */ - private void createTask(final Workflow workflow, - final Task template, - final Map tasks) { - + private void createTask( + final Workflow workflow, + final Task template, + final Map tasks + ) { final Class templateClass = template.getClass(); final Task task; try { task = templateClass.getDeclaredConstructor().newInstance(); - } catch (InstantiationException | IllegalAccessException | NoSuchMethodException | InvocationTargetException ex) { + } catch (InstantiationException + | IllegalAccessException + | NoSuchMethodException + | InvocationTargetException ex) { throw new RuntimeException(ex); } @@ -227,8 +257,9 @@ public class WorkflowManager implements Serializable { final LocalizedString localized = (LocalizedString) value; final LocalizedString copy = new LocalizedString(); - localized.getValues().forEach( - (locale, str) -> copy.addValue(locale, str)); + localized + .getValues() + .forEach((locale, str) -> copy.addValue(locale, str)); writeMethod.invoke(task, copy); } else { @@ -247,16 +278,18 @@ public class WorkflowManager implements Serializable { if (template instanceof AssignableTask) { final AssignableTask assignableTemplate - = (AssignableTask) template; + = (AssignableTask) template; final AssignableTask assignableTask = (AssignableTask) task; assignableTemplate .getAssignments() .stream() .map(TaskAssignment::getRole) - .forEach(role -> { - assignableTaskManager.assignTask(assignableTask, role); - }); + .forEach( + role -> assignableTaskManager.assignTask( + assignableTask, role + ) + ); } } @@ -270,19 +303,19 @@ public class WorkflowManager implements Serializable { * @param task * @param tasks */ - private void fixTaskDependencies(final Task template, - final Task task, - final Map tasks) { - + private void fixTaskDependencies( + final Task template, final Task task, final Map tasks + ) { if (template.getBlockedTasks() != null && !template.getBlockedTasks().isEmpty()) { - for (final TaskDependency blocked : template.getBlockedTasks()) { - final Task blockingTask = tasks - .get(blocked.getBlockingTask().getTaskId()); - final Task blockedTask = tasks - .get(blocked.getBlockedTask().getTaskId()); + final Task blockingTask = tasks.get( + blocked.getBlockingTask().getTaskId() + ); + final Task blockedTask = tasks.get( + blocked.getBlockedTask().getTaskId() + ); try { taskManager.addDependentTask(blockingTask, blockedTask); } catch (CircularTaskDependencyException ex) { @@ -297,17 +330,20 @@ public class WorkflowManager implements Serializable { // -> task.addDependentTask(tasks.get(dependent.getTaskId()))); // } for (final TaskDependency blocking : template.getBlockingTasks()) { - - final Task blockingTask = tasks - .get(blocking.getBlockingTask().getTaskId()); - final Task blockedTask = tasks - .get(blocking.getBlockedTask().getTaskId()); + final Task blockingTask = tasks.get( + blocking.getBlockingTask().getTaskId() + ); + final Task blockedTask = tasks.get( + blocking.getBlockedTask().getTaskId() + ); try { - taskManager.addDependentTask(blockingTask, blockedTask); - } catch(CircularTaskDependencyException ex) { + taskManager.addDependentTask( + blockingTask, blockedTask + ); + } catch (CircularTaskDependencyException ex) { throw new UnexpectedErrorException(ex); } - + } // if (template.getDependsOn() != null @@ -331,14 +367,18 @@ public class WorkflowManager implements Serializable { public List findEnabledTasks(final Workflow workflow) { if (workflow.getState() == WorkflowState.DELETED || workflow.getState() == WorkflowState.STOPPED) { - LOGGER.debug(String.format("Workflow state is \"%s\". Workflow " - + "has no enabled tasks.", - workflow.getState().toString())); + LOGGER.debug( + String.format( + "Workflow state is \"%s\". Workflow has no enabled tasks.", + workflow.getState().toString() + ) + ); return Collections.emptyList(); } final TypedQuery query = entityManager.createNamedQuery( - "Task.findEnabledTasks", Task.class); + "Task.findEnabledTasks", Task.class + ); query.setParameter("workflow", workflow); return Collections.unmodifiableList(query.getResultList()); @@ -357,7 +397,8 @@ public class WorkflowManager implements Serializable { @Transactional(Transactional.TxType.REQUIRED) public List findFinishedTasks(final Workflow workflow) { final TypedQuery query = entityManager.createNamedQuery( - "Task.findFinishedTasks", Task.class); + "Task.findFinishedTasks", Task.class + ); query.setParameter("workflow", workflow); return Collections.unmodifiableList(query.getResultList()); @@ -376,7 +417,8 @@ public class WorkflowManager implements Serializable { @Transactional(Transactional.TxType.REQUIRED) public List findOverdueTasks(final Workflow workflow) { final TypedQuery query = entityManager.createNamedQuery( - "AssignableTask.findOverdueTasks", AssignableTask.class); + "AssignableTask.findOverdueTasks", AssignableTask.class + ); query.setParameter("workflow", workflow); query.setParameter("now", new Date()); @@ -392,7 +434,6 @@ public class WorkflowManager implements Serializable { @RequiresPrivilege(CoreConstants.PRIVILEGE_ADMIN) @Transactional(Transactional.TxType.REQUIRED) public void start(final Workflow workflow) { - final WorkflowState oldState = workflow.getState(); workflow.setState(WorkflowState.STARTED); @@ -414,10 +455,13 @@ public class WorkflowManager implements Serializable { final Optional currentUser = shiro.getUser(); if (currentUser.isPresent() && assignableTaskManager - .isAssignedTo((AssignableTask) firstTask, - currentUser.get())) { - assignableTaskManager - .lockTask((AssignableTask) firstTask); + .isAssignedTo( + (AssignableTask) firstTask, + currentUser.get() + )) { + assignableTaskManager.lockTask( + (AssignableTask) firstTask + ); } } } @@ -437,7 +481,8 @@ public class WorkflowManager implements Serializable { private void updateState(final Workflow workflow) { if (workflow.getTasksState() == TaskState.ENABLED) { final TypedQuery query = entityManager.createNamedQuery( - "Task.countUnfinishedAndActiveTasksForWorkflow", Long.class); + "Task.countUnfinishedAndActiveTasksForWorkflow", Long.class + ); query.setParameter("workflow", workflow); final Long result = query.getSingleResult(); @@ -451,7 +496,8 @@ public class WorkflowManager implements Serializable { if (workflow.getTasksState() == TaskState.FINISHED) { final TypedQuery query = entityManager.createNamedQuery( - "Task.countUnfinishedTasksForWorkflow", Long.class); + "Task.countUnfinishedTasksForWorkflow", Long.class + ); query.setParameter("workflow", workflow); final Long result = query.getSingleResult(); @@ -485,9 +531,12 @@ public class WorkflowManager implements Serializable { @Transactional(Transactional.TxType.REQUIRED) public void finish(final Workflow workflow) { if (workflow.getTasksState() != TaskState.ENABLED) { - throw new IllegalArgumentException(String.format( - "Workflow \"%s\" is not enabled.", - workflow.getName().getValue(defaultLocale))); + throw new IllegalArgumentException( + String.format( + "Workflow \"%s\" is not enabled.", + workflow.getName().getValue(defaultLocale) + ) + ); } workflow.setTasksState(TaskState.FINISHED); @@ -510,8 +559,10 @@ public class WorkflowManager implements Serializable { switch (workflow.getTasksState()) { case DISABLED: - LOGGER.debug("Workflow \"{}\" is disabled; enabling it.", - workflow.getName().getValue(defaultLocale)); + LOGGER.debug( + "Workflow \"{}\" is disabled; enabling it.", + workflow.getName().getValue(defaultLocale) + ); workflow.setTasksState(TaskState.ENABLED); workflowRepo.save(workflow); break; @@ -521,10 +572,12 @@ public class WorkflowManager implements Serializable { workflowRepo.save(workflow); break; default: - LOGGER.debug("Workflow \"{}\" has tasksState \"{}\", " - + "#enable(Workflow) does nothing.", - workflow.getName().getValue(defaultLocale), - workflow.getTasksState()); + LOGGER.debug( + "Workflow \"{}\" has tasksState \"{}\", " + + "#enable(Workflow) does nothing.", + workflow.getName().getValue(defaultLocale), + workflow.getTasksState() + ); break; } } diff --git a/ccm-core/src/main/java/org/libreccm/workflow/WorkflowRepository.java b/ccm-core/src/main/java/org/libreccm/workflow/WorkflowRepository.java index f696c10fa..de9d454de 100644 --- a/ccm-core/src/main/java/org/libreccm/workflow/WorkflowRepository.java +++ b/ccm-core/src/main/java/org/libreccm/workflow/WorkflowRepository.java @@ -76,6 +76,7 @@ public class WorkflowRepository extends AbstractEntityRepository * @return An {@link Optional} containing the {@link Workflow} identified by * the provided UUID. */ + @Transactional(Transactional.TxType.REQUIRED) public Optional findByUuid(final String uuid) { if (uuid == null) { throw new IllegalArgumentException( @@ -102,6 +103,7 @@ public class WorkflowRepository extends AbstractEntityRepository * {@code object} if the object has a workflow. Otherwise an empty * {@link Optional} is returned. */ + @Transactional(Transactional.TxType.REQUIRED) public Optional findWorkflowForObject(final CcmObject object) { if (object == null) { throw new IllegalArgumentException(