Fix copy link issue in Safari #1633

leanneeliatra · 2023-10-24T09:59:46Z

Description

Bug fix for broken copy link in Safari

Issues Resolved

Closes [BUG] Share > Copy link broken in Safari OpenSearch-Dashboards#1716

Testing

Companion PR Integration tests for broken safari link opensearch-dashboards-functional-test#940

Check List

New functionality includes testing
New functionality has been documented
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

peternied · 2023-10-31T13:14:41Z

@leanneeliatra Please fix the ci failures and see about adding some kind of tests to validate this change.

leanneeliatra · 2023-10-31T15:09:24Z

@leanneeliatra Please fix the ci failures and see about adding some kind of tests to validate this change.

Unit tests in progress to conclude this ticket (integration test and bugfix complete).

codecov · 2023-11-02T10:20:06Z

Codecov Report

Attention: 11 lines in your changes are missing coverage. Please review.

Comparison is base (cfc83dd) 66.63% compared to head (d2aa727) 67.06%.

Files	Patch %	Lines
public/services/shared-link.ts	63.33%	8 Missing and 3 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1633      +/-   ##
==========================================
+ Coverage   66.63%   67.06%   +0.43%     
==========================================
  Files          94       94              
  Lines        2395     2402       +7     
  Branches      319      318       -1     
==========================================
+ Hits         1596     1611      +15     
+ Misses        728      713      -15     
- Partials       71       78       +7

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

peternied · 2023-11-02T21:39:51Z

@leanneeliatra It looks like there are unintended changes included in this PR, maybe you'd be better of rebasing from main and force pushing again?

peternied · 2023-11-08T14:43:57Z

@leanneeliatra Looks like the rebase issues are behind now, happy to review once the test case is built out - thanks!

leanneeliatra · 2023-11-14T15:51:25Z

@leanneeliatra Looks like the rebase issues are behind now, happy to review once the test case is built out - thanks!

Thank you Peter the cypress test is now complete for functionality, there is an linting error I am fixing. The jest test in progress. I will update once complete for review.

leanneeliatra · 2023-11-15T10:41:48Z

Update: Linting errors for corresponding integration tests are now solved , I have some comments to address there and unit tests in progress.

stephen-crawford

Looks good to me

Signed-off-by: [email protected] <[email protected]>

Signed-off-by: leanneeliatra <[email protected]> Signed-off-by: [email protected] <[email protected]>

…cks (opensearch-project#1613) Signed-off-by: Craig Perkins <[email protected]> Signed-off-by: Craig Perkins <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]> Signed-off-by: [email protected] <[email protected]>

…oject#1564) * changing return to dashboards with functioning logout button on error page Signed-off-by: [email protected] <[email protected]> Co-authored-by: Darshit Chanpura <[email protected]> Co-authored-by: Peter Nied <[email protected]> Signed-off-by: [email protected] <[email protected]>

…project#1626) Signed-off-by: Vamsi Manohar <[email protected]> Signed-off-by: [email protected] <[email protected]>

Signed-off-by: [email protected] <[email protected]>

…t#1636) Signed-off-by: Derek Ho <[email protected]> Signed-off-by: [email protected] <[email protected]>

opensearch-project#1619) * finished ticket Signed-off-by: Prabhas Kurapati <[email protected]> * fixed invalid auth type error Signed-off-by: Prabhas Kurapati <[email protected]> * fixed unit tests Signed-off-by: Prabhas Kurapati <[email protected]> * made requested changes Signed-off-by: Prabhas Kurapati <[email protected]> * updated saml to AuthType.SAML + fixed basicauth test Signed-off-by: Prabhas Kurapati <[email protected]> --------- Signed-off-by: Prabhas Kurapati <[email protected]> Signed-off-by: [email protected] <[email protected]>

…nsearch-project#1641) Stabilize SAML integration test cases for security dashboard CIs (opensearch-project#1641) --------- Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: [email protected] <[email protected]>

Signed-off-by: [email protected] <[email protected]>

…nsearch-project#1641) Stabilize SAML integration test cases for security dashboard CIs (opensearch-project#1641) --------- Signed-off-by: Ryan Liang <[email protected]> Signed-off-by: [email protected] <[email protected]>

Signed-off-by: [email protected] <[email protected]>

This reverts commit 01063d1289ab2aae7c13c114d7c608dac8002e98. Signed-off-by: [email protected] <[email protected]>

leanneeliatra · 2023-11-23T10:25:44Z

@peternied could you review this when you have a moment please, all checks are passing here now and the integration tests are also in review. Thanks.

leanneeliatra · 2023-11-24T13:17:00Z

cc @peternied @cwperks @DarshitChanpura @peternied @RyanL1997
Thank you all, just to flag this PR just needs 1 more review if anyone is available.

leanneeliatra · 2023-11-27T09:31:08Z

The integration tests are now reviewed and are waiting on the final review and merge of this PR, to be merged themselves.

cc @peternied @cwperks @DarshitChanpura @davidlago @RyanL1997 this PR just needs 1 more review if anyone is available.

derek-ho

No major concerns with this PR, but not able to replicate the original issue, so not sure if this is needed

leanneeliatra · 2023-11-28T15:34:31Z

My Safari Version is 17.1 (19616.2.9.11.7). I checked again and I can indeed reproduce this and my fix does fix it locally. From speaking with @davidlago, in the case that it can't be reproduced by some but I myself can reproduce it as could the original reporter, we can go ahead and merge this if possible please?

Then the integration tests can also be merged. opensearch-project/opensearch-dashboards-functional-test#940 Thanks all.

public/services/shared-link.ts

cwperks

LGMT! Thank you @leanneeliatra 🚢

Signed-off-by: [email protected] <[email protected]> Signed-off-by: leanneeliatra <[email protected]> Signed-off-by: leanneeliatra <[email protected]> (cherry picked from commit 8f1334e)

Signed-off-by: [email protected] <[email protected]> Signed-off-by: leanneeliatra <[email protected]> Signed-off-by: leanneeliatra <[email protected]> (cherry picked from commit 8f1334e) Co-authored-by: leanneeliatra <[email protected]>

leanneeliatra requested review from cliu123, cwperks, DarshitChanpura, davidlago, peternied, RyanL1997 and stephen-crawford as code owners October 24, 2023 09:59

leanneeliatra force-pushed the safari-copy-link-fix branch from 5ff4d9c to 255d7f5 Compare October 24, 2023 11:41

leanneeliatra mentioned this pull request Oct 24, 2023

[BUG] Share > Copy link broken in Safari opensearch-project/OpenSearch-Dashboards#1716

Closed

peternied changed the title ~~Safari copy link fix~~ Fix copy link issue in Safari Oct 27, 2023

leanneeliatra force-pushed the safari-copy-link-fix branch from bea354e to 1c88499 Compare November 2, 2023 10:11

leanneeliatra force-pushed the safari-copy-link-fix branch from 80ac3d3 to dd3f902 Compare November 16, 2023 10:42

stephen-crawford previously approved these changes Nov 16, 2023

View reviewed changes

leanneeliatra dismissed stephen-crawford’s stale review via 29734dc November 17, 2023 13:05

DarshitChanpura and others added 9 commits November 17, 2023 13:05

Adds 2.11.0.0 release notes (opensearch-project#1600)

aa1c1d7

Signed-off-by: [email protected] <[email protected]>

added fix for safari link not copying

8492ad2

Signed-off-by: leanneeliatra <[email protected]> Signed-off-by: [email protected] <[email protected]>

removing unneeded code and comments to create final PR

790e27a

Signed-off-by: leanneeliatra <[email protected]> Signed-off-by: [email protected] <[email protected]>

Add permissions for async query and patch datasource API (opensearch-…

fac056a

…project#1626) Signed-off-by: Vamsi Manohar <[email protected]> Signed-off-by: [email protected] <[email protected]>

wip unit tests for safari link in progress

fa79471

Signed-off-by: [email protected] <[email protected]>

Fix bug where custom permission groups are missing (opensearch-projec…

f59a29d

…t#1636) Signed-off-by: Derek Ho <[email protected]> Signed-off-by: [email protected] <[email protected]>

RyanL1997 and others added 12 commits November 17, 2023 13:05

merge from main

a56a73d

Signed-off-by: [email protected] <[email protected]>

funtions extracted for unit testing

eb146db

Signed-off-by: [email protected] <[email protected]>

funtions extracted for unit testing

ad38c05

Signed-off-by: [email protected] <[email protected]>

specific clipboard range code extracted for unit testing

07c5222

Signed-off-by: [email protected] <[email protected]>

clipboard range code tested

75c6242

Signed-off-by: [email protected] <[email protected]>

clipboard range code tested

8057f24

Signed-off-by: [email protected] <[email protected]>

covering uncovered test lines

842afb8

Signed-off-by: [email protected] <[email protected]>

run ci

0a5fad4

Signed-off-by: [email protected] <[email protected]>

lint fixed

952c00c

Signed-off-by: [email protected] <[email protected]>

Revert "covering uncovered test lines"

4a89daf

This reverts commit 01063d1289ab2aae7c13c114d7c608dac8002e98. Signed-off-by: [email protected] <[email protected]>

leanneeliatra force-pushed the safari-copy-link-fix branch from 29734dc to 4a89daf Compare November 17, 2023 13:06

Merge branch 'main' into safari-copy-link-fix

9672422

Hailong-am mentioned this pull request Nov 27, 2023

Integration tests for broken safari link opensearch-project/opensearch-dashboards-functional-test#940

Merged

1 task

derek-ho reviewed Nov 27, 2023

View reviewed changes

peternied approved these changes Nov 28, 2023

View reviewed changes

Merge branch 'main' into safari-copy-link-fix

d2aa727

cwperks reviewed Nov 28, 2023

View reviewed changes

public/services/shared-link.ts Show resolved Hide resolved

cwperks approved these changes Nov 28, 2023

View reviewed changes

cwperks added the backport 2.x backport to 2.x branch label Nov 28, 2023

peternied merged commit 8f1334e into opensearch-project:main Nov 28, 2023
9 of 10 checks passed

opensearch-trigger-bot bot mentioned this pull request Nov 28, 2023

[Backport 2.x] Fix copy link issue in Safari #1672

Merged

vilorij mentioned this pull request Jan 9, 2024

[BUG] Copy Link button in Share does not actually copy the link on Safari opensearch-project/OpenSearch-Dashboards#5629

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix copy link issue in Safari #1633

Fix copy link issue in Safari #1633

leanneeliatra commented Oct 24, 2023 •

edited by peternied

Loading

peternied commented Oct 31, 2023

leanneeliatra commented Oct 31, 2023

codecov bot commented Nov 2, 2023 •

edited

Loading

peternied commented Nov 2, 2023

peternied commented Nov 8, 2023

leanneeliatra commented Nov 14, 2023

leanneeliatra commented Nov 15, 2023 •

edited

Loading

stephen-crawford left a comment

leanneeliatra commented Nov 23, 2023 •

edited

Loading

leanneeliatra commented Nov 24, 2023

leanneeliatra commented Nov 27, 2023 •

edited

Loading

derek-ho left a comment

leanneeliatra commented Nov 28, 2023 •

edited

Loading

cwperks left a comment

Fix copy link issue in Safari #1633

Fix copy link issue in Safari #1633

Conversation

leanneeliatra commented Oct 24, 2023 • edited by peternied Loading

Description

Issues Resolved

Testing

Check List

peternied commented Oct 31, 2023

leanneeliatra commented Oct 31, 2023

codecov bot commented Nov 2, 2023 • edited Loading

Codecov Report

peternied commented Nov 2, 2023

peternied commented Nov 8, 2023

leanneeliatra commented Nov 14, 2023

leanneeliatra commented Nov 15, 2023 • edited Loading

stephen-crawford left a comment

Choose a reason for hiding this comment

leanneeliatra commented Nov 23, 2023 • edited Loading

leanneeliatra commented Nov 24, 2023

leanneeliatra commented Nov 27, 2023 • edited Loading

derek-ho left a comment

Choose a reason for hiding this comment

leanneeliatra commented Nov 28, 2023 • edited Loading

cwperks left a comment

Choose a reason for hiding this comment

leanneeliatra commented Oct 24, 2023 •

edited by peternied

Loading

codecov bot commented Nov 2, 2023 •

edited

Loading

leanneeliatra commented Nov 15, 2023 •

edited

Loading

leanneeliatra commented Nov 23, 2023 •

edited

Loading

leanneeliatra commented Nov 27, 2023 •

edited

Loading

leanneeliatra commented Nov 28, 2023 •

edited

Loading