[MM-50111] Use /install-listed path directly to install marketplace apps

[mickmister]

Summary

As we are transitioning to support HTTP apps in Cloud servers, we need to have a consistent install path across all environments.

At the moment, the frontend has conditional logic of how to install marketplace apps. This PR makes it so the App framework can instead have this logic.

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-50111

NONE

[mattermost-build]

E2E tests not automatically triggered, because PR has no approval yet. Please ask a developer to review and then try again to attach the QA label.

[hanzei]

Thanks for cleanin up my messy code 👍

I leave a few comment, but nothing blocking.

[hanzei]

Is /marketplace the location here? What is the location used in other form calls?

[mickmister]

@hanzei Yes I kind of made this new location up here. The location in other form calls would be something like /command/jira/issue/create. 0/5 on what the location should be here, or if we should include one at all

[hanzei]

Can we just not use a location at all?

[mickmister]

@hanzei I think there are tradeoffs between including or not including a location. I think we have the convention that all calls come from a location, but I think that can be avoided in this case. Do you see value in including the location? We can instead have it be an empty string.

[hanzei]

Can we de-duplicate the code used here? Is there a way to throw the call response in the normal event pipeline?

[mickmister]

@hanzei We purposely have the logic different in each binding location case, so special cases can easily be handled. We could have a "default" handler to be used that does these sort of actions shown above. A "nothing special" call response handler function

[mickmister]

I think this is out of scope of this PR though.

There is an improvement I made in the mobile app for this https://github.com/mattermost/mattermost-mobile/blob/aa6c1ff0583751085967c9fdffd67789f7b24421/app/hooks/apps.ts#L21-L26

export type UseAppBindingConfig = {
    onSuccess: (callResponse: AppCallResponse, message: string) => void;
    onError: (callResponse: AppCallResponse, message: string) => void;
    onForm?: (form: AppForm) => void;
    onNavigate?: (callResp: AppCallResponse) => void;
}

This allows the caller to supply at least onSuccess and onError, and things like opening a form is done automatically. I made a ticket to add this to the webapp https://mattermost.atlassian.net/browse/MM-50197

[hanzei]

/update-branch

[amyblais]

/update-branch

[amyblais]

@levb @DHaussermann Kind note to prioritize this PR review if this is needed for v7.8.

[DHaussermann]

@mickmister can you clarify if this will have a code dependancy on what version of the Apps framework plugin is running? Or would existing version of the Apps framework also be functional with this code change?

I can of course test this - I'm just curious about the expected behavior here.

[mickmister]

@DHaussermann This change is backwards compatible with the framework. No changes are needed on the framework side

[hmhealey]

I don't understand the apps logic enough to review that fully, but the code itself looks good to me from what I can tell

[DHaussermann]

/update-branch

[github-actions]

Test Results

       1 files     815 suites   14m 27s ⏱️
6 933 tests 6 932 ✔️ 1 💤 0 ❌
7 109 runs  7 108 ✔️ 1 💤 0 ❌

Results for commit 5ccdb5f.

[DHaussermann]

Tested and passed

• Tested that marketplace install button works consistently now
• Cloud license and non-cloud license
• Apps 1.2.0 as well as newest 1.2.1 (not yet released)
• In all cases the marketplace UI button works now
• Regression tested installing via command line as a precaution
• Regression tested HTTP installs as a precaution

LGTM!