@sgiehl opened this Pull Request on June 18th 2018 Member

We already added check at various places. This aims to complete it:

  • Disallow usage of Marketplace
  • Disables GeoIP Autoupdate tasks
  • Disables GeoLocation Auto Updater UI

Is anything still missing?

refs #6324

@diosmosis commented on June 20th 2018 Member

Did a find usages of Http class and found some other possible candidates for this check:

CoreAdminHome\Tasks::updateSpammerBlacklist()
Referrers\Tasks::updateSocials()
Referrers\SearchEngines::updateSearchEngines()
maybe the MobileMessaging plugin?

@diosmosis commented on June 20th 2018 Member

Noticed that the 'Marketplace' link is displayed even if internet access is disabled:

image

Think we should remove it since it will never work?

@sgiehl commented on June 22nd 2018 Member

CoreAdminHome\Tasks::updateSpammerBlacklist()
Referrers\Tasks::updateSocials()
Referrers\SearchEngines::updateSearchEngines()

Those tasks should already only be added when internet is enabled

@sgiehl commented on June 22nd 2018 Member

regarding MobileMessaging and Marketplace: Wondering if we maybe should simply "deactivate" those plugins if internetfeatures are disabled.

@diosmosis commented on June 24th 2018 Member

If they're deactivated, will they be activated when the setting is enabled? I guess we could just unload it at the start of a request, might have the same effect...

@sgiehl commented on June 24th 2018 Member

Marketplace and MobileMessaging plugins will now be dynamically removed from the list of activated plugins in each request if internet connection is disabled in config.

@diosmosis commented on June 25th 2018 Member

@sgiehl thought of one more thing, would it be a good idea to mention if a plugin requires internet in the manage plugin page?

Btw @mattab some API/product changes were added here (new API to Plugin class and product change in that plugins requiring an internet connection are unloaded), can you provide your opinion?

@sgiehl commented on June 25th 2018 Member

@sgiehl thought of one more thing, would it be a good idea to mention if a plugin requires internet in the manage plugin page?

we could do so. But not sure about it, as other plugins require a working internet connection for some specific features as well. And that wouldn't be displayed then

@diosmosis commented on June 25th 2018 Member

I was mostly thinking about the plugins that are unloaded, if there's no signal to the user anywhere that the plugins require enabling internet, maybe some users will think there's a bug. Do you think this is an issue?

@sgiehl commented on June 25th 2018 Member

The is a compatibility message shown when enable_internet_features is set to 0. See https://github.com/matomo-org/matomo/blob/c36f041fddf277478205675ff7efefbcc6d9cb38/tests/UI/expected-screenshots/UIIntegrationTest_admin_plugins_no_internet.png

If the features are enabled, but there is actually no internet connection available the will probably get various connection timeout errors

@diosmosis commented on June 25th 2018 Member

Ah ok, that's what I was thinking about, then 👍 will approve

@sgiehl commented on July 7th 2018 Member

@mattab / @tsteur could someone else give a comment on:

diosmosis commented 12 days ago
Btw @mattab some API/product changes were added here (new API to Plugin class and product change in that plugins requiring an internet connection are unloaded), can you provide your opinion?

@mattab commented on July 10th 2018 Member

The new API & marking internet-requiring plugins in the Plugins overview look good to me :+1:

This Pull Request was closed on July 10th 2018
Powered by GitHub Issue Mirror