Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

MM-22825 Add E2E on skipping Slack-compatible interactive message response #5183

Merged
merged 2 commits into from
Mar 26, 2020
Merged

MM-22825 Add E2E on skipping Slack-compatible interactive message response #5183

merged 2 commits into from
Mar 26, 2020

Conversation

saturninoabril
Copy link
Member

Summary

Test Steps:
Text: a < a | b > a

  1. Create an interactive message buttons via webhook
  2. Click on a message button that skips parsing
  • verify that it renders original "Text"
  1. Click on a message button that do parsing
  • verify that it renders as Markdown link (based on Slack link parsing)

Ticket Link

JIRA ticket - https://mattermost.atlassian.net/browse/MM-22825

Related Pull Requests

@saturninoabril saturninoabril added 2: Dev Review Requires review by a core commiter 3: QA Review Requires review by a QA tester labels Mar 26, 2020
@saturninoabril saturninoabril added this to the v5.22.0 milestone Mar 26, 2020
Copy link
Contributor

@srkgupta srkgupta left a comment

Choose a reason for hiding this comment

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

Hi @saturninoabril
I am consistently seeing this failure. Can you please check. I have ensured that I have the latest code from master branch on webapp & server.

Screenshot 2020-03-26 at 4 11 17 PM

expected <div#postMessageText_p39noqxztbnwbd4hfmb3oe7krw.post-message__text> to have HTML <p>a < a | b > a</p>, but the HTML was <p>a <a class="theme markdown__link" href="https://a" rel="noreferrer" target="_blank"> b </a> a</p>

@saturninoabril
Copy link
Member Author

saturninoabril commented Mar 26, 2020

@srkgupta It requires server change which is not yet merged at the time of your testing. Please update again to master and see how it's working for you, thanks

@srkgupta srkgupta self-requested a review March 26, 2020 15:37
Copy link
Contributor

@srkgupta srkgupta left a comment

Choose a reason for hiding this comment

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

Thanks @saturninoabril. Tested it on the latest master and now the test is working fine. Approving the PR

Copy link
Member

@mickmister mickmister left a comment

Choose a reason for hiding this comment

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

Wow this is great that this can be tested this way! LGTM!

Copy link
Contributor

@josephbaylon josephbaylon left a comment

Choose a reason for hiding this comment

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

LGTM. All tests passed

@josephbaylon josephbaylon added QA Review Done and removed 3: QA Review Requires review by a QA tester 2: Dev Review Requires review by a core commiter labels Mar 26, 2020
@josephbaylon josephbaylon merged commit d653ba9 into mattermost:master Mar 26, 2020
@amyblais amyblais added Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation labels Mar 26, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Changelog/Not Needed Does not require a changelog entry Docs/Not Needed Does not require documentation QA Review Done
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants