[#40176] - [5.3] Allow articles with missing workflow data
- Pending
- Medium
- Build: 4.3-dev
- # 40176
- Diff
- Denitz:load_workflows_on_demand
User tests: Successful: Unsuccessful:
Summary of Changes
We need to fix these problems:
- Articles imported into
#__contenttable are not displayed in backend because currentArticlesModelhas a weirdINNER JOINon#__workflow_associationstable which can not contain the data for the articles imported manually. - With enabled workflow feature, the article association in
#__workflow_associationstable is not automatically re-created on article save. - I am sure that most of Joomla users are not using workflows, hence we need to use
#__workflow_XXXtables only on demand. Similar like for associations. - It will de-couple articles from #__workflow tables because we don't need this relationship if workflows are not used. Imho, the normal idea was to add
workflow_stage_idcolumn into#__content tableand don't create#__workflow_associationswhich contains the mixed data from multiple extensions and makes a spaghetti in the database. Anyway, it's not the topic for this PR.
Testing Instructions
Import articles into #__content table directly.
These articles are not displayed in backend with/without workflow enabled.
Enable workflow and try to edit such article directly via ID in URL, save article - still no displayed.
Actual result BEFORE applying this Pull Request
No articles are displayed in backend.
No items are re-created in #__workflow_associations after article save with enabled workflow feature.
Expected result AFTER applying this Pull Request
All fine.
joomla-cms-bot
- | Category | ⇒ | Administration com_content Libraries |
| Status | New | ⇒ | Pending |
@brianteeman I would totally disagree, what for do we need this extra dependency on #__workflow_associations table even if we don't use workflows?
You have a j3 site with a thousand articles. You upgrade to j4 and decide to use workflows. With this PR you can not do that unless you open and save all thousand articles
No.
This PR doesn't affect this case. It doesn't block creation of #__workflow_associations records for articles if workflow is disabled. It works event better and auto-creates missing records to minimize possible data loss.
How does it auto-create the missing record without the article being opened and saved again?
Honestly just fix the script you are using to import the articles
Two purposes:
- Don't load workflows data if workflows are not enabled.
- Auto-create missing workflow association on save of existing article.
It helps to fix the possible issues with inconsistent database data. Otherwise, if article record is missed in#__workflow_associations, the user won't even see this article in backend.
Maybe we also need to always have records in #__content_rating table for all articles and INNER JOIN on it?
Exactly as I said. You cannot use workflow unless you open and save again all thousand articles
At least a user will be to do it.
| Category | Administration com_content Libraries | ⇒ | Unit Tests Repository Administration com_admin SQL |
| Labels |
Added:
bug
PR-4.3-dev
|
||
| Title |
|
||||||
| Title |
|
||||||
| Category | Administration Unit Tests Repository com_admin SQL | ⇒ | Administration com_content Libraries |
I think the conditional join in the articles model is fine - that's how things already work for other extensions like voting and associations.
The part in default.php is also totally fine
I think the problem is in the workflow trait. This could be triggered in the frontend and be run on unlimited sites. I really don't think this is the right place to have it if you want to create associations. It's no different to the fact we don't magically create assets either. I'd remove it from here and it's fine. If we don't remove it I'm against this being merged in the current form. I'd be happy enough if it was some sort of backend status check.
Nice improvements.
Still I agree with @wilsonge here, we should not try to fix workflow associations when opening the article (read: the changes in the trait should not be done) but have a way to recover from a e.g. "broken" import without associations. All the other fixes are good, but could you (@Denitz ) enhance this PR to use the database repair to fix associations? (This could later be enhanced to also fix menus, categories, asset structures).
This pull request has been automatically rebased to 5.2-dev.
| Title |
|
||||||
This pull request has been automatically rebased to 5.3-dev.
| Title |
|
||||||
| Labels |
Added:
Feature
Updates Requested
PR-5.3-dev
Removed: bug PR-4.3-dev |
||
System tests are failing with PHP warning:
Undefined property: stdClass::$stage_title in /tests/www/cmysql/administrator/components/com_content/tmpl/articles/default.php on line 169. That could be caused by the changes of this PR in the articles model.
@richard67 as stated by others the failing system tests are the least of the issue with this PR
@richard67 as stated by others the failing system tests are the least of the issue with this PR
@brianteeman Sure. I just wanted to document them here.
Undefined property: stdClass::$stage_title in /tests/www/cmysql/administrator/components/com_content/tmpl/articles/default.php on line 169
Fixed.
The problem is with the import script being used not with the core code. This pr is obviously wrong