Merge pull request #17 from zhangvi7/github/improvements

fix: github minor bug fixes/improvements

querybook/config/querybook_public_config.yaml

@@ -38,3 +38,6 @@ table_sampling:
     default_sample_rate: 0 # 0 means no sampling
     sample_user_guide_link: ''
     sampling_tool_tip_delay: 10000 # delay duration (ms) of sampling tool tip
+
+github_integration:
+    enabled: false

querybook/tests/test_lib/test_github_integration/test_github_client.py

@@ -30,105 +30,147 @@ def mock_repo():
 
 def test_initialization(mock_github, mock_github_link, mock_repo):
     access_token = "fake_token"
+    repo_name = "test_repo"
+    branch = "main"
     mock_github_instance = mock_github.return_value
     mock_github_instance.get_repo.return_value = mock_repo
-    client = GitHubClient(access_token=access_token, github_link=mock_github_link)
+    client = GitHubClient(
+        access_token=access_token,
+        repo_name=repo_name,
+        branch=branch,
+        github_link=mock_github_link,
+    )
     assert client.client is not None
     assert client.user is not None
     assert client.repo is not None
 
 
 def test_commit_datadoc_update(mock_github, mock_github_link, mock_repo):
     access_token = "fake_token"
+    repo_name = "test_repo"
+    branch = "main"
     mock_github_instance = mock_github.return_value
     mock_github_instance.get_repo.return_value = mock_repo
     mock_repo.get_contents.return_value = MagicMock(sha="fake_sha")
-    client = GitHubClient(access_token=access_token, github_link=mock_github_link)
+    client = GitHubClient(
+        access_token=access_token,
+        repo_name=repo_name,
+        branch=branch,
+        github_link=mock_github_link,
+    )
     client.commit_datadoc()
     mock_repo.update_file.assert_called_once()
 
     with pytest.raises(Exception) as excinfo:
         client = GitHubClient(
-            access_token="fake_token",
+            access_token="fake_token", repo_name=repo_name, branch=branch
         )
         client.commit_datadoc()
     assert "GitHub link is required for this operation" in str(excinfo.value)
 
 
 def test_commit_datadoc_create(mock_github, mock_github_link, mock_repo):
     access_token = "fake_token"
+    repo_name = "test_repo"
+    branch = "main"
     mock_github_instance = mock_github.return_value
     mock_github_instance.get_repo.return_value = mock_repo
     mock_repo.get_contents.side_effect = GithubException(404, "Not Found", None)
-    client = GitHubClient(access_token=access_token, github_link=mock_github_link)
+    client = GitHubClient(
+        access_token=access_token,
+        repo_name=repo_name,
+        branch=branch,
+        github_link=mock_github_link,
+    )
     client.commit_datadoc()
     mock_repo.create_file.assert_called_once()
 
     with pytest.raises(Exception) as excinfo:
         client = GitHubClient(
-            access_token="fake_token",
+            access_token="fake_token", repo_name=repo_name, branch=branch
         )
         client.commit_datadoc()
     assert "GitHub link is required for this operation" in str(excinfo.value)
 
 
 def test_get_datadoc_versions(mock_github, mock_github_link, mock_repo):
     access_token = "fake_token"
+    repo_name = "test_repo"
+    branch = "main"
     mock_github_instance = mock_github.return_value
     mock_github_instance.get_repo.return_value = mock_repo
     mock_commit = MagicMock()
     mock_commit.raw_data = {"sha": "123"}
     mock_commits = MagicMock()
     mock_commits.get_page.return_value = [mock_commit]
     mock_repo.get_commits.return_value = mock_commits
-    client = GitHubClient(access_token=access_token, github_link=mock_github_link)
+    client = GitHubClient(
+        access_token=access_token,
+        repo_name=repo_name,
+        branch=branch,
+        github_link=mock_github_link,
+    )
     versions = client.get_datadoc_versions()
     assert len(versions) == 1
     assert versions[0]["sha"] == "123"
 
     with pytest.raises(Exception) as excinfo:
         client = GitHubClient(
-            access_token="fake_token",
+            access_token="fake_token", repo_name=repo_name, branch=branch
         )
         client.get_datadoc_versions()
     assert "GitHub link is required for this operation" in str(excinfo.value)
 
 
 def test_get_repo_directories(mock_github, mock_github_link, mock_repo):
     access_token = "fake_token"
+    repo_name = "test_repo"
+    branch = "main"
     mock_github_instance = mock_github.return_value
     mock_github_instance.get_repo.return_value = mock_repo
     mock_directory = MagicMock()
     mock_directory.type = "dir"
     mock_directory.path = "datadocs"
     mock_repo.get_contents.return_value = [mock_directory]
-    client = GitHubClient(access_token=access_token, github_link=mock_github_link)
+    client = GitHubClient(
+        access_token=access_token,
+        repo_name=repo_name,
+        branch=branch,
+        github_link=mock_github_link,
+    )
     directories = client.get_repo_directories()
     assert len(directories) == 1
     assert directories[0] == "datadocs"
 
 
 def test_get_datadoc_at_commit(mock_github, mock_github_link, mock_repo):
     access_token = "fake_token"
+    repo_name = "test_repo"
+    branch = "main"
     mock_github_instance = mock_github.return_value
     mock_github_instance.get_repo.return_value = mock_repo
     mock_file_contents = MagicMock()
     mock_file_contents.decoded_content = (
         b"---\nid: 1\ntitle: DataDoc\n---\n\n"
         b"<!--\nid: 1\ncell_type: text\ncreated_at: 2023-01-01T00:00:00Z\n"
         b"updated_at: 2023-01-01T00:00:00Z\nmeta: {}\n-->\n"
-        b"## Text\n\n```text\nContent\n```\n"
+        b"## Text\n\nContent\n"
     )
     mock_repo.get_contents.return_value = mock_file_contents
-    client = GitHubClient(access_token=access_token, github_link=mock_github_link)
+    client = GitHubClient(
+        access_token=access_token,
+        repo_name=repo_name,
+        branch=branch,
+        github_link=mock_github_link,
+    )
     datadoc = client.get_datadoc_at_commit(commit_sha="fake_sha")
     assert datadoc.title == "DataDoc"
     assert datadoc.id == 1
     assert datadoc.cells[0].context == "Content"
 
     with pytest.raises(Exception) as excinfo:
         client = GitHubClient(
-            access_token="fake_token",
+            access_token="fake_token", repo_name=repo_name, branch=branch
         )
         client.get_datadoc_at_commit(commit_sha="fake_sha")
     assert "GitHub link is required for this operation" in str(excinfo.value)

querybook/webapp/components/DataDocGitHub/CommitCard.tsx

@@ -1,5 +1,7 @@
 import React from 'react';
 
+import { ComponentType, ElementType } from 'const/analytics';
+import { trackClick } from 'lib/analytics';
 import { ICommit } from 'resource/github';
 import { AsyncButton } from 'ui/AsyncButton/AsyncButton';
 import { Button } from 'ui/Button/Button';
@@ -48,7 +50,13 @@ export const CommitCard: React.FC<IProps> = ({
                 pushable
             />
             <Button
-                onClick={() => onCompare(version)}
+                onClick={() => {
+                    trackClick({
+                        component: ComponentType.GITHUB,
+                        element: ElementType.GITHUB_COMPARE_BUTTON,
+                    });
+                    onCompare(version);
+                }}
                 className="ml8"
                 title="Compare"
                 hoverColor="var(--color-accent-dark)"

querybook/webapp/components/DataDocGitHub/GitHubDirectory.tsx

@@ -18,7 +18,24 @@ interface IProps {
 }
 
 const validationSchema = Yup.object().shape({
-    directory: Yup.string().notRequired(),
+    /**
+     * Regex Examples:
+     * Valid:
+     * - parent
+     * - parent/child
+     * - parent/child_grandchild
+     *
+     * Invalid:
+     * - parent/               (Trailing slash)
+     * - parent//child         (Consecutive slashes)
+     * - parent/child#         (Invalid character '#')
+     */
+    directory: Yup.string()
+        .notRequired()
+        .matches(
+            /^(?!.*\/$)(?!.*\/\/)[A-Za-z0-9_-]+(?:\/[A-Za-z0-9_-]+)*$/,
+            'Invalid directory path. Use letters, numbers, "_", or "-". No trailing or consecutive "/". Example: parent/child'
+        ),
 });
 
 const DEFAULT_DIRECTORY = 'datadocs';
@@ -49,15 +66,6 @@ export const GitHubDirectory: React.FC<IProps> = ({
         fetchDirectories();
     }, [fetchDirectories]);
 
-    const directoryOptions = useMemo(
-        () =>
-            directories.map((dir) => ({
-                label: dir,
-                value: dir,
-            })),
-        [directories]
-    );
-
     const handleSubmit = async (values: { directory: string }) => {
         const directory = values.directory || DEFAULT_DIRECTORY;
         try {
@@ -85,21 +93,22 @@ export const GitHubDirectory: React.FC<IProps> = ({
                             name="directory"
                             label="Search Directory:"
                             type="react-select"
-                            options={directoryOptions}
+                            options={directories}
                             creatable
                             formatCreateLabel={(inputValue) => (
                                 <div className="flex-row">
                                     <Icon name="Plus" size={16} />{' '}
                                     <span>Create '{inputValue}' directory</span>
                                 </div>
                             )}
-                            onCreateOption={(inputValue) =>
-                                setFieldValue('directory', inputValue)
-                            }
-                            onChange={(option) => {
-                                setFieldValue('directory', option);
+                            onCreateOption={(inputValue) => {
+                                setDirectories((prev) => [...prev, inputValue]);
+                                setFieldValue('directory', inputValue);
                             }}
-                            help={`Select or create a directory for DataDoc commits. Defaults to ${DEFAULT_DIRECTORY} if left empty.`}
+                            onChange={(option) =>
+                                setFieldValue('directory', option)
+                            }
+                            help={`Select or create a directory for DataDoc commits. You can input nested directory paths like 'parent/child'. Defaults to ${DEFAULT_DIRECTORY} if left empty.`}
                         />
                         <div className="mt8">
                             <AsyncButton

querybook/webapp/components/DataDocGitHub/GitHubIntegration.tsx

@@ -38,7 +38,7 @@ export const GitHubIntegration: React.FC<IProps> = ({ docId, onClose }) => {
         checkStatus();
     }, [docId]);
 
-    const handleAuthenticateGitHub = useCallback(async () => {
+    const handleAuthorizeGitHub = useCallback(async () => {
         trackClick({
             component: ComponentType.GITHUB,
             element: ElementType.GITHUB_CONNECT_BUTTON,
@@ -67,18 +67,16 @@ export const GitHubIntegration: React.FC<IProps> = ({ docId, onClose }) => {
         }
     }, []);
 
-    if (isLoading) {
-        return <Loading fullHeight text="Loading, please wait..." />;
-    }
-
     return (
         <Modal
             onHide={onClose}
             title="GitHub Integration"
             className="GitHubIntegration"
         >
-            {!isAuthorized ? (
-                <GitHubAuth onAuthorize={handleAuthenticateGitHub} />
+            {isLoading ? (
+                <Loading fullHeight text="Loading, please wait..." />
+            ) : !isAuthorized ? (
+                <GitHubAuth onAuthorize={handleAuthorizeGitHub} />
             ) : (
                 <GitHubFeatures docId={docId} />
             )}

querybook/webapp/components/DataDocGitHub/GitHubVersions.tsx

@@ -1,9 +1,11 @@
 import React, { useCallback, useState } from 'react';
 
 import { QueryComparison } from 'components/TranspileQueryModal/QueryComparison';
+import { ComponentType, ElementType } from 'const/analytics';
 import { useRestoreDataDoc } from 'hooks/dataDoc/useRestoreDataDoc';
 import { usePaginatedResource } from 'hooks/usePaginatedResource';
 import { useResource } from 'hooks/useResource';
+import { trackClick } from 'lib/analytics';
 import { GitHubResource, ICommit } from 'resource/github';
 import { AsyncButton } from 'ui/AsyncButton/AsyncButton';
 import { IconButton } from 'ui/Button/IconButton';
@@ -95,8 +97,14 @@ export const GitHubVersions: React.FunctionComponent<IProps> = ({
     );
 
     const toggleFullScreen = useCallback(() => {
+        if (!isFullScreen) {
+            trackClick({
+                component: ComponentType.GITHUB,
+                element: ElementType.GITHUB_COMPARE_FULLSCREEN_BUTTON,
+            });
+        }
         setIsFullScreen((prev) => !prev);
-    }, []);
+    }, [isFullScreen]);
 
     if (!linkedDirectory) {
         return (

querybook/webapp/components/DataDocRightSidebar/DataDocRightSidebar.tsx

@@ -6,6 +6,7 @@ import { DataDocDAGExporterButton } from 'components/DataDocDAGExporter/DataDocD
 import { DataDocGitHubButton } from 'components/DataDocGitHub/DataDocGitHubButton';
 import { DataDocTemplateButton } from 'components/DataDocTemplateButton/DataDocTemplateButton';
 import { DataDocUIGuide } from 'components/UIGuide/DataDocUIGuide';
+import PublicConfig from 'config/querybook_public_config.yaml';
 import { ComponentType, ElementType } from 'const/analytics';
 import { IDataDoc, IDataDocMeta } from 'const/datadoc';
 import { useAnnouncements } from 'hooks/redux/useAnnouncements';
@@ -49,6 +50,7 @@ export const DataDocRightSidebar: React.FunctionComponent<IProps> = ({
 }) => {
     const numAnnouncements = useAnnouncements().length;
     const exporterExists = useExporterExists();
+    const githubIntegrationEnabled = PublicConfig.github_integration.enabled;
 
     const selfRef = React.useRef<HTMLDivElement>();
     const { showScrollToTop, scrollToTop } = useScrollToTop({
@@ -84,7 +86,9 @@ export const DataDocRightSidebar: React.FunctionComponent<IProps> = ({
         <DataDocRunAllButton docId={dataDoc.id} />
     );
 
-    const githubButtonDOM = <DataDocGitHubButton docId={dataDoc.id} />;
+    const githubButtonDOM = githubIntegrationEnabled && (
+        <DataDocGitHubButton docId={dataDoc.id} />
+    );
 
     const buttonSection = (
         <div className="DataDocRightSidebar-button-section vertical-space-between">

querybook/webapp/config.d.ts

@@ -126,6 +126,9 @@ declare module 'config/querybook_public_config.yaml' {
             sample_user_guide_link: string;
             sampling_tool_tip_delay: number;
         };
+        github_integration: {
+            enabled: boolean;
+        };
     };
     export default data;
 }

querybook/webapp/const/analytics.ts

@@ -119,8 +119,9 @@ export enum ElementType {
 
     // Github Integration
     GITHUB_CONNECT_BUTTON = 'GITHUB_CONNECT_BUTTON',
-    GITHUB_LINK_BUTTON = 'GITHUB_LINK_BUTTON',
     GITHUB_RESTORE_DATADOC_BUTTON = 'GITHUB_RESTORE_DATADOC_BUTTON',
+    GITHUB_COMPARE_BUTTON = 'GITHUB_COMPARE_BUTTON',
+    GITHUB_COMPARE_FULLSCREEN_BUTTON = 'GITHUB_COMPARE_FULLSCREEN_BUTTON',
 }
 
 export interface EventData {