Metris: improve marketplace activity metrics to prepare next version #10495
Conversation
Codecov Report
@@ Coverage Diff @@
## master #10495 +/- ##
=======================================
Coverage 58.01% 58.01%
=======================================
Files 185 185
Lines 6431 6431
Branches 1400 1400
=======================================
Hits 3731 3731
Misses 2236 2236
Partials 464 464
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
| const { error, isLoading, data } = useFetchPluginsFromMarketPlace(); | ||
|
|
||
| if (isLoading || error) { | ||
| return <LoadingIndicatorPage />; | ||
| } | ||
|
|
||
| const handleDownloadPlugin = async pluginId => { | ||
| emitEvent('willInstallPlugin', { pluginId }); |
There was a problem hiding this comment.
For other events with use the property plugin for the name of the plugin we should probably make this consistent
There was a problem hiding this comment.
ok, did you mean { plugin: pluginId }. I made that update but if I misunderstood just let me know 🙃
markkaylor
force-pushed
the
chore/marketplace-tracking
branch
from
88462ff
to
716d3c7
Compare
| @@ -53,6 +56,12 @@ const LeftMenuLinkContent = ({ | |||
| <LinkLabel>{labelId}</LinkLabel> | |||
| ); | |||
|
|
|||
| const handleClick = () => { | |||
There was a problem hiding this comment.
Can you set it in a useEffect hook directly in the <MarketPlacePage /> ?
There was a problem hiding this comment.
ok yes I considered it but wasn't sure which approach was better. Thanks.
| @@ -38,6 +41,7 @@ const MarketPlacePage = () => { | |||
| const response = await request('/admin/plugins/install', opts, overlayblockerParams); | |||
|
|
|||
| if (response.ok) { | |||
| emitEvent('didInstallPlugin', { plugin: pluginId }); | |||
There was a problem hiding this comment.
Installing a plugin takes quite some time I am not sure that this event will ever pop up but let's keep it.
| const memoizedEmit = useCallback(() => { | ||
| emitEvent('didGoToMarketplace'); | ||
| }, [emitEvent]); | ||
|
|
||
| useEffect(() => { | ||
| memoizedEmit(); | ||
| }, [memoizedEmit]); | ||
|
|
There was a problem hiding this comment.
@soupette Is this the correct way to do this? Originally I thought if I just added an empty dependency array to the useEffect it would only fire on first render, but eslint wanted the emitEvent function in the dependency array.
There was a problem hiding this comment.
the best way to remove the emit from the dependency array is to use a ref:
const emitEventRef = useRef(emitEvent);
useEffect(() => {
emitEventRef.current('didGotToMarketplace');
}, [])
markkaylor
force-pushed
the
chore/marketplace-tracking
branch
from
e94e7ee
to
3fc576c
Compare
markkaylor
force-pushed
the
chore/marketplace-tracking
branch
from
3fc576c
to
bf233da
Compare
alexandrebodin
added
source: core:admin
alexandrebodin
changed the title
What does it do?
adds send events when user visits marketplace or downloads a plugin