Plugins: Add QueryJetpackPlugins query component

[tyxla]

This PR adds a QueryJetpackPlugins query component, which can be used in managing network requests for Jetpack plugins.

To test:

I haven't yet integrated it within the plugins components, because this will be done in a series of subsequent PRs that are outside of the scope of this PR. So to test it, insert it in a component and verify that it performs the expected network requests.

[matticbot]

Test live: https://calypso.live/?branch=add/query-plugins-component

[ryelle]

@tyxla letting you know directly — I got feedback that we don't really use hasRequested*-style selectors, so I'll be removing those. You should be able to use isRequesting* for the same purpose.

[tyxla]

@ryelle that's good to know - go ahead and remove them at will 👍

[lamosty]

Just a small naming detail: in #8103, we are trying to add a definite Calypso plugin pattern so people can extend Calypso with plugins (w00t woot). When I saw this PR, I thought: "wow, more people got involved in this Calypso plugin stuff". However, I suppose QueryPlugins is not used for querying Calypso plugins (which are not ready yet :)) but instead, Jetpack Plugins.

Could we rename QueryPlugins to QueryJetpackPlugins (+ all the related components/stuff which has to do something with Jetpack plugins) to avoid naming collisions in the (hopefully near) future?

[tyxla]

@lamosty sorry to disappoint that this is not a Calypso plugin PR 😆 But that's a good point, I've renamed the component to what you suggested. Jetpack/.com plugins currently occupy plugins in the state tree, so it might have to be renamed or reorganized in the future too.

[tyxla]

cc @ryelle @roccotripaldi @johnHackworth @oskosk - let's land it and start integrating it into the plugjn components altogether with the redux state.

[johnHackworth]

Related with what @lamosty, I don't think "jetpack plugins" is a good name ... eventually "jetpack plugins" could become a thing (we have discussed a lot of times if jetpack modules should be independent plugins, so who knows what the future will bring), so it's not a good name. Maybe DotOrgPlugins ... anyway, I think what we should change is not this 'plugins' but the new calypso 'plugins' name. It's going to be confusing enough to reuse 'plugins' for something else than 'classic' WordPress plugins

[tyxla]

That's a good point @johnHackworth - we should be careful about naming here. But anyway, I'm positive that the name of the component should't stop us from getting this query component in. We can always rename the component at a later point if necessary.

[johnHackworth]

since we are injecting fetchPlugins before exporting the component, is it necessary to declare the default?

[tyxla]

Good point - it's actually not necessary. Omitted.

[johnHackworth]

Let's ask a native english speaker about it, but shouldn't it be "isRequestingSites", without the 'for' ?

[ryelle]

isRequestingForSites makes sense to me because we're checking if it's requesting plugins for sites, but I can see it either way. English ¯_(ツ)_/¯

[johnHackworth]

ok, yeah, that makes sense ... "isRequestingForSites" sounds ok if you think about it like that .

[tyxla]

I agree with @ryelle on this one - we're requesting the plugins for the given sites, so naming makes sense.

[johnHackworth]

I left two minor comments about naming & defaults, but this is looking good!

[ryelle]

I just updated this PR with the updated action name & selector parameters - which have changed since this was first written.

[tyxla]

@ryelle: Actually, since we strive to use mostly site IDs instead of site objects everywhere, doesn't it make sense to expect sites to contain just site IDs and not the entire objects? @aduth - you might have an opinion on this?

[ryelle]

Yes… it should, but I think the action still expects the site object. It doesn't need to, fetchPlugins is the only action that does. So we could update the action, and then update this component to only use site IDs.

[tyxla]

That sounds like a good way to proceed @ryelle. I've updated the PR:

1. fetchPlugins() will now accept site IDs. This has been also applied to the tests and the README.
2. QueryJetpackPlugins will now accept site IDs - applied to the README as well.
3. Renamed the describe() calls of the action tests to correspond to the action names.
4. Stubbed console.error to hide the Promise rejection errors from the test results output.

Feel free to review again. 😃

[ryelle]

Looks great. Thanks for the test cleanup too :)