Skip to content

Added check for the 'show' prop before making API request for the link tooltip.#453

Merged
mickmister merged 4 commits intomattermost:masterfrom
Brightscout:MM-197
Feb 9, 2024
Merged

Added check for the 'show' prop before making API request for the link tooltip.#453
mickmister merged 4 commits intomattermost:masterfrom
Brightscout:MM-197

Conversation

@raghavaggarwal2308
Copy link
Copy Markdown

Summary

  • Added check for the 'show' prop before making API request for the link tooltip.
  • Added check if the data is already present for the link, do not make new API requests.

What to test?

  • API request to fetch the data is sent when we hover over a Gitlab issue/PR link.
  • API request is not sent if we have already made a request in the past and have not refreshed the page after that.
Steps to test:
  1. Connect your Gitlab account.
  2. Open the network tab of your browser.
  3. Hover over a Gitlab issue/PR link.

Ticket Link

Fixes: https://community.mattermost.com/core/pl/zao9gnwgjpf6mmeo9y9xu8kngy

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (7834d0f) 33.50% compared to head (1485e82) 33.38%.
Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #453      +/-   ##
==========================================
- Coverage   33.50%   33.38%   -0.12%     
==========================================
  Files          22       22              
  Lines        4000     4014      +14     
==========================================
  Hits         1340     1340              
- Misses       2527     2541      +14     
  Partials      133      133              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@hanzei hanzei added the 2: Dev Review Requires review by a core committer label Feb 7, 2024
@mickmister
Copy link
Copy Markdown
Contributor

I'm thinking we should also have the return value of the component be null when props.show is false, just as we are doing here:

@raghavaggarwal2308
Copy link
Copy Markdown
Author

@mickmister Updated

@mickmister mickmister merged commit 175c61d into mattermost:master Feb 9, 2024
@raghavaggarwal2308 raghavaggarwal2308 deleted the MM-197 branch February 9, 2024 07:48
@ayusht2810 ayusht2810 mentioned this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2: Dev Review Requires review by a core committer

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants