Skip to content

Conversation

@infograf768
Copy link
Member

@infograf768 infograf768 commented Aug 11, 2018

Pull Request for Issue #21548

Summary of Changes

Adapting multilingual sample data to workflow

Testing Instructions

Patch tester should be OK.
Just do as usual, i.e. install some languages (French, German, Persian) and click install in CPanel

Why is it a POC?

For the simple reason that the default workflow may have been changed or the "Joomla! Default" one in core modified (WHICH BTW should have as title a translatable language constant), thus state 2 could be anything instead of "Published".

@bembelimen
@franz-wohlkoenig

@ghost
Copy link

ghost commented Aug 11, 2018

will test Today.

@ghost
Copy link

ghost commented Aug 11, 2018

I have tested this item ✅ successfully on b1776f3

Got on Blog (maybe Part of #21551):
screen shot 2018-08-11 at 12 30 25


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/21553.

->select($db->quoteName('id'))
->from($db->quoteName('#__workflows'))
->where($db->quoteName('extension') . ' = ' . $db->quote('com_content'))
->where($db->quoteName('title') . ' = ' . $db->quote('Joomla! Default'));
Copy link
Contributor

Choose a reason for hiding this comment

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

I would not check for the title of the workflow but for the column default, so you get a default workflow, when the user changes it. But you don't need any workflow ID in the category itself (which I wouldn't set), because the default is always the fallback.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, if I choose the column default and it is not containing the published state as I wish, what can I do?

'published' => 1,
'access' => 1,
'params' => '{"target":"","image":""}',
'params' => '{"target":"","image":"", "workflow_id":"' . $workflowId . '"}',
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't set the workflow_id here. Just let it away, so the default will be used.

Copy link
Member Author

Choose a reason for hiding this comment

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

If I do not need the workflow id, then I do not need either the query as its only purpose is to get that id.
What I need here is definitely to set the articles to Published, and set the category to use the correct workflow id where Published is the default or can be obtained.

But what if another workflow is set as default and it is not using Published as its default state?
screen shot 2018-08-12 at 08 28 11

That one has Unpublished as default state
screen shot 2018-08-12 at 08 30 39

Therefore, basically, IF workflows IS enabled, I want to force using the default Joomla defaults which should NEVER be modifed or deleted. But we get there into other issues as we do not want save2copy to autopublish the copies, etc. which means Joomla Default workflow is not fit for save2copy....
A vicious circle...


$query->clear()
->insert($db->qn('#__workflow_associations'))
->values($newId . ', 2,' . $db->quote('com_content'));
Copy link
Contributor

@bembelimen bembelimen Aug 11, 2018

Choose a reason for hiding this comment

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

You should not use 2 as hardcode value, but you should take the default state (it's a column, too) from the pool of states which is part of the default-workflow (from above)

Copy link
Member Author

Choose a reason for hiding this comment

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

what if that default state is NOT "published"?

Copy link
Member Author

@infograf768 infograf768 Aug 11, 2018

Choose a reason for hiding this comment

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

looks like we have first to find which workflow is the default, and then which states are proposed by this default workflow, and then the correct condition "1" (for published) from which we have to find the id to use. In our default Joomla, that id is 2

@infograf768
Copy link
Member Author

  • evidently first check is workflows are Enabled...

@brianteeman
Copy link
Contributor

There is NO option to disable/enable workflows. You have no choice - this workflows pr completely replaces the current working system.

That is why I am so insistent that the code is correct. The only reason that the current/previous states still exist in the com-content table is for b/c with extensions eg content-modules. Although that is not true either as it will only write published/unpublished/trashed into that table and will not write archived.

The option that you are thinking of works the same as the one for custom fields. ie it hides/shows the menu admin links. The logic being that once you have workflows setup then you don't need to see it again. Yes the language string for both needs to be improved.

If people spent the time to truly test this workflows stuff in a real world scenario they would see why it is not ready at all and will require fundamental changes before it can even be considered. The fact that it ignores the ACL is the most critical to me.

@alikon
Copy link
Contributor

alikon commented Aug 12, 2018

after applying this pr still issues
screenshot from 2018-08-12 12-16-59

@chmst
Copy link
Contributor

chmst commented Aug 12, 2018

I had this too until I have deleted the autoload file and repeated the composer / npm stuff.

@alikon
Copy link
Contributor

alikon commented Aug 12, 2018

i've used nigthly build too..... so....there should be no need to run composer ect .i'm afraid something is still wrong after #21544

@Bakual
Copy link
Contributor

Bakual commented Aug 12, 2018

There is NO option to disable/enable workflows. You have no choice - this workflows pr completely replaces the current working system.

There is an option in the com_content options. But all it does is hiding the workflow menuitem. Unfortunately, it doesn't disable workflows itself as you said.
And this is a fundamental problem with workflows. Because most users will not need it at all and the chance to mess up something there is big. Fixing it once it is broken needs full understanding of the inner workings. It's not intuitive at all if you're not a developer.

@brianteeman
Copy link
Contributor

We were told at jab that out of the box it would work the same as the status quo. Sadly that is clearly not the case at all especially with it ignoring the ACL

@Bakual
Copy link
Contributor

Bakual commented Aug 12, 2018

with it ignoring the ACL

I brings its own ACL in the workflow and the transitions, making the ACL in the component itself quite redundant. Good luck with finding why someone suddenly can't publish thought.

@infograf768
Copy link
Member Author

@alikon
See #21566 for new fixes...

@joomla-cms-bot joomla-cms-bot changed the title [4.0][WORKFLOW] POC fixing multilingual sample data [4.0][Workflow] POC fixing multilingual sample data Aug 19, 2018
@ghost
Copy link

ghost commented Aug 24, 2018

is this PR waiting for a second Test or something else?

@infograf768
Copy link
Member Author

is this PR waiting for a second Test or something else?

Responsible for 4.0 development should decide.

I personally think we should wait a general solution to workflow.
For example: what happens if a workflow, including the default one, has been deleted?

@infograf768
Copy link
Member Author

Closing as another code has taken care of this.

@infograf768 infograf768 deleted the 4.0_workflow_multi_sampledata branch October 13, 2018 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants