[4.0] Workflow prevent trashing of used stages/workflows by bembelimen · Pull Request #21579 · joomla/joomla-cms

bembelimen · 2018-08-13T03:25:54Z

Pull Request for Issue # .

Summary of Changes

This PR prevents the trashing of workflows/stages when articles are assigned to them

Testing Instructions

Create different stages/workflows and articles assigned to them. Then try to trash the articles

Expected result

Error messages when an article is in the workflow/stage

wilsonge · 2018-08-13T08:57:28Z

plugins/content/joomla/joomla.php

+	private function _canDeleteStage($pk)
+	{
+		// Check if this function is enabled.
+		if (!$this->params->def('check_states_workflow', 1))


Why do we need this parameter? Why not just use the com_content enabled parameter (and when it's disabled it should always stop you from deleting a stage)

The category check uses their own parameter, too. So it was a "copy&extend".

Then we need to fix that too :P Like the parameters here need to respect their extensions helper values - not have their own

So to be clear: it should not be possible to disable this function (beside deactivating the plugin)?

Correct. If workflows is enabled in the relevant component, then this is enabled

Quy · 2018-08-13T12:15:40Z

plugins/content/joomla/joomla.php

 use Joomla\Component\Content\Administrator\Table\ArticleTable;
 use Joomla\CMS\Workflow\Workflow;
 use Joomla\Utilities\ArrayHelper;
+use Joomla\Component\Workflow\Administrator\Table\StageTable;


Please sort in alpha order. See this PR #21577.

Quy · 2018-08-13T12:16:00Z

plugins/content/joomla/joomla.php

+		}
+
+		return $result;
+


Remove blank line

Quy · 2018-08-13T12:20:31Z

plugins/content/joomla/joomla.xml

 					class="switcher"
-					label="PLG_CONTENT_JOOMLA_FIELD_CHECK_STATES_LABEL"
-					description="PLG_CONTENT_JOOMLA_FIELD_CHECK_STATES_DESC"
+					label="PLG_CONTENT_JOOMLA_FIELD_CHECK_STATES_WORKFLOW_LABEL"


Per Joomla Coding Standards:

The four attributes name, type, label and description should be written in this order and at the top of the element definition.

…prevent-trashing

brianteeman · 2018-08-13T16:59:07Z

administrator/language/en-GB/en-GB.com_workflow.ini

 COM_WORKFLOW_MSG_DELETE_DEFAULT="You are trying to delete the default item."
 COM_WORKFLOW_MSG_FROM_TO_STAGE="From Stage and To Stage must have different names."
 COM_WORKFLOW_MSG_DELETE_IS_ASSIGNED="This item is in use by the component."
+COM_WORKFLOW_MSG_DELETE_IS_DEFAULT="You can't delete the default item"


please add a . at the end

brianteeman · 2018-08-13T18:17:56Z

unable to test due to 500 error

Quy · 2018-08-14T05:50:38Z

administrator/language/en-GB/en-GB.plg_content_joomla.ini

 PLG_CONTENT_JOOMLA_FIELD_CHECK_CATEGORIES_LABEL="Check Category Deletion"
-PLG_CONTENT_JOOMLA_FIELD_CHECK_STATES_DESC="Check that states are not assigned to an item before they are deleted."
-PLG_CONTENT_JOOMLA_FIELD_CHECK_STATES_LABEL="Check States Deletion"
+PLG_CONTENT_JOOMLA_FIELD_CHECK_STATES_WORKFLOW_DESC="Check that stages/workflows are not assigned to an item before they are deleted."


Rename STATES

brianteeman · 2018-08-14T11:03:23Z

wilsonge · 2018-08-14T11:08:24Z

Looks like a merge bug? That string shouldn't exist in this branch (because this hasn't been synced since last night and JM only introduced that string with ea4064a today). Either way I just merged 4.0-dev into this PR again so that should be fixed?

bembelimen · 2018-08-14T11:13:26Z

I don't get this String either.

…prevent-trashing

ghost · 2018-09-08T09:51:53Z

I have tested this item 🔴 unsuccessfully on ff70dc6

What I've done:

created workflow with transitions and stages
created category and assign the new workflow
created an article and assign to the category
try to trash assigned workflow

expected result:
Can't trash the assigned workflow

actual result:
workflow can be trashed

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21579.}

lavipr · 2018-09-08T09:55:33Z

I have tested this item ✅ successfully on ff70dc6

testing steps:

created new custom workflow
created new article
used batch-function to assign the created workflow to the new created article
tried to delete this workflow

-> Error message appeared: "This item is in use by the component."

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21579.}

…prevent-trashing # Conflicts: # plugins/content/joomla/joomla.php

…mla-cms into workflow-prevent-trashing

Refactor the code

bembelimen · 2018-09-09T22:31:34Z

Thanks @f-hamel for this find, could you test again.
@wilsonge I moved the methods, is that ok?

wilsonge · 2018-09-09T23:56:26Z

libraries/src/Workflow/WorkflowServiceTrait.php


 defined('JPATH_PLATFORM') or die;

+use Joomla\CMS\Factory;


This change isn't needed

wilsonge · 2018-09-09T23:57:29Z

I'll do another review tomorrow. It's changed a lot since I first reviewed this

ghost · 2018-09-10T05:39:45Z

I have tested this item ✅ successfully on 10d8bc0

@bembelimen done ;-)

_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21579.}

ghost · 2018-09-10T05:51:22Z

@lavipr can you please retest?

lavipr · 2018-09-10T10:49:58Z

I have tested this item ✅ successfully on 10d8bc0

1. testing steps:
created new custom workflow
created new article
used batch-function to assign the created workflow to the new created article
tried to delete this workflow
-> Error message appeared: "This item is in use by the component."

testing steps:

created new category and assigned new workflow (from test 1) to it
created new article with new category
tried to delete this workflow
-> Error message appeared: "This item is in use by the component."
_{This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21579.}

ghost · 2018-09-10T11:21:27Z

Ready to Commit after two successful tests. @bembelimen please resolve conflicting Files, thanks.

…prevent-trashing # Conflicts: # administrator/language/en-GB/en-GB.com_workflow.ini

bembelimen · 2018-09-10T13:29:35Z

I resolved the conflict, but let's wait for @wilsonge review

…prevent-trashing # Conflicts: # plugins/content/joomla/joomla.php

wilsonge · 2018-12-11T00:16:58Z

Thanks :)

bembelimen added 3 commits August 13, 2018 05:07

Prevent trashing of active items

f64c9ca

Fix wrong method name

61d9c0b

Improve translation

6be26a1

bembelimen requested a review from brianteeman as a code owner August 13, 2018 03:25

joomla-cms-bot added Language Change This is for Translators PR-4.0-dev labels Aug 13, 2018

Improve translation

b25c0b3

wilsonge reviewed Aug 13, 2018

View reviewed changes

Quy reviewed Aug 13, 2018

View reviewed changes

plugins/content/joomla/joomla.php Outdated

}

return $result;

Copy link
Copy Markdown

Contributor

Quy Aug 13, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove blank line

Quy reviewed Aug 13, 2018

View reviewed changes

bembelimen added 4 commits August 13, 2018 18:08

Merge branch '4.0-dev' of github.com:joomla/joomla-cms into workflow-…

647092d

…prevent-trashing

Reorder xml structure

b1eebd6

Remove blank line

ab6369e

Order use statements

0a2cb77

bembelimen mentioned this pull request Aug 13, 2018

[4.0] [Workflow] Add checks for deletion of workflow/states/transitions #21514

Closed

brianteeman reviewed Aug 13, 2018

View reviewed changes

Add point

f21b84b

Merge branch '4.0-dev' into workflow-prevent-trashing

d4428d7

joomla-cms-bot changed the title ~~Workflow prevent trashing of used stages/workflows~~ [4.0] Workflow prevent trashing of used stages/workflows Aug 14, 2018

Quy reviewed Aug 14, 2018

View reviewed changes

brianteeman added the New Feature Workflow label Aug 14, 2018

wilsonge mentioned this pull request Aug 14, 2018

[4.0] Workflows - delete stage #21601

Closed

Merge branch '4.0-dev' into workflow-prevent-trashing

974c564

bembelimen added 2 commits August 14, 2018 13:20

Sniffer satisfaction

272fcf8

Sniffer satisfaction

fac863e

Merge branch '4.0-dev' of github.com:joomla/joomla-cms into workflow-…

e93e017

…prevent-trashing

lavipr mentioned this pull request Sep 8, 2018

[4.0] [RFC] Changing article category with different workflows #21973

Closed

wilsonge and others added 5 commits September 8, 2018 21:28

Merge branch '4.0-dev' into workflow-prevent-trashing

e762553

Merge branch '4.0-dev' of github.com:joomla/joomla-cms into workflow-…

d9b2b2d

…prevent-trashing # Conflicts: # plugins/content/joomla/joomla.php

Merge branch 'workflow-prevent-trashing' of github.com:bembelimen/joo…

e204edf

…mla-cms into workflow-prevent-trashing

Fix trashing over edit view

22b2446

Refactor the code

Remove unused string

9c066cc

Merge branch '4.0-dev' into workflow-prevent-trashing

10d8bc0

wilsonge reviewed Sep 9, 2018

View reviewed changes

joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 10, 2018

bembelimen added 2 commits September 10, 2018 15:26

Merge branch '4.0-dev' of github.com:joomla/joomla-cms into workflow-…

14fcc14

…prevent-trashing # Conflicts: # administrator/language/en-GB/en-GB.com_workflow.ini

Remove unused "use statement"

0843fbc

bembelimen and others added 3 commits September 22, 2018 06:31

Merge branch '4.0-dev' into workflow-prevent-trashing

4e68625

Merge branch '4.0-dev' of github.com:joomla/joomla-cms into workflow-…

5f31e9f

…prevent-trashing # Conflicts: # plugins/content/joomla/joomla.php

Merge branch '4.0-dev' into workflow-prevent-trashing

a9b60ff

wilsonge approved these changes Dec 10, 2018

View reviewed changes

wilsonge merged commit cc7e508 into joomla:4.0-dev Dec 11, 2018

joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 11, 2018

wilsonge added this to the Joomla 4.0 milestone Dec 11, 2018

bembelimen deleted the workflow-prevent-trashing branch December 30, 2018 23:56

Uh oh!

Conversation

bembelimen commented Aug 13, 2018

Summary of Changes

Testing Instructions

Expected result

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brianteeman commented Aug 13, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brianteeman commented Aug 14, 2018

Uh oh!

wilsonge commented Aug 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bembelimen commented Aug 14, 2018

Uh oh!

ghost commented Sep 8, 2018

Uh oh!

lavipr commented Sep 8, 2018

Uh oh!

bembelimen commented Sep 9, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilsonge commented Sep 9, 2018

Uh oh!

ghost commented Sep 10, 2018

Uh oh!

ghost commented Sep 10, 2018

Uh oh!

lavipr commented Sep 10, 2018

Uh oh!

ghost commented Sep 10, 2018

Uh oh!

bembelimen commented Sep 10, 2018

Uh oh!

wilsonge commented Dec 11, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

wilsonge commented Aug 14, 2018 •

edited

Loading