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

[GH-15740] channel mentions in message attachments #6628

Merged
merged 5 commits into from
Oct 27, 2020
Merged

[GH-15740] channel mentions in message attachments #6628

merged 5 commits into from
Oct 27, 2020

Conversation

jufab
Copy link
Contributor

@jufab jufab commented Oct 4, 2020 •
edited by prapti
Loading

Summary

This PR fix the webapp problem with message attachements for channel and internal link.

Ticket Link

Jira ticket: https://mattermost.atlassian.net/browse/MM-28980
mattermost/mattermost#15740 => For the webapp part

Related Pull Requests

None

Screenshots

No UI change

@mattermod
Copy link
Contributor

Hello @jufab,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

@jasonblais jasonblais added 1: PM Review Requires review by a product manager Setup Cloud Test Server Setup a test server using Mattermost Cloud labels Oct 5, 2020
@jufab
Copy link
Contributor Author

jufab commented Oct 5, 2020 •
edited
Loading

I create a message with attachments in the town-square channel => This redirect to Off-Topic without reload

aaronrothschild
aaronrothschild approved these changes Oct 5, 2020
View reviewed changes
Copy link
Contributor

@aaronrothschild aaronrothschild left a comment

Choose a reason for hiding this comment

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

Looks good to me @jufab thanks for your contribution!

@levb levb requested review from nevyangelova and larkox and removed request for levb October 6, 2020 11:04
@levb levb added the 2: Dev Review Requires review by a core commiter label Oct 6, 2020
larkox
larkox suggested changes Oct 6, 2020
View reviewed changes
Copy link
Contributor

@larkox larkox left a comment

Choose a reason for hiding this comment

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

Awesome! Thank you for fixing this.
Just one small change. Could you revert the changes in package-lock.json. You are not actually changing anything there, and the changes are probably just due a difference between your npm version (or other build related tools) and the one we use.

nevyangelova
nevyangelova approved these changes Oct 6, 2020
View reviewed changes
Copy link
Contributor

@nevyangelova nevyangelova left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM, can you just revert the changes in package-lock.json?

@jufab
Copy link
Contributor Author

jufab commented Oct 6, 2020

No problem, it's done!

@larkox larkox self-requested a review October 6, 2020 11:45
larkox
larkox approved these changes Oct 6, 2020
View reviewed changes
Copy link
Contributor

@larkox larkox left a comment

Choose a reason for hiding this comment

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

LGTM!

@larkox larkox added 3: QA Review Requires review by a QA tester and removed 1: PM Review Requires review by a product manager 2: Dev Review Requires review by a core commiter labels Oct 6, 2020
@hanzei hanzei added the CherryPick/Approved Meant for the quality or patch release tracked in the milestone label Oct 7, 2020
@hanzei hanzei added this to the v5.29.0 milestone Oct 7, 2020
@hanzei
Copy link
Contributor

hanzei commented Oct 13, 2020

/update-branch

@jasonblais jasonblais removed the request for review from DHaussermann October 21, 2020 11:49
@amyblais
Copy link
Member

@prapti Kind reminder to review for v5.29.

@prapti
Copy link
Contributor

prapti commented Oct 23, 2020

/update-branch

@prapti
Copy link
Contributor

prapti commented Oct 23, 2020

Testing this, but waiting on some answers from Alejandro about test steps. Will continue on Monday.

agarciamontoro
agarciamontoro approved these changes Oct 26, 2020
View reviewed changes
Copy link
Member

@agarciamontoro agarciamontoro left a comment

Choose a reason for hiding this comment

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

Great work, @jufab!

I just tested this in the webapp and confirmed that:

  • If the link is contained in the Pretext field, there is no longer a page refresh on webapp when navigating to the link.
  • If the link is contained in the Text field, there is no longer a Not Found error when navigating to the link.

We'll duplicate the original ticket to keep track of the mobile bug as well.

Thank you again!

prapti
prapti approved these changes Oct 26, 2020
View reviewed changes
Copy link
Contributor

@prapti prapti left a comment

Choose a reason for hiding this comment

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

Thank you @jufab for working on this one! 🎉
A big thanks to @agarciamontoro for doing the peer QA review on this PR.. approving.

@prapti prapti added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Oct 26, 2020
@hanzei hanzei removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Oct 27, 2020
@mm-cloud-bot
Copy link

Test server destroyed

@hanzei hanzei merged commit c970b04 into mattermost:master Oct 27, 2020
@mattermod
Copy link
Contributor

Cherry pick is scheduled.

mattermost-build pushed a commit to mattermost-build/mattermost-webapp that referenced this pull request Oct 27, 2020
@jufab @mattermost-build
[GH-15740] channel mentions in message attachments (mattermost#6628)
1b812bc 
(cherry picked from commit c970b04)
@mattermost-build mattermost-build mentioned this pull request Oct 27, 2020
Merged
@mattermod mattermod added CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone and removed CherryPick/Approved Meant for the quality or patch release tracked in the milestone labels Oct 27, 2020
@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation labels Oct 27, 2020
mattermod pushed a commit that referenced this pull request Oct 27, 2020
@mattermost-build
[GH-15740] channel mentions in message attachments (#6628) (#6937)
447ee3d 
Automatic Merge
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@prapti prapti prapti approved these changes

@aaronrothschild aaronrothschild aaronrothschild approved these changes

@larkox larkox larkox approved these changes

@agarciamontoro agarciamontoro agarciamontoro approved these changes

@nevyangelova nevyangelova nevyangelova approved these changes

Assignees
No one assigned
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written CherryPick/Done Successfully cherry-picked to the quality or patch release tracked in the milestone Docs/Not Needed Does not require documentation Hacktoberfest hacktoberfest-accepted
Projects
None yet
Milestone
v5.29.0