Fix search for pinned notebooks (#12719) (#12766)

* fix search for pinned notebooks

* fix filtering when verifying that a search folder is not a subdirectory from the current folder queries path

* Show book node on pinned notebooks search results

* fix parent node on pinned notebooks search results

* fix search for pinned notebook and modify how pinned notebooks are stored in workspace

* update format of pinned notebooks for users that used the september release version

* removed unused functions

* Address PR comments

* fix parent node for legacy version of jupyter books

* remove cast from book path

extensions/notebook/package.json

@@ -65,7 +65,7 @@
           "default": [],
           "description": "%notebook.pinnedNotebooks.description%",
           "items": {
-            "type": "string"
+            "type": "object"
           }
         }
       }

extensions/notebook/src/book/bookModel.ts

@@ -35,7 +35,8 @@ export class BookModel {
 		public readonly bookPath: string,
 		public readonly openAsUntitled: boolean,
 		public readonly isNotebook: boolean,
-		private _extensionContext: vscode.ExtensionContext) {
+		private _extensionContext: vscode.ExtensionContext,
+		public readonly notebookRootPath?: string) {
 		this._bookItems = [];
 	}
 
@@ -107,7 +108,7 @@ export class BookModel {
 		let notebookItem = new BookTreeItem({
 			title: pathDetails.name,
 			contentPath: this.bookPath,
-			root: pathDetails.dir,
+			root: this.notebookRootPath ? this.notebookRootPath : pathDetails.dir,
 			tableOfContents: { sections: undefined },
 			page: { sections: undefined },
 			type: BookTreeItemType.Notebook,

extensions/notebook/src/book/bookPinManager.ts

@@ -6,7 +6,7 @@ import * as path from 'path';
 import * as vscode from 'vscode';
 import * as constants from './../common/constants';
 import { BookTreeItem } from './bookTreeItem';
-import { getPinnedNotebooks } from '../common/utils';
+import { getPinnedNotebooks, setPinnedBookPathsInConfig, IBookNotebook } from '../common/utils';
 
 export interface IBookPinManager {
 	pinNotebook(notebook: BookTreeItem): boolean;
@@ -33,7 +33,7 @@ export class BookPinManager implements IBookPinManager {
 	}
 
 	isNotebookPinned(notebookPath: string): boolean {
-		if (getPinnedNotebooks().findIndex(x => x === notebookPath) > -1) {
+		if (getPinnedNotebooks().findIndex(x => x.notebookPath === notebookPath) > -1) {
 			return true;
 		}
 		return false;
@@ -47,41 +47,23 @@ export class BookPinManager implements IBookPinManager {
 		return this.updatePinnedBooks(notebook, PinBookOperation.Unpin);
 	}
 
-	getPinnedBookPathsInConfig(): string[] {
-		let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(constants.notebookConfigKey);
-		let pinnedBookDirectories: string[] = config.get(constants.pinnedBooksConfigKey);
-
-		return pinnedBookDirectories;
-	}
-
-	setPinnedBookPathsInConfig(bookPaths: string[]) {
-		let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(constants.notebookConfigKey);
-		let storeInWorspace: boolean = this.hasWorkspaceFolders();
-
-		config.update(constants.pinnedBooksConfigKey, bookPaths, storeInWorspace ? false : vscode.ConfigurationTarget.Global);
-	}
-
-	hasWorkspaceFolders(): boolean {
-		let workspaceFolders = vscode.workspace.workspaceFolders;
-		return workspaceFolders && workspaceFolders.length > 0;
-	}
-
 	updatePinnedBooks(notebook: BookTreeItem, operation: PinBookOperation) {
 		let modifiedPinnedBooks = false;
 		let bookPathToChange: string = notebook.book.contentPath;
 
-		let pinnedBooks: string[] = this.getPinnedBookPathsInConfig();
-		let existingBookIndex = pinnedBooks.map(pinnedBookPath => path.normalize(pinnedBookPath)).indexOf(bookPathToChange);
+		let pinnedBooks: IBookNotebook[] = getPinnedNotebooks();
+		let existingBookIndex = pinnedBooks.map(pinnedBookPath => path.normalize(pinnedBookPath?.notebookPath)).indexOf(bookPathToChange);
 
 		if (existingBookIndex !== -1 && operation === PinBookOperation.Unpin) {
 			pinnedBooks.splice(existingBookIndex, 1);
 			modifiedPinnedBooks = true;
 		} else if (existingBookIndex === -1 && operation === PinBookOperation.Pin) {
-			pinnedBooks.push(bookPathToChange);
+			let addNotebook: IBookNotebook = { notebookPath: bookPathToChange, bookPath: notebook.book.root };
+			pinnedBooks.push(addNotebook);
 			modifiedPinnedBooks = true;
 		}
 
-		this.setPinnedBookPathsInConfig(pinnedBooks);
+		setPinnedBookPathsInConfig(pinnedBooks);
 		this.setPinnedSectionContext();
 
 		return modifiedPinnedBooks;

extensions/notebook/src/book/bookTreeItem.ts

@@ -39,6 +39,7 @@ export class BookTreeItem extends vscode.TreeItem {
 	private _nextUri: string;
 	public readonly version: string;
 	public command: vscode.Command;
+	public resourceUri: vscode.Uri;
 
 	constructor(public book: BookTreeItemFormat, icons: any) {
 		super(book.title, book.treeItemCollapsibleState);
@@ -74,6 +75,7 @@ export class BookTreeItem extends vscode.TreeItem {
 		}
 		else {
 			this.tooltip = this.book.type === BookTreeItemType.Book ? (this.book.version === BookVersion.v1 ? path.join(this.book.root, content) : this.book.root) : this.book.contentPath;
+			this.resourceUri = vscode.Uri.file(this.book.root);
 		}
 	}
 

extensions/notebook/src/book/bookTreeView.ts

@@ -57,9 +57,9 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 
 	private async initialize(workspaceFolders: vscode.WorkspaceFolder[]): Promise<void> {
 		if (this.viewId === constants.PINNED_BOOKS_VIEWID) {
-			await Promise.all(getPinnedNotebooks().map(async (notebookPath) => {
+			await Promise.all(getPinnedNotebooks().map(async (notebook) => {
 				try {
-					await this.createAndAddBookModel(notebookPath, true);
+					await this.createAndAddBookModel(notebook.notebookPath, true, notebook.bookPath);
 				} catch {
 					// no-op, not all workspace folders are going to be valid books
 				}
@@ -90,7 +90,7 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 	}
 
 	trustBook(bookTreeItem?: BookTreeItem): void {
-		let bookPathToTrust = bookTreeItem ? bookTreeItem.root : this.currentBook?.bookPath;
+		let bookPathToTrust: string = bookTreeItem ? bookTreeItem.root : this.currentBook?.bookPath;
 		if (bookPathToTrust) {
 			let trustChanged = this._bookTrustManager.setBookAsTrusted(bookPathToTrust);
 			if (trustChanged) {
@@ -169,7 +169,8 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 	async addNotebookToPinnedView(bookItem: BookTreeItem): Promise<void> {
 		let notebookPath: string = bookItem.book.contentPath;
 		if (notebookPath) {
-			await this.createAndAddBookModel(notebookPath, true);
+			let rootPath: string = bookItem.book.root ? bookItem.book.root : '';
+			await this.createAndAddBookModel(notebookPath, true, rootPath);
 		}
 	}
 
@@ -215,10 +216,12 @@ export class BookTreeViewProvider implements vscode.TreeDataProvider<BookTreeIte
 	 * Creates a model for the specified folder path and adds it to the known list of books if we
 	 * were able to successfully parse it.
 	 * @param bookPath The path to the book folder to create the model for
+	 * @param isNotebook A boolean value to know we are creating a model for a notebook or a book
+	 * @param notebookBookRoot For pinned notebooks we need to know if the notebook is part of a book or it's a standalone notebook
 	 */
-	private async createAndAddBookModel(bookPath: string, isNotebook: boolean): Promise<void> {
+	private async createAndAddBookModel(bookPath: string, isNotebook: boolean, notebookBookRoot?: string): Promise<void> {
 		if (!this.books.find(x => x.bookPath === bookPath)) {
-			const book: BookModel = new BookModel(bookPath, this._openAsUntitled, isNotebook, this._extensionContext);
+			const book: BookModel = new BookModel(bookPath, this._openAsUntitled, isNotebook, this._extensionContext, notebookBookRoot);
 			await book.initializeContents();
 			this.books.push(book);
 			if (!this.currentBook) {

extensions/notebook/src/common/utils.ts

@@ -325,16 +325,45 @@ export async function getRandomToken(size: number = 24): Promise<string> {
 }
 
 export function isBookItemPinned(notebookPath: string): boolean {
-	let pinnedNotebooks: string[] = getPinnedNotebooks();
-	if (pinnedNotebooks?.indexOf(notebookPath) > -1) {
+	let pinnedNotebooks: IBookNotebook[] = getPinnedNotebooks();
+	if (pinnedNotebooks?.find(x => x.notebookPath === notebookPath)) {
 		return true;
 	}
 	return false;
 }
 
-export function getPinnedNotebooks(): string[] {
+export function getPinnedNotebooks(): IBookNotebook[] {
 	let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(notebookConfigKey);
-	let pinnedNotebooks: string[] = config.get(pinnedBooksConfigKey) ?? [];
-
-	return pinnedNotebooks;
+	let pinnedNotebooks: [] = config.get(pinnedBooksConfigKey);
+	let updateFormat: boolean = false;
+	const pinnedBookDirectories = pinnedNotebooks.map(elem => {
+		if (typeof (elem) === 'string') {
+			updateFormat = true;
+			return { notebookPath: elem, bookPath: '' };
+		} else {
+			return elem as IBookNotebook;
+		}
+	});
+	if (updateFormat) {
+		//Need to modify the format of how pinnedNotebooks are stored for users that used the September release version.
+		setPinnedBookPathsInConfig(pinnedBookDirectories);
+	}
+	return pinnedBookDirectories;
+}
+
+function hasWorkspaceFolders(): boolean {
+	let workspaceFolders = vscode.workspace.workspaceFolders;
+	return workspaceFolders && workspaceFolders.length > 0;
+}
+
+export function setPinnedBookPathsInConfig(pinnedNotebookPaths: IBookNotebook[]) {
+	let config: vscode.WorkspaceConfiguration = vscode.workspace.getConfiguration(notebookConfigKey);
+	let storeInWorspace: boolean = hasWorkspaceFolders();
+
+	config.update(pinnedBooksConfigKey, pinnedNotebookPaths, storeInWorspace ? false : vscode.ConfigurationTarget.Global);
+}
+
+export interface IBookNotebook {
+	bookPath?: string;
+	notebookPath: string;
 }

extensions/notebook/src/extension.ts

@@ -43,7 +43,7 @@ export async function activate(extensionContext: vscode.ExtensionContext): Promi
 	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.openBook', () => bookTreeViewProvider.openNewBook()));
 	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.closeBook', (book: any) => bookTreeViewProvider.closeBook(book)));
 	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.closeNotebook', (book: any) => bookTreeViewProvider.closeBook(book)));
-	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.openNotebookFolder', (folderPath?: string, urlToOpen?: string, showPreview?: boolean,) => bookTreeViewProvider.openNotebookFolder(folderPath, urlToOpen, showPreview)));
+	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.openNotebookFolder', (folderPath?: string, urlToOpen?: string, showPreview?: boolean) => bookTreeViewProvider.openNotebookFolder(folderPath, urlToOpen, showPreview)));
 	extensionContext.subscriptions.push(vscode.commands.registerCommand('notebook.command.pinNotebook', async (book: any) => {
 		await bookTreeViewProvider.pinNotebook(book);
 		await pinnedBookTreeViewProvider.addNotebookToPinnedView(book);

extensions/notebook/src/test/common/utils.test.ts

@@ -340,7 +340,7 @@ describe('Utils Tests', function () {
 
 	describe('getPinnedNotebooks', function (): void {
 		it('Should NOT have any pinned notebooks', async function (): Promise<void> {
-			let pinnedNotebooks: string[] = utils.getPinnedNotebooks();
+			let pinnedNotebooks: utils.IBookNotebook[] = utils.getPinnedNotebooks();
 
 			should(pinnedNotebooks.length).equal(0, 'Should not have any pinned notebooks');
 		});

src/sql/workbench/contrib/notebook/browser/notebookExplorer/notebookExplorerViewlet.ts

@@ -270,15 +270,40 @@ export class NotebookExplorerViewPaneContainer extends ViewPaneContainer {
 						if (treeView.dataProvider) {
 							let items = await treeView?.dataProvider.getChildren(treeView?.root);
 							items?.forEach(root => {
-								if (root.contextValue !== 'pinnedNotebook') {
-									this.updateViewletsState();
-									let rootFolder = URI.file(isString(root.tooltip) ? root.tooltip : root.tooltip.value);
+								this.updateViewletsState();
+								if (root.contextValue === 'providedBook' || root.contextValue === 'savedBook') {
+									let rootFolder = URI.file(root.resourceUri.path);
 									let folderToSearch = { folder: rootFolder };
+									if (root.tooltip.toString().includes('content')) {
+										let pattern = {};
+										pattern['content/**'] = true;
+										folderToSearch['includePattern'] = pattern;
+									}
+									query.folderQueries = query.folderQueries.filter((folder) => {
+										if (!!folder.includePattern && !folder.includePattern.toString().startsWith('content')) {
+											//verify if pinned notebook is not opened in Books Viewlet
+											return path.relative(folder.folder.fsPath, folderToSearch.folder.fsPath) !== '..';
+										} else {
+											return true;
+										}
+									});
 									query.folderQueries.push(folderToSearch);
 									filesToIncludeFiltered = filesToIncludeFiltered + path.join(folderToSearch.folder.fsPath, '**', '*.md') + ',' + path.join(folderToSearch.folder.fsPath, '**', '*.ipynb') + ',';
-									this.searchView.startSearch(query, null, filesToIncludeFiltered, false, this.searchWidget);
+								} else {
+									let pattern = {};
+									let rootFolder = URI.file(root.resourceUri.path);
+									let pathToNotebook = isString(root.tooltip) ? root.tooltip : root.tooltip.value;
+									let baseName = path.join('**', path.basename(pathToNotebook));
+									pattern[baseName] = true;
+									let folderToSearch = { folder: rootFolder, includePattern: pattern };
+									query.folderQueries.push(folderToSearch);
+									filesToIncludeFiltered = filesToIncludeFiltered + rootFolder + ',';
 								}
 							});
+
+							if (query.folderQueries.length > 0) {
+								this.searchView.startSearch(query, null, filesToIncludeFiltered, false, this.searchWidget);
+							}
 						}
 					}
 				});

src/sql/workbench/contrib/notebook/browser/notebookExplorer/notebookSearch.ts

@@ -259,7 +259,7 @@ export class NotebookSearchView extends SearchView {
 			progressComplete();
 
 			// Do final render, then expand if just 1 file with less than 50 matches
-			await this.onSearchResultsChanged();
+			this.onSearchResultsChanged();
 
 			const collapseResults = this.searchConfig.collapseResults;
 			if (collapseResults !== 'alwaysCollapse' && this.viewModel.searchResult.matches().length === 1) {