Skip to content

Conversation

@bembelimen
Copy link
Contributor

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

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

Choose a reason for hiding this comment

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

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

Copy link
Contributor

Choose a reason for hiding this comment

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

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

use Joomla\Component\Content\Administrator\Table\ArticleTable;
use Joomla\CMS\Workflow\Workflow;
use Joomla\Utilities\ArrayHelper;
use Joomla\Component\Workflow\Administrator\Table\StageTable;
Copy link
Contributor

Choose a reason for hiding this comment

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

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

}

return $result;

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove blank line

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"
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

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"
Copy link
Contributor

Choose a reason for hiding this comment

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

please add a . at the end

@brianteeman
Copy link
Contributor

unable to test due to 500 error

@joomla-cms-bot 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
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."
Copy link
Contributor

Choose a reason for hiding this comment

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

Rename STATES

@brianteeman
Copy link
Contributor

chrome_2018-08-14_12-02-58

@wilsonge
Copy link
Contributor

wilsonge commented Aug 14, 2018

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
Copy link
Contributor Author

I don't get this String either.

@ghost
Copy link

ghost commented Sep 8, 2018

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
Copy link
Contributor

lavipr commented Sep 8, 2018

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.

@bembelimen
Copy link
Contributor Author

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


defined('JPATH_PLATFORM') or die;

use Joomla\CMS\Factory;
Copy link
Contributor

Choose a reason for hiding this comment

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

This change isn't needed

@wilsonge
Copy link
Contributor

wilsonge commented Sep 9, 2018

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

@ghost
Copy link

ghost commented Sep 10, 2018

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
Copy link

ghost commented Sep 10, 2018

@lavipr can you please retest?

@lavipr
Copy link
Contributor

lavipr commented Sep 10, 2018

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."

  1. testing steps:

@ghost
Copy link

ghost commented Sep 10, 2018

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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Sep 10, 2018
@bembelimen
Copy link
Contributor Author

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

@wilsonge wilsonge merged commit cc7e508 into joomla:4.0-dev Dec 11, 2018
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Dec 11, 2018
@wilsonge wilsonge added this to the Joomla 4.0 milestone Dec 11, 2018
@wilsonge
Copy link
Contributor

Thanks :)

@bembelimen bembelimen deleted the workflow-prevent-trashing branch December 30, 2018 23:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Language Change This is for Translators

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants