Mattermost-server: New plugin: migrate Gfycat built-in integration to a plugin

[esethna]

If you're interested please comment here and come join our "Contributors" community channel on our daily build server, where you can discuss questions with community members and the Mattermost core team. For technical advice or questions, please join our "Developers" community channel.

New contributors please see our Developer's Guide.

---

Notes: Jira ticket

Migrate Gfycat built-in integration to a plugin.

The feature was merged to Redux, server and webapp repos. Mobile has not yet been implemented.

Additional things to consider, which the Gfycat team is helping with as well:

1. Better handling when blank values set for API key and secret in config.json
2. Better handling when incorrect values set for API key and secret in config.json
3. Style issues that might be affecting the entire page vs just the component
4. Changes that would help improve memory/performance
5. Removing files and components that are not being used anymore
6. Remaining style changes requested by Harrison

[fcorrea]

I'll take this one.

[enahum]

Awesome @fcorrea thanks!

[fcorrea]

Hi there! I have a couple of questions regarding this one.

1) Do we want to keep the same/current integration that's already in place in the emoji_picker? Checking the picker tabs, gfycat seems to be hardcoded and gated with a check if it should be enabled or not. I'm probably wrong but I'm thinking that in order to allow the same integration, the registry would have to grow a new method to allow for this? Meaning that the tabs code would have to pull from the plugin infrastructure.

2) For the sake of development and testing, I was thinking on adding something similar like PostReaction, (with an icon followed by the gfycat view on click) but from a plugin perspective, the closest thing I got was registerPostDropdownMenuAction or registerPostDropdownMenuComponent. What would be the easiest way to approach this before going deep on integration?

Thanks!

[fcorrea]

A bit more on this as I go.
While fixing the import paths, I've noticed the component is importing webapp code and I'm not sure how to address this.
For certain components like svg ones, I guess it's fine to duplicate OR, maybe, it should be pushed to a library that could be pulled by plugins, say, mattermost-svg.
There are other things like localized_input which gfycat is also pulling from webapp, but this one if pretty self-contained so just duplicated but...could be exported in a library as well.
But the problem really happens when we pull post_utils as it has a hard dependency on webapp.

I'll hold on this for now as I really need to understand it better.

[crspeller]

Sorry for the delay!

1. Yes you will need a new integration point to accomplish this. It should be able to add a tab to the emoji picker just like the gfycat one.

2. Not sure that would be helpful or not. The final integration would have to have a new integration point so you will need to develop that anyway. But for testing anything would work.

Ya. That's really the crux of the issue in porting stuff out of the webapp. Lots of dependencies. We don't want plugins to depend on the webapp too much so I would lean towards isolating it rather than exposing more stuff. I wouldn't be afraid to just start copying functions that the gfycat code depends on into the plugin to start. Then once you get something working, clean it up so there is only what the plugin really needs.

[fcorrea]

Thanks for checking in @crspeller!

I had a chat with @enahum and he pointed the same direction as you so that's good. I already have a webapp branch in which the picker tabs are plugins and on the Gyfcat plugin side, I'm trying to lessen the hard dependencies. post_utils.js pull in webapp/store/ which really opens up a pandora box in terms of dependencies.

Just to make it a bit more clear webapp/store, will pull in the following dependencies (and their subdependencies):

import {storageRehydrate} from 'actions/storage';
import {clearUserCookie} from 'actions/views/root';
import appReducer from 'reducers';
import {transformSet} from 'store/utils';
import {detect} from 'utils/network.js';
import {ActionTypes} from 'utils/constants.jsx';
import {getBasePath} from 'selectors/general';

If I manage to use mattermost-redux/store and replace it the plugin version of post_utils, then dependencies will be cut significantly. All that post_utils really needs from store is getState. So one way or another we should be able to achieve that w/o all those dependencies.

I'll keep reporting progress as this became something a bit more broad.

[fcorrea]

Turns out that the only thing the Gyfcat uses from post_utils is getImageSrc. Easy enough to just backport that single function.

[fcorrea]

Ok, I've managed to isolate all the dependencies and now it builds just fine. I'm keeping the plugin code in this repo https://github.com/fcorrea/mattermost-plugin-gfycat

It's not finished yet so I'm moving on with the fixes for the integration.

[crspeller]

@fcorrea Great! Sounds like you are making good progress.

[fcorrea]

Hey all. Unfortunately I wont be able to continue to work on this now.

In case someone needs to revisit this, here's the webapp PR branch that is able to load and register the plugin: https://github.com/fcorrea/mattermost-webapp/tree/MM-9914

And here's the plugin itself: https://github.com/fcorrea/mattermost-plugin-gfycat

[jasonblais]

Thanks @fcorrea for the heads up and for sharing your existing work :tada:

[seansackowitz]

If this is still open, I'll pick it up

[jasonblais]

@seansackowitz It's open, thank you!

[hanzei]

Hey @seansackowitz,

How is your work going? Do you have any questions?

[hanzei]

Making this one available for the public again due to inactivity.

[kop]

I'll take this one.

[bbodenmiller]

Why not ask this plugin author to take it over? https://github.com/moussetc/mattermost-plugin-giphy cc @moussetc

[jasonblais]

cc @aaronrothschild @hanzei

[hanzei]

Hey @bbodenmiller,

Thanks for the suggestion. The main question is, if the gfycat integration should be shipped and enabled by default. If this feature get's moved to a community plugin, Mattermost won't ship it by default any longer.

@aaronrothschild What do you think about this?

[moussetc]

Hi,
Thanks for the mention. In addition to the points of the previous comment, as maintainer of the community plugin, I don't think I would be able to integrate the gfycat webapp integration in the foreseeable future by myself. Should it make sense to add it to this community plugin, and should someone want to become contributor to the plugin to add the integration, I'd welcome it.

[aaronrothschild]

Hey @bbodenmiller,

Thanks for the suggestion. The main question is, if the gfycat integration should be shipped and enabled by default. If this feature get's moved to a community plugin, Mattermost won't ship it by default any longer.

@aaronrothschild What do you think about this?

Hmmm, I worry about not shipping it by default. Is this referring tothe emoji picker " gfycat tab" that shows up in Webapp?:

This is related to the bigger conversation of "How do we ship features as Plugins more effectively" IMO.

[jasonblais]

@aaronrothschild yeah, that's the feature

If this feature get's moved to a community plugin, Mattermost won't ship it by default any longer.

Is that correct? I thought the plan was to ship community plugins in future, through the marketplace.

[aaronrothschild]

Is that correct? I thought the plan was to ship community plugins in future, through the marketplace.

@jasonblais Yes, BUT we always said that community plugins would not be BUNDLED with Mattermost server as "pre-downloaded". I think that's the stopper here. Currently, no one needs to install anything to get the Gfycat feature, but I'm not sure how often it is currently used. Maybe when rudderstack lets us capture all user events again we can quantify it?

[jasonblais]

@aaronrothschild we might have telemetry on it since there's a config setting admins have to toggle to enable the integration. And that's true regarding bundling of plugins.

[bbodenmiller]

At this point I personally think removing built-in Gfycat feature would be a bad idea. What is the driver for this? Some instances likely cannot install community plugins.

[esethna]

Gfycat picker is default off right now in the config as an FYI. I hope to turn it on by default though if we can solve some of the UX issues (ie collapsing and using in-line images instead of link previews)

@aaronrothschild perhaps we can schedule something to discuss the future of some of these specific cases of "plugin vs core feature" we're hitting? (Remindbot, Gyfcat, Copy Text, etc)

[hanzei]

@esethna Would you mind also sending me an invitation and I see if I can make it?