? ? Pending

User tests: Successful: Unsuccessful:

avatar bembelimen
bembelimen
19 Aug 2018

Summary of Changes

Change hardcoded state calls to the new condition constants

Testing Instructions

  • Create mod_stats or mod_related_items modules and check the numbers
  • Search content in frontend
  • Check pagenav in article view
  • They should work now like in J! 3 again.
avatar bembelimen bembelimen - open - 19 Aug 2018
avatar bembelimen bembelimen - change - 19 Aug 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 19 Aug 2018
Category Modules Front End Plugins
avatar bembelimen bembelimen - change - 19 Aug 2018
Labels Added: ?
avatar bembelimen bembelimen - change - 19 Aug 2018
Title
Implement condition query for frontend modules/plugins
[4.0] Implement condition query for frontend modules/plugins
avatar bembelimen bembelimen - edited - 19 Aug 2018
avatar bembelimen
bembelimen - comment - 23 Aug 2018

@brianteeman could you please tag this PR, too? Thx.

avatar brianteeman
brianteeman - comment - 23 Aug 2018

tagged as requested.

avatar bembelimen bembelimen - change - 24 Aug 2018
Labels Added: ?
avatar bembelimen bembelimen - change - 5 Sep 2018
The description was changed
avatar bembelimen bembelimen - edited - 5 Sep 2018
avatar csthomas
csthomas - comment - 10 Sep 2018

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 [...]
avatar chmst
chmst - comment - 10 Sep 2018

I have tested this item successfully on d5aaf5f

Tested successfuly for all modules mentioned in testing instructions.


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

avatar chmst chmst - test_item - 10 Sep 2018 - Tested successfully
avatar bembelimen
bembelimen - comment - 10 Sep 2018

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.

avatar ggppdk
ggppdk - comment - 10 Sep 2018

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"

avatar csthomas
csthomas - comment - 10 Sep 2018

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

avatar csthomas
csthomas - comment - 10 Sep 2018

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:)

avatar bembelimen
bembelimen - comment - 10 Sep 2018

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

avatar ggppdk
ggppdk - comment - 10 Sep 2018

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 ?

avatar bembelimen
bembelimen - comment - 10 Sep 2018

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.

avatar csthomas
csthomas - comment - 11 Sep 2018
  1. 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.

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

avatar ggppdk
ggppdk - comment - 11 Sep 2018

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

  • when viewing,
  • when doing accounting calculation, etc

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

avatar alikon
alikon - comment - 11 Sep 2018

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

avatar bembelimen bembelimen - change - 22 Nov 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-11-22 12:33:02
Closed_By bembelimen
avatar bembelimen bembelimen - close - 22 Nov 2018

Add a Comment

Login with GitHub to post a comment