-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unpublish/Run transition article when saving as copy #45014
base: 5.3-dev
Are you sure you want to change the base?
Conversation
Second attempt at fixing joomla#37351. Previous PR (joomla#38810) was reverted (in joomla#41790) because it would not work if Workflow was enabled. This super simple PR checks if workflows are enabled and if so set a transition value of 1. If workflows not enabled, it sets state to 0, as in joomla#38810.
administrator/components/com_content/src/Model/ArticleModel.php
Outdated
Show resolved
Hide resolved
Much as I hate to say add an option for this changed behaviour in this case I think it is necessary as the behaviour has been not to unpublish for such a long time (even if that was a bug) that it would be a disruptive change in expected behaviour |
Hi I'd be with @crystalenka here (#38810 (comment)) and say that this change reverts to a "safe" behavior, hence I don't see the need for a setting. When this was changed from "unpublish on copy" to "keep published on copy", it may have needed a setting, as the change was disruptive and would cause invalid content to be published. None was added at the time. Going back means at worst the article is not going to be published. But that would hardly go unnoticed as I'd expect "Save copy" was done on purpose, to modify the original content. So I'd vote to keep it simple and jsut revert to the safe behavior. |
@weeblr Could you check the log of the PHPCS check here https://ci.joomla.org/joomla/joomla-cms/82592/1/7 ? It reports a syntax error. |
Sorry, fixed now. |
@weeblr No need for sorry, it can happen to the best. Thanks for contributing. |
I have tested this item ✅ successfully on dc24644 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45014. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45014. |
I have tested this item 🔴 unsuccessfully on dc24644 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45014. |
chrome_gui5xWudwZ.mp4opened an article for editing |
I have tested this item 🔴 unsuccessfully on dc24644 Without pull request
With pull requestSame as without pull request. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45014. |
administrator/components/com_content/src/Model/ArticleModel.php
Outdated
Show resolved
Hide resolved
@brianteeman @fgsw Which Joomla version are you testing this on? |
@brianteeman @fgsw Forget my last question, I did a quick change late yesterday and used the wrong variable name. I have fixed that now. |
I have tested this item 🔴 unsuccessfully on 16e2056 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45014. |
I use the prebuilt package custom update server. |
Suspected there'd be something like that coming up. In essence, we should not set to unpublish but to the starting stage I guess. Starting to be tricky and requires much more work. Maybe the thing to do at this stage is to only unpublish if workflow is not enabled and be done with it. |
that would be my preference together with completely removing workflow |
from the CMS ? |
yes |
…ardcoded value for transition
Well, won't comment on that. For now I have found that I can easily revert to Basic stage by setting the transition to an empty string instead of the hardcode 1. This works for me with the Blog sample data, workflow enabled or not. |
Pull Request for Issue #37351
Summary of Changes
Second attempt at fixing #37351.
Previous PR (#38810) was reverted (in #41790) because it would not work if Workflow was enabled.
This super simple PR checks if workflows are enabled and if so set a transition value of 1. If workflows not enabled, it sets state to 0, as in #38810.
Testing Instructions
Repeat 1-3 with Workflow enabled and disabled (System -> Global Configuration -> Articles -> Integrations -> Workflow.
Result should be the same.