User tests: Successful: Unsuccessful:
Change hardcoded state calls to the new condition constants
Status | New | ⇒ | Pending |
Category | ⇒ | Modules Front End Plugins |
Labels |
Added:
?
|
Title |
|
tagged as requested.
Labels |
Added:
?
|
Adding workflow tables to each sql query will slow down joomla 4 on front end.
As #__content.state
is a copy of wf.condition
why can not we stay with something like
a.state = ContentComponent::CONDITION_PUBLISHED
.
IMO workflow staff should be loaded optionally, I mean workflow tables should not be used in the sql WHERE
clause. The database should first find and sort the rows, and finally add the workflow columns to the result.
Example using subquery
SELECT old.*, wa.*, ws.*
( SELECT * FROM #__content INNER JOIN #__categories [...] WHERE [...] ORDER BY [...] LIMIT 30) AS old
LEFT JOIN #__workflow_associations wa [...]
LEFT JOIN #__workflow_stages ws [...]
I have tested this item
Tested successfuly for all modules mentioned in testing instructions.
Hi @csthomas thanks for your feedback.
Yes I'm aware (ofc) that more JOINs make queries slower. Do you see a big impact although we're using keys?
Could you show some benchmarks how big th eimpact is?
The state field of com_content should be viewed as "deprecated", it's only there because of B/C reasons but is not used by the workflow in any case.
The state field of com_content should be viewed as "deprecated", it's only there because of B/C reasons but is not used by the workflow in any case.
So the workflow tables need to be used when listing / querying the records of components that use workflow ?
Could we not say that it is not there for B/C,
but that it will stay there for performance / simplicity too ? and because workflow is "optional"
I manage website with 400k articles, 140k of them are published now.
Now on upgrade to 4.0 I will have to create 400k rows in #__workflow_associations
. It will be long process...
So the workflow tables need to be used when listing / querying the records of components that use workflow ?
Joomla goes in this direction. If you want to read article you have to ask "a workflow" for permission:)
Joomla goes in this direction. If you want to read article you have to ask "a workflow" for permission:)
Not the workflow, but in the future, the stage/state of the article should matter, yes. At the moment everyone can edit an article all the time with edit rights. In the future only people with permission based on the stage will be able to edit. For example if you're responsible for proofreading, you should be able to edit an article when it's unpublished but not when it's published and so on.
So you need this "mapping".
For example if you're responsible for proofreading, you should be able to edit an article when it's unpublished but not when it's published and so on.
ok,
when editing the record
and / or changing stage of the record
and / or "modifying" the record in some way
and / or modifying "related" data of the record,
then i understand the argument of needing to check workflow
but is workflow required to be checked when viewing the records too ?
later it will effect viewing too ?
but is workflow required to be checked when viewing the records too ?
later it will effect viewing too ?
I think that was a joke from @csthomas viewing is based on the Access level (view levels) of the article and if it's published. That makes no sense to solve it over the workflow.
The workflow has several tables, queries to #__workflow_stages
or #__workflow_associations
for check published state is IMO too much.
To view the article(-s) we should use the fastest way.
I would like to replace:
->where('ws.condition = ' . (int) ContentComponent::CONDITION_PUBLISHED)
by:
->where('a.state = ' . (int) ContentComponent::CONDITION_PUBLISHED)
and only left join to #__workflow_stages
and #__workflow_associations
.
Why we use #__workflow_associations
at all?
There is a many-to-many sql relation:
#__content
-> #__workflow_associations
-> #__workflow_stages
but one article can have only one workflow_stage
.
Please consider to add workflow_stage_id
column to #__content
and remove the table #__workflow_associations
.
Workflow feature is very nice,
but it should stay out of "usage" queries as much as possible
not polluting them without a real necessity to do so
at most we should have an API call to get the viewable conditions
->where('a.state IN (' . implode(',' SOMEAPICALL) . ')'
or this
->where('a.state = ' . (int) ContentComponent::CONDITION_PUBLISHED)
About state being a deprecated B/C thing, i would not agree,
workflow should continue set to a condition into the state column in the future, without having the column removed !!
and the workflow tables should not be required at all
they should be used only when finding current workflow information to show in management,
when doing transitions, etc
not when using the content
it feels really wrong
the joke was merging the workflow in the 4.x branch to begin with
...looking for a permanent ban...
personal opinion is not allowed in this project
Status | Pending | ⇒ | Closed |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2018-11-22 12:33:02 |
Closed_By | ⇒ | bembelimen |
@brianteeman could you please tag this PR, too? Thx.