Separate calls for Apps #9263

larkox · 2021-10-26T10:05:29Z

Summary

Add separate calls for each function on Apps (submit, form source, form submit and lookup). Also updated the filtering with the new logic.

This PR also piggybacks the renaming of markdow and error to text, and filtering out "white space labels" (to avoid no text labels on post options and channel header).

Notes for testing: This is a breaking change. All Apps will need to be updated to work properly. The test app will be updated to fit the new approach.

Ticket Link

https://mattermost.atlassian.net/browse/MM-39092
https://mattermost.atlassian.net/browse/MM-39096
https://mattermost.atlassian.net/browse/MM-31121
https://mattermost.atlassian.net/browse/MM-33532

Related Pull Requests

Apps Plugin: mattermost/mattermost-plugin-apps#296
Mobile: mattermost/mattermost-mobile#5877
Test App: TBD

Release Note

Breaking change for Apps: calls are now separated between submit, form, refresh and lookup calls.

DHaussermann · 2021-10-27T20:51:34Z

@larkox just to confirm - This will create a code dependancy with mattermost/mattermost-plugin-apps#276

We would need to co-ordinate this release with an Apps plugin release. Or Should Apps 0.7.0 continue to work with this Webapp change as well (which obviously I will test for :) )

larkox · 2021-10-28T08:43:44Z

Old apps (and 0.7.0) won't work with this changes. What it is worth trying is that no crashes nor strange artifacts are seen in any 0.7.0 deployment. We might have to test the same about 0.8.0 in "older servers", or bump the minimum required server version.

mickmister

Awesome work @larkox! LGTM, just a few suggestions not directly related to the purpose of the PR

packages/mattermost-redux/src/types/apps.ts

components/post_view/embedded_bindings/embedded_binding/embedded_binding.tsx

packages/mattermost-redux/src/client/client4.ts

components/suggestion/command_provider/app_command_parser/app_command_parser.ts

actions/apps.ts

mickmister · 2021-11-12T16:09:19Z

actions/apps.ts

+                    window.open(res.navigate_to_url);
+                    return {data: res};
+                }
+                browserHistory.push(res.navigate_to_url.slice(getSiteURL().length));


What if the url is s relative path? I'm also concerned about using a site URL with a subpath cc @DHaussermann

Suggested change

browserHistory.push(res.navigate_to_url.slice(getSiteURL().length));

let navigateURL = res.navigate_to_url;

if (navigateURL.startsWith(getSiteURL())) {

navigateURL = navigateURL.slice(getSiteURL().length);

}

browserHistory.push(navigateURL);

@mickmister @hanzei I've been thinking about this, is there a reason to even allow relative URLs here? Even if we want to allow them as apps return values, we can normalize in the server, and consider them valid here. Also need to check the mobile handling.

Even if we want to allow them as apps return values, we can normalize in the server, and consider them valid here

@levb Makes sense to me

apps API changes fixup

DHaussermann · 2022-03-22T01:25:38Z

Tested and passed

Regression 0.7.0 Apps plugin with webapp changes. No functional regression or new dev console errors found
0.8.0 Apps plugin works as expected - Some know issues are documented outside the scope of this bindings PR
Test-app is functional a couple issues have been documented that have no baring on this bindings webapp change
Start the test-app differently. Pass in var INCLUDE-INVALID invalid bindings Run-with-invalid
Completed authentication test with Zendesk and hello oAuth2 Apps
Quick look over for new latest webapp changes with both v0.7.0 and v0.8.0

DHaussermann · 2022-03-23T12:28:56Z

@amyblais just a heads up that these changes are needed for the March 30 Cloud release and for v6.6

@mickmister can you please make a cherry-pick PR for the cloud branch?

amyblais · 2022-03-23T12:29:26Z

/cherry-pick cloud

mattermod · 2022-03-23T12:29:26Z

Cherry pick is scheduled.

* WIP - need to restore Binding.Call * Address missing pieces * Re-add the submit field on calls and improve tests * Update i18n * apps API changes fixup * apps API changes fixup mattermost#2: form/modal * Address feedback * Address feedback * Fix lint * Fix missing error->text change * fix openAppsModal references * lint, types, and i18n * lint * missing method * update snapshots * Fixed handleBindingClick, was double-wrapping .data * fixed embedded button binding * various fixes * fix lookups for command parser * sync app command parser * types * fix tests * change control flow for cleaning bindings * re-expand bindings when post is updated * getDerivedStateFromProps to avoid lint rule * avoid `this` usage in static function * fix other spot with same issue Co-authored-by: Lev Brouk <lev@mattermost.com> Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com> Co-authored-by: Mattermod <mattermod@users.noreply.github.com> (cherry picked from commit 25f31f5)

…attermost#9263-upstream-cloud

* WIP - need to restore Binding.Call * Address missing pieces * Re-add the submit field on calls and improve tests * Update i18n * apps API changes fixup * apps API changes fixup #2: form/modal * Address feedback * Address feedback * Fix lint * Fix missing error->text change * fix openAppsModal references * lint, types, and i18n * lint * missing method * update snapshots * Fixed handleBindingClick, was double-wrapping .data * fixed embedded button binding * various fixes * fix lookups for command parser * sync app command parser * types * fix tests * change control flow for cleaning bindings * re-expand bindings when post is updated * getDerivedStateFromProps to avoid lint rule * avoid `this` usage in static function * fix other spot with same issue Co-authored-by: Lev Brouk <lev@mattermost.com> Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com> Co-authored-by: Mattermod <mattermod@users.noreply.github.com> (cherry picked from commit 25f31f5) Co-authored-by: Daniel Espino García <larkox@gmail.com> Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

* Port mattermost/mattermost-webapp#9263 to mobile * Fix forms props from commands and missing error->text change * lint * various fixes * fix lookups for command parser * update app command parser * fixes * fixes with embedded forms * lint and types * remove bindings.expanded fix for cleaning bindings on render * lint * re-expand bindings on post update Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>

cwarnermm · 2022-04-04T14:01:08Z

@mickmister - Given that this is a breaking change and that all Apps will need to be updated to work properly, what do we need to communicate in order for others to be successful with this change? Are changes to our app developer documentation needed?

mickmister · 2022-04-05T21:17:14Z

Are changes to our app developer documentation needed?

@cwarnermm Yes we are due for doc updates for the Apps framework. I've created a ticket for us to triage next week. Thank you!

* WIP - need to restore Binding.Call * Address missing pieces * Re-add the submit field on calls and improve tests * Update i18n * apps API changes fixup * apps API changes fixup mattermost#2: form/modal * Address feedback * Address feedback * Fix lint * Fix missing error->text change * fix openAppsModal references * lint, types, and i18n * lint * missing method * update snapshots * Fixed handleBindingClick, was double-wrapping .data * fixed embedded button binding * various fixes * fix lookups for command parser * sync app command parser * types * fix tests * change control flow for cleaning bindings * re-expand bindings when post is updated * getDerivedStateFromProps to avoid lint rule * avoid `this` usage in static function * fix other spot with same issue Co-authored-by: Lev Brouk <lev@mattermost.com> Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com> Co-authored-by: Mattermod <mattermod@users.noreply.github.com>

cwarnermm · 2022-09-19T13:46:24Z

@mickmister - Has the apps framework documentation been updated to address the changes introduced in this PR?

mickmister · 2022-10-13T20:59:00Z

@cwarnermm Yes, this is being factored in to @neflyte's efforts

Lev Brouk and others added 5 commits October 10, 2021 09:21

WIP - need to restore Binding.Call

f2f9368

Merge branch 'master' into lev-MM-39092-separate-calls

2b9554c

Merge branch 'master' into lev-MM-39092-separate-calls

98075c4

Address missing pieces

f05821f

Re-add the submit field on calls and improve tests

b5cd58a

larkox added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Oct 26, 2021

larkox requested review from levb, mickmister and DHaussermann October 26, 2021 10:05

mm-cloud-bot added release-note labels Oct 26, 2021

larkox mentioned this pull request Oct 26, 2021

Separate calls mattermost/mattermost-plugin-apps#276

Closed

4 tasks

Update i18n

df295bf

larkox mentioned this pull request Oct 28, 2021

Update binding filter to accept bindings with a form with a call but no call #8782

Closed

mickmister approved these changes Nov 12, 2021

View reviewed changes

hanzei added the Do Not Merge Should not be merged until this label is removed label Nov 16, 2021

Lev Brouk added 2 commits November 22, 2021 09:19

apps API changes fixup

09e0b25

apps API changes fixup #2: form/modal

46a1615

levb mentioned this pull request Nov 27, 2021

API changes - separate calls etc. Replaces #276. mattermost/mattermost-plugin-apps#296

Merged

5 tasks

larkox added 6 commits November 30, 2021 13:35

Merge branch 'master' into separateCalls

3dadf0f

Address feedback

a26ac2b

Merge branch 'separateCalls' into lev-separateCalls

5e757aa

Address feedback

88ab662

Merge pull request #7 from levb/lev-separateCalls

d9182f2

apps API changes fixup

Fix lint

27406da

larkox added a commit to larkox/mattermost-mobile that referenced this pull request Dec 1, 2021

Port mattermost/mattermost-webapp#9263 to mobile

7f41a6e

levb approved these changes Dec 7, 2021

View reviewed changes

mickmister added 4 commits March 21, 2022 21:53

re-expand bindings when post is updated

4a27b70

getDerivedStateFromProps to avoid lint rule

d43c479

avoid this usage in static function

4c60359

fix other spot with same issue

7f31aa3

mickmister merged commit 25f31f5 into mattermost:master Mar 22, 2022

DHaussermann added 4: Reviews Complete All reviewers have approved the pull request CherryPick/Candidate A candidate for a quality or patch release, but not yet approved and removed 3: QA Review Requires review by a QA tester labels Mar 23, 2022

mattermost-build mentioned this pull request Mar 23, 2022

Automated cherry pick of #9263 #10049

Merged

mattermod added the CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone label Mar 23, 2022

amyblais added Changelog/Done Required changelog entry has been written Docs/Needed Requires documentation and removed CherryPick/Candidate A candidate for a quality or patch release, but not yet approved labels Mar 23, 2022

amyblais added this to the v6.6.0 milestone Mar 23, 2022

mattermod added a commit to mattermost-build/mattermost-webapp that referenced this pull request Mar 23, 2022

Merge branch 'cloud' into automated-cherry-pick-of-mattermost-webapp-m…

1079d04

…attermost#9263-upstream-cloud

cwarnermm mentioned this pull request Apr 6, 2022

v6.6 Changelog and Docs mattermost/docs#5505

Merged

cwarnermm added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Nov 9, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Separate calls for Apps #9263

Separate calls for Apps #9263

larkox commented Oct 26, 2021 •

edited

Loading

DHaussermann commented Oct 27, 2021

larkox commented Oct 28, 2021

mickmister left a comment

mickmister Nov 12, 2021

levb Jan 14, 2022

mickmister Mar 8, 2022

DHaussermann commented Mar 22, 2022

DHaussermann commented Mar 23, 2022 •

edited

Loading

amyblais commented Mar 23, 2022

mattermod commented Mar 23, 2022

cwarnermm commented Apr 4, 2022

mickmister commented Apr 5, 2022

cwarnermm commented Sep 19, 2022

mickmister commented Oct 13, 2022

-                browserHistory.push(res.navigate_to_url.slice(getSiteURL().length));
+                let navigateURL = res.navigate_to_url;
+                if (navigateURL.startsWith(getSiteURL())) {
+                    navigateURL = navigateURL.slice(getSiteURL().length);
+                }
+                browserHistory.push(navigateURL);

Separate calls for Apps #9263

Separate calls for Apps #9263

Conversation

larkox commented Oct 26, 2021 • edited Loading

Summary

Ticket Link

Related Pull Requests

Release Note

DHaussermann commented Oct 27, 2021

larkox commented Oct 28, 2021

mickmister left a comment

Choose a reason for hiding this comment

mickmister Nov 12, 2021

Choose a reason for hiding this comment

levb Jan 14, 2022

Choose a reason for hiding this comment

mickmister Mar 8, 2022

Choose a reason for hiding this comment

DHaussermann commented Mar 22, 2022

DHaussermann commented Mar 23, 2022 • edited Loading

amyblais commented Mar 23, 2022

mattermod commented Mar 23, 2022

cwarnermm commented Apr 4, 2022

mickmister commented Apr 5, 2022

cwarnermm commented Sep 19, 2022

mickmister commented Oct 13, 2022

larkox commented Oct 26, 2021 •

edited

Loading

DHaussermann commented Mar 23, 2022 •

edited

Loading