Fix extensions in actualize_script#5243
Conversation
Follow-up of FreshRSS#3440 The hook was called before registering all the extensions for the current user
|
@aledeg If you are around, do you happen to have a test case for that? |
|
|
||
| Minz_ExtensionManager::callHook('freshrss_user_maintenance'); | ||
|
|
||
| $app->init(); |
There was a problem hiding this comment.
User extensions are first registered in $app->init();
Not at the moment. I was meaning to create one to do backend stuff but never took the time. |
And remove 5-year old legacy format of enabled extensions < FreshRSS 1.11.1
Alkarex
changed the title
| $ext_list = array_filter($ext_list, function($key) use($type) { | ||
| // Remove from list the extensions that have disappeared or changed type | ||
| $extension = Minz_ExtensionManager::findExtension($key); | ||
| return $extension !== null && $extension->getType() === $type; | ||
| }, ARRAY_FILTER_USE_KEY); |
There was a problem hiding this comment.
We had a case of when extensions were deleted, they would stay enabled for ever in the configuration.
There was a problem hiding this comment.
This would also do strange things with extensions that have changed type (e.g. enable an extension, which should not)
| */ | ||
| public static function init() { | ||
| public static function init(): void { | ||
| self::reset(); |
There was a problem hiding this comment.
During a cron job to refresh feeds, extensions enabled by a previous user risked being enabled for another user.
Furthermore, system extensions were enabled multiple times.
| if ($onlyOfType !== null && $ext->getType() !== $onlyOfType) { | ||
| // Do not enable an extension of the wrong type | ||
| return; |
There was a problem hiding this comment.
In the case of extensions changing type (from user to system or vice versa), we risked enabling wrongly or two times an extension
| if ($res === true) { | ||
| $ext_list = $conf->extensions_enabled; | ||
| $legacyKey = array_search($ext_name, $ext_list, true); | ||
| if ($legacyKey !== false) { //Legacy format FreshRSS < 1.11.1 |
There was a problem hiding this comment.
Removed support for 5 year-old configuration syntax for FreshRSS < 1.11.1.
Automatically updated by just saving the configuration
| /** | ||
| * List of enabled extensions. | ||
| */ | ||
| private $extensions_enabled = []; | ||
|
|
||
| public function removeExtension($ext_name) { | ||
| unset($this->extensions_enabled[$ext_name]); | ||
| $legacyKey = array_search($ext_name, $this->extensions_enabled, true); | ||
| if ($legacyKey !== false) { //Legacy format FreshRSS < 1.11.1 | ||
| unset($this->extensions_enabled[$legacyKey]); | ||
| } | ||
| } | ||
| public function addExtension($ext_name) { | ||
| if (!isset($this->extensions_enabled[$ext_name])) { | ||
| $this->extensions_enabled[$ext_name] = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
Apparently dead code, unused
| * @return void | ||
| */ | ||
| abstract public function init(); |
There was a problem hiding this comment.
Note: We should not change the signature of methods supposed to be overridden
|
Ping @marienfressinaud also, if you could give a quick try at this PR with your extensions |
|
Let's merge to start testing a bit more widely (Marien is not available in the coming days but will test a bit later) |
|
It took me some time, but it seems to work fine for my extensions 👍 |
|
Excellent :-) I will try to prepare a new release in the coming days |
3 participants
Follow-up of #3440
Fix multiple bugs:
Contributes to #4112