[Dashboard] Fix "Unlink from library" panel title bug

[Heenawter]

Closes #156539
Closes #156544

Summary

As part of investigating the attached flaky test suite, I realized that the flakiness was because we weren't waiting long enough for a panel to be added and/or removed from the library - so, I added a check to ensure the library notification appears in saveToLibrary, and similarly I added a check to ensure that the library notification disappears in unlinkFromLibary.

However, after adding these extra checks, the following test started to fail:

kibana/x-pack/test/functional/apps/dashboard/group2/panel_titles.ts

Lines 148 to 155 in 23a45bd

	it('unlinking a by reference panel with a custom title will keep the current title', async () => {
	await dashboardPanelActions.customizePanel();
	await dashboardCustomizePanel.setCustomPanelTitle(CUSTOM_TITLE);
	await dashboardCustomizePanel.clickSaveButton();
	await dashboardPanelActions.unlinkFromLibary();
	const newPanelTitle = (await PageObjects.dashboard.getPanelTitles())[0];
	expect(newPanelTitle).to.equal(CUSTOM_TITLE);
	});

Way, way, way back in 8.1, one of my first PRs was meant to fix some problems with dashboard panel titles - as part of this, I was supposed to make sure that, if a by-reference panel is given a custom panel title, the title should remain the same after unlinking (i.e. it should remain as the custom title rather than resetting to the by-reference title). So, I added the above test to verify this behaviour.

Turns out, though, that this test had a flaw - because we weren't waiting long enough for the panel to actually be disconnected from the library, this test was only passing because it was grabbing and comparing titles before the unlink was complete - so, even though the title actually was being reset back to the original library title, this test did not catch this bug. This has been the case since 8.1 when this test was introduced - not sure how it was missed, but we never actually fixed the bug where dashboard panel titles are getting reset when unlinking from the library.

So, this PR actually accomplishes two things:

1. It fixes the flakiness of the attached tests by adding the extra library notification checks:
   
2. It ensures that, if a by-reference panel has a custom title, unlinking it from the library will not impact that title.

Before

Screen.Recording.2023-05-03.at.4.19.44.PM.mov

After

Screen.Recording.2023-05-03.at.4.21.09.PM.mov

Checklist

• Unit or functional tests were updated or added to match the most common scenarios
• This was checked for cross-browser compatibility

For maintainers

• This was checked for breaking API changes and was labeled appropriately

[Heenawter]

This was added as part of #122199 - I tried to find justification for this in the original PR, but couldn't find anything. So, after digging into my Slack discussions with @ThomThomson at the time, I believe this may have been left in accidentally.

Basically, since saved searches can only ever be cloned by reference, I wasn't sure how to handle custom titles when they were cloned - should they reflect the library title, or the custom title? I originally went with displaying the library title to make it clear that this object was cloned by reference, but @ThomThomson brought up that this might be confusing and was a fairly niche scenario - so ultimately, we decided that a saved search with a custom title, when cloned, should display the custom title in the dashboard while the cloned saved object would still be under the library title. This is what that looks like now:

Screen.Recording.2023-05-04.at.10.28.22.AM.mov

As mentioned, my best guess is that this line may have been left in accidentally when exploring these options (although this would have been silly, because getInputAsValueType isn't even called when cloning saved search embeddables since they are cloned by reference... 🤷) and, because the test meant to catch this was not working as expected, we didn't catch it at the time.

I've tested a bunch of different custom title + cloning scenarios, and I can't think of any reason this line would be necessary... So I think it's safe to remove so that we can get the desired behaviour when unlinking panels.

[Heenawter]

Now that we are waiting for the object to be added to the library as part of the saveToLibrary method, this was causing a failure because the 'confirmSaveSavedObjectButton' was no longer present on the screen.

[elasticmachine]

Pinging @elastic/kibana-presentation (Team:Presentation)

[kibana-ci]

💚 Build Succeeded

• Buildkite Build
• Commit: 3078225

Metrics [docs]

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id	before	after	diff
embeddable	75.7KB	75.7KB	-18.0B

Unknown metric groups

ESLint disabled line counts

id	before	after	diff
enterpriseSearch	19	21	+2
securitySolution	398	401	+3
total			+5

Total ESLint disabled count

id	before	after	diff
enterpriseSearch	20	22	+2
securitySolution	478	481	+3
total			+5

History

• 💚 Build #125356 succeeded fba01b5
• 💚 Build #125327 succeeded 2fd11d5
• 💔 Build #125256 failed f72ebcf

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @Heenawter

[nreese]

LGTM thanks for unskipping this test so quickly. Great to see tests getting re-enabled.
code review only

[stratoula]

Changes LGTM, code review only!

[kibanamachine]

💚 All backports created successfully

Status	Branch	Result
✅	8.8

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation