Skip to content
This repository has been archived by the owner on Mar 17, 2020. It is now read-only.

MASTER PR DO NOT MERGE #156

Open
wants to merge 352 commits into
base: 4.0-dev
Choose a base branch
from
Open

MASTER PR DO NOT MERGE #156

wants to merge 352 commits into from

Conversation

bembelimen
Copy link
Contributor

This PR exists to see the changes for the official PR to the Joomla! repo

DO NOT MERGE YET

Jan Jaracz and others added 30 commits August 2, 2017 02:53
…modified date in list workflows view, Fix error in installation script
* Remove "archived" status

* Update workflow.xml

* Update transition.xml
* Remove "archived" status (#69)

* Remove "archived" status

* Update workflow.xml

* Update transition.xml

* Rename the default workflow (#68)

* Implement frontend filter

* Fix published
Set archived as published condition
* reordering states' condition numbers

* update state.xml accordingly

* fixing enum issue in sql script

* updating front-end content component according to back-end condition value changes

* change trash condition value to -2

* adding missed where clause for sql
Buddhima and others added 28 commits October 22, 2017 14:49
# Conflicts:
#	administrator/components/com_categories/tmpl/categories/default.php
#	administrator/components/com_content/Model/ArticlesModel.php
#	administrator/components/com_content/tmpl/articles/default.php
#	components/com_content/Model/ArchiveModel.php
#	components/com_content/Model/ArticlesModel.php
#	components/com_content/tmpl/category/blog.php
#	components/com_content/tmpl/category/blog_item.php
#	components/com_content/tmpl/featured/default.php
#	installation/sql/mysql/joomla.sql
#	media/system/js/core.min.js
# Conflicts:
#	administrator/components/com_content/Model/ArticlesModel.php
#	administrator/components/com_content/tmpl/articles/default.php
#	administrator/index.php
#	administrator/modules/mod_menu/tmpl/default.php
#	administrator/templates/atum/js/template.js
#	components/com_content/Model/ArchiveModel.php
#	components/com_content/Model/ArticlesModel.php
#	components/com_content/forms/filter_articles.xml
#	components/com_content/tmpl/category/blog_item.php
#	grunt-settings.yaml
#	index.php
#	installation/index.php
#	installation/sql/mysql/joomla.sql
#	media/system/js/core.min.js
#	media/vendor/tinymce/changelog.txt
#	media/vendor/tinymce/plugins/help/plugin.min.js
#	media/vendor/tinymce/plugins/image/plugin.js
#	media/vendor/tinymce/plugins/image/plugin.min.js
#	media/vendor/tinymce/plugins/imagetools/plugin.js
#	media/vendor/tinymce/plugins/imagetools/plugin.min.js
#	media/vendor/tinymce/plugins/nonbreaking/plugin.js
#	media/vendor/tinymce/plugins/nonbreaking/plugin.min.js
#	media/vendor/tinymce/plugins/paste/plugin.js
#	media/vendor/tinymce/plugins/paste/plugin.min.js
#	media/vendor/tinymce/plugins/table/plugin.js
#	media/vendor/tinymce/plugins/table/plugin.min.js
#	media/vendor/tinymce/plugins/template/plugin.js
#	media/vendor/tinymce/plugins/template/plugin.min.js
#	media/vendor/tinymce/plugins/visualchars/plugin.js
#	media/vendor/tinymce/plugins/visualchars/plugin.min.js
#	media/vendor/tinymce/themes/inlite/theme.js
#	media/vendor/tinymce/themes/inlite/theme.min.js
#	media/vendor/tinymce/themes/mobile/theme.js
#	media/vendor/tinymce/themes/mobile/theme.min.js
#	media/vendor/tinymce/themes/modern/theme.js
#	media/vendor/tinymce/themes/modern/theme.min.js
#	media/vendor/tinymce/tinymce.js
#	media/vendor/tinymce/tinymce.min.js
#	package.json
#	plugins/editors/tinymce/tinymce.xml
#	templates/cassiopeia/css/template.css
#	templates/cassiopeia/js/template.js
Move transition action language string to the Joomla! core language file
…ates or only the active one

Add extension filter parameter to WorflowState field
Remove inline-form class to archive view
Renamed archived module
…ith_associations

Update archive article with associations
update admin mod_latest to use workflow_state
Implement email notification for transition execution
# Conflicts:
#	administrator/language/en-GB/en-GB.com_content.ini
#	media/system/js/core.min.js
#	modules/mod_articles_archive/Helper/ArticlesArchiveHelper.php
Copy link
Member

@nibra nibra left a comment

Choose a reason for hiding this comment

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

You're missing the opportunity to separate the workflow from the components it is applied to. Although not all events exist, that might be needed to decouple things properly, the Workflow code should be in its own classes, like Workflow\Transit and Workflow\Filter. The trails in the affected components should be reduced to a single call with all needed parameters, so the pollution is minimal. Refactoring to event based workflow will be much easier then.

@@ -499,7 +493,7 @@ public function save($data)
$isNew = true;
$context = $this->option . '.' . $this->name;

if (!empty($data['tags']) && $data['tags'][0] != '')
if ((!empty($data['tags']) && $data['tags'][0] != ''))
{
$table->newTags = $data['tags'];
}
Copy link
Member

Choose a reason for hiding this comment

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

The changes in this file seem not to be related to Workflow.

}

if ($article->state == -2)
if ($article->condition == -2)
{
$item->count_trashed = $article->count;
}
Copy link
Member

Choose a reason for hiding this comment

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

This approach requires changes when states/conditions are added or removed. Better:
Define constants, eg.

class WorkflowConditions
{
    const PUBLISHED   =  1;
    const UNPUBLISHED =  0;
    const ARCHIVED    =  2;
    const TRASHED     = -2;
}

The loop should look like

$item->count = [];
foreach ($articles as $article)
{
    $item->count[$article->condition] = ($item->count[$article->condition] ?: 0) + $article->count;
}

In subsequent code, you'll access

$item->count[WorkflowConditions::PUBLISHED]

instead of

$item->count_published

@@ -328,23 +383,31 @@ public function getForm($data = array(), $loadData = true)
$id = $jinput->get('a_id', $jinput->get('id', 0));

// Determine correct permissions to check.
if ($this->getState('article.id'))
if ($id = $this->getState('article.id', $id))
Copy link
Member

Choose a reason for hiding this comment

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

Never use assignment in a condition.

->where($db->qn('ws.default') . ' = 1')
->where($db->qn('ws.workflow_id') . ' = ' . $db->qn('w.id'))
->where($db->qn('w.published') . ' = 1')
->where($db->qn('ws.published') . ' = 1')
Copy link
Member

@nibra nibra Jan 18, 2018

Choose a reason for hiding this comment

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

Put he common part of the query before line 1033.

# Conflicts:
#	administrator/components/com_content/access.xml
#	administrator/components/com_menus/presets/joomla.xml
#	administrator/components/com_menus/presets/modern.xml
#	administrator/language/en-GB/en-GB.com_content.ini
#	administrator/language/en-GB/en-GB.ini
#	components/com_content/Model/ArchiveModel.php
#	media/system/js/core.min.js
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants