Display stored procedures in graphs #639
Conversation
| @@ -20,7 +21,7 @@ export class DocDBStoredProceduresTreeItem extends DocDBTreeItemBase<ProcedureMe | |||
| public readonly contextValue: string = DocDBStoredProceduresTreeItem.contextValue; | |||
| public readonly childTypeLabel: string = "Stored Procedure"; | |||
|
|
|||
| constructor(endpoint: string, masterKey: string, private _collection: DocDBCollectionTreeItem, isEmulator: boolean) { | |||
| constructor(endpoint: string, masterKey: string, private _collection: DocDBCollectionTreeItem | GraphCollectionTreeItem, isEmulator: boolean) { | |||
There was a problem hiding this comment.
I feel the change made to DocDBStoredProcedure is minimal, hence it doesn't warrant any refactoring.
| * Copyright (c) Microsoft Corporation. All rights reserved. | ||
| * Licensed under the MIT License. See License.txt in the project root for license information. | ||
| *--------------------------------------------------------------------------------------------*/ | ||
|
|
There was a problem hiding this comment.
The name is not the clearest. Would GraphViewTreeItem be clearer?
There was a problem hiding this comment.
I think GraphTreeItem is fine.
| public hasMoreChildren(): boolean { | ||
| return false; | ||
| } | ||
|
|
There was a problem hiding this comment.
I see some repeated code, but not enough to make a DocDBCollectionTreeItemBase as opposed to DocDBDatabaseTreeItemBase
Also, see #414 - I did not want to perpetuate that.
|
What does it look like with mongo? I remember talking about this structure a while back and the original thought included a "documents" node, did that go away? |
|
| @@ -18,7 +19,7 @@ export class DocDBStoredProcedureTreeItem implements IAzureTreeItem { | |||
| public readonly contextValue: string = DocDBStoredProcedureTreeItem.contextValue; | |||
| public readonly commandId: string = 'cosmosDB.openStoredProcedure'; | |||
|
|
|||
| constructor(private _endpoint: string, private _masterKey: string, private _isEmulator: boolean, private _collection: DocDBCollectionTreeItem, public procedure: ProcedureMeta) { | |||
| constructor(private _endpoint: string, private _masterKey: string, private _isEmulator: boolean, private _collection: DocDBCollectionTreeItem | GraphCollectionTreeItem, public procedure: ProcedureMeta) { | |||
There was a problem hiding this comment.
Offline feedback: The only reason _collection is required is for the DocumentClient here. Consider:
- An interface type to handle this.
- Passing just the client.
- Passing merely the getDocumentClient. This feels weird to me because a parameterless function storing state seems obfuscated if I were to debug this later.
@StephenWeatherford was there anything else I missed?
|
Commit performs option 2 from this previous comment |
|
Ping |
Fixes #422
Extension:
Portal:
Leaving some comments at places I would like some clarification.