Issue #3311472 Set minimum stability for form
Closes #3311472
Merge request reports
Activity
added 2 commits
added 2 commits
added 1 commit
- 3152923d - only add rule when not cron, Probably not correct solution
84 85 $rules[] = ForbidDowngrade::class; 85 86 // ...and in the same major version as the installed version... 86 87 $rules[] = MajorVersionMatch::class; 87 // ...and it must be a known, secure, installable release. 88 // ...and it must be a known, secure, installable release... 88 89 $rules[] = TargetVersionInstallable::class; 90 if (!$stage instanceof ConsoleUpdateStage) { 91 // ...and must be either a release candidate, or stable. 92 $rules[] = TargetVersionNotPreRelease::class; 93 } - Comment on lines +90 to +93
Surround this in an
ifgets the tests in\Drupal\Tests\automatic_updates\Kernel\StatusCheck\VersionPolicyValidatorTest::testCronPreCreateto pass but it raises some questions for me about how we set some of this up.In
validateVersion()we only add cron specific rules if$stage instanceof ConsoleUpdateStage && $mode !== CronUpdateRunner::DISABLED. So this means if you callConsoleUpdateStage::create()directly you won't get any of the cron specific rules.VersionPolicyValidatorTest::testCronPreCreateactually depends on this because we assert we don't get an error if you use theConsoleUpdateStage, cron is disabled and the target is alpha. There may be reason we did this but it seems very strange now.-
ConsoleUpdateStageis not actually marked as internal so callingcreatedirectly is not obviously discouraged. I think we should probably mark this an internal. I guess you would want to check for it in a validator but you should never call any methods on it directly. - In
ConsoleUpdateStage::performUpdate()we do return early if cron is disabled so when used how intended you should never be able perform operations via this stage that don't have the cron specific rules applied even we don't enforce if cron is disabled. - We could throw a logic exception in
ConsoleUpdateStage::create()before we call the parent if cron is disabled. But we really only would have to do this we haveUpdateStage::beginthat callStageBase::createinstead of just overridingcreateand adding the an extra optional(which would be enforced to have a value)$project_versions. Then instead ofConsoleUpdateStage::beginthrowing aBadMethodCallExceptionwe could haveConsoleUpdateStage::createthrow that exception.
-
-
Added a todo to https://www.drupal.org/project/automatic_updates/issues/3398782 I think we can leave the if in for now and remove it in the follow-up
