Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MM-33989] Show bindings in all threads and post menus #5232

Merged
merged 24 commits into from
Mar 24, 2021

Conversation

mickmister
Copy link
Contributor

@mickmister mickmister commented Mar 19, 2021

Summary

No Apps currently have any channel-specific bindings. This PR makes it so we are no longer hiding bindings in threads and post menus. This includes commands and post menu items.

Ticket Link

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

Awaiting merge of feature/cloud-apps -> master #5226

larkox and others added 18 commits February 10, 2021 15:23
* Add redux related information

* Add binding loading on channel refresh

* Add channel header and post option bindings

* Fix test

* Minor fixes

* Fix snapshots

* Handle errors and show bindings only on main channel

* Update Expand Levels keys and values to match latest changes

* Add NAVIGATE call response handling.

* Add more isolation to apps related code

* Add defaults to send ephemeral

* Update variable naming

* Rename shouldProcessApps by a more meaningful appsEnabled
* Add redux related information

* Add binding loading on channel refresh

* Add channel header and post option bindings

* Fix test

* Minor fixes

* Fix snapshots

* Handle errors and show bindings only on main channel

* Update Expand Levels keys and values to match latest changes

* Add NAVIGATE call response handling.

* Add more isolation to apps related code

* Add Embedded Forms

* Fix snapshots

* Add Embedded Forms

* Improve naming, change the root element to be a binding and improve binding handling

* Get post down on the buttons, remove unneeded variables and minor fixes from the review

* Allow undefined bindings to fillBindingsInformation and add logging for error.

* Address review feedback
* Add App Forms

* Address feedback and self review

* Add dynamic select

* Fixes and improvements

* Add the ability to refresh on submit.

* Use AppFormValue instead of redoing the type

* Address feedback
* Add refresh websocket event to refetch bindings

* Add missing changes

* Declare socket event constant and separate the handler to a different function
* Add redux related information

* Add binding loading on channel refresh

* Add channel header and post option bindings

* Fix test

* Minor fixes

* Fix snapshots

* apps modals draft

* Handle errors and show bindings only on main channel

* Update Expand Levels keys and values to match latest changes

* Add NAVIGATE call response handling.

* Add more isolation to apps related code

* reuse command parser throughout slash_suggestion component's lifecycle

* fix prop and lint

* using alert to show error message. need another way

* duplicate import

* types

* types

* types

* rename file

* copy webapp parser and test

* dependencies moved. tests pass

* move app command parser into its own folder

* converted to typescript, all tests are passing

* automated and manual tests work

* lint

* lint

* remove fall throughs with blocks

* types

* doAppCall type

* extract displayError to deps file

* test types

* lint

* fix tests

* unused import

* PR feedback

* fix imports and show errors

* types

* remove execute suggestion for mobile

* watch feature flag

* fix tests

* change form text arugment behavior to show user input and not hint

* return call response error in doAppCall

* update tests to remove hint from text field suggestions

* lint

* use new base command structure

* fix tests

* wrap appsEnabled

* typescript actions/command.ts

* update app constants

* PR feedback

* fix error handling from doAppCall action

* Use App CallRequest structure (#5212)

* error handling

* move test files

* remove unused import

Co-authored-by: Daniel Espino García <larkox@gmail.com>
* Add feature flag

* Simplify return
* Add localization, call validation and use call type on subpath

* Add more localization to the parser and bring fixes from webapp

* Fix ephemerals and channel header / post options crashes

* fix app command parser deps and alert messages

* Improve suggestion handling

* Fix test

* Fix lint

* Return errors as error

* Address PR feedback

* return error property

* fix error string wordings

Co-authored-by: Michael Kochell <6913320+mickmister@users.noreply.github.com>
@mickmister mickmister added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Mar 19, 2021
@mickmister mickmister added the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Mar 19, 2021
@DHaussermann DHaussermann added the Build Apps for PR Build the mobile app for iOS and Android to test label Mar 21, 2021
@mattermod
Copy link
Contributor

Building app in separate branch.

@mattermod mattermod removed the Build Apps for PR Build the mobile app for iOS and Android to test label Mar 21, 2021
@DHaussermann DHaussermann added the Build Apps for PR Build the mobile app for iOS and Android to test label Mar 22, 2021
@mattermod
Copy link
Contributor

Building app in separate branch.

@mattermod mattermod removed the Build Apps for PR Build the mobile app for iOS and Android to test label Mar 22, 2021
Base automatically changed from feature/cloud-apps to master March 22, 2021 22:02
@mickmister
Copy link
Contributor Author

I addressed Dylan's comment about ephemeral messages in the last commit

@mickmister mickmister removed the Do Not Merge/Awaiting PR Awaiting another pull request before merging (e.g. server changes) label Mar 23, 2021
@DHaussermann DHaussermann added the Build Apps for PR Build the mobile app for iOS and Android to test label Mar 23, 2021
@mattermod
Copy link
Contributor

Building app in separate branch.

@mattermod mattermod removed the Build Apps for PR Build the mobile app for iOS and Android to test label Mar 23, 2021
@mattermod
Copy link
Contributor

@mickmister mickmister removed the 2: Dev Review Requires review by a core commiter label Mar 23, 2021
@DHaussermann
Copy link

DHaussermann commented Mar 23, 2021

@mickmister this is strange as I still see the same behavior where the post is made as a new root post in the channel and not the reply thread.
I used builds from this post here #5232 (comment)

It seems odd to me that I see no change in behavior. Can you repro this with he same build?

@DHaussermann DHaussermann added the Build Apps for PR Build the mobile app for iOS and Android to test label Mar 23, 2021
@mattermod
Copy link
Contributor

Building app in separate branch.

@mattermod mattermod removed the Build Apps for PR Build the mobile app for iOS and Android to test label Mar 23, 2021
@mattermod
Copy link
Contributor

@DHaussermann
Copy link

I have tested this and the issue described is resolved.

However, I also notice that the binding in the post menu are now visible in things such as saved posts, search results and pinned posts.

@aaronrothschild I'm not sure if this should be the case here. Previously these posts would not show the bindings I thought this was the expected behavior. Any thoughts on this?

Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After discussing with Aaron - We are moving forward with the current implementation that the MM Apps binding will be shown on all posts including Saved, Search results, Pinned posts and permalink view.

@DHaussermann DHaussermann self-requested a review March 24, 2021 18:35
Copy link

@DHaussermann DHaussermann left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested and passed

  • Reply threads show in bindings on posts and input boxes
  • When a bots posts a response to an action in the reply hread, this post is now displayed in the reply thread and does not create a new root post
  • MM Apps options are shown when you long press a post from Pinned, Search, saved and from perma-link view
  • Tested on Android and iOS

LGTM!

@DHaussermann DHaussermann added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Mar 24, 2021
@mickmister mickmister merged commit 6fd9b6d into master Mar 24, 2021
@mickmister mickmister deleted the show-post-options-rhs branch March 24, 2021 21:06
@amyblais amyblais added this to the v1.42.0 milestone Mar 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4: Reviews Complete All reviewers have approved the pull request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants