notes: reduce gh api usage by delay of gh issues fetch until notes ar…

…e open (#1225)

* notes: reduce gh api usage by delay of initial gh issues fetch until notes is open

* Add tests around gh fetching in NotesControl.test.jsx

package.json

@@ -1,6 +1,6 @@
 {
   "name": "bldrs",
-  "version": "1.0.1093",
+  "version": "1.0.1068",
   "main": "src/index.jsx",
   "license": "AGPL-3.0",
   "homepage": "https://github.com/bldrs-ai/Share",

src/Components/Notes/NotesControl.jsx

@@ -31,49 +31,51 @@ export default function NotesControl() {
 
   // Fetch issues/notes
   useEffect(() => {
-    // TODO(pablo): NotesControl loads onViewer, bc viewer for non-logged in
-    // session is valid.  But!  When the model is private, there's a delayed
-    // load until after auth succeeds.  If we don't check model here, then Notes
-    // initially fails during an unauthenticated load via oauthproxy, which gets
-    // a 302 DIY, and somehow seems to keep that state in Octokit.
-    //
-    // We detect we're in a delayed load state here by checking model first,
-    // which then doesn't touch octokit until later when auth is available.
-    if (!model) {
-      return
-    }
-    (async () => {
-      toggleIsLoadingNotes()
-      try {
-        const newNotes = []
-        let issueIndex = 0
-        const issueArr = await getIssues(repository, accessToken)
-        debug().log('Notes#useEffect: issueArr: ', issueArr)
+    if (isNotesVisible) {
+      // TODO(pablo): NotesControl loads onViewer, bc viewer for non-logged in
+      // session is valid.  But!  When the model is private, there's a delayed
+      // load until after auth succeeds.  If we don't check model here, then Notes
+      // initially fails during an unauthenticated load via oauthproxy, which gets
+      // a 302 DIY, and somehow seems to keep that state in Octokit.
+      //
+      // We detect we're in a delayed load state here by checking model first,
+      // which then doesn't touch octokit until later when auth is available.
+      if (!model) {
+        return
+      }
+      (async () => {
+        toggleIsLoadingNotes()
+        try {
+          const newNotes = []
+          let issueIndex = 0
+          const issueArr = await getIssues(repository, accessToken)
+          debug().log('Notes#useEffect: issueArr: ', issueArr)
 
-        issueArr.reverse().map((issue, index) => {
-          newNotes.push({
-            index: issueIndex++,
-            id: issue.id,
-            number: issue.number,
-            title: issue.title || '',
-            body: issue.body || '',
-            date: issue.created_at,
-            username: issue.user.login,
-            avatarUrl: issue.user.avatar_url,
-            numberOfComments: issue.comments,
-            locked: issue.locked,
-            synched: true,
+          issueArr.reverse().map((issue, index) => {
+            newNotes.push({
+              index: issueIndex++,
+              id: issue.id,
+              number: issue.number,
+              title: issue.title || '',
+              body: issue.body || '',
+              date: issue.created_at,
+              username: issue.user.login,
+              avatarUrl: issue.user.avatar_url,
+              numberOfComments: issue.comments,
+              locked: issue.locked,
+              synched: true,
+            })
           })
-        })
-        setNotes(newNotes)
-        toggleIsLoadingNotes()
-      } catch (e) {
-        setSnackMessage({text: 'Notes: Cannot fetch from GitHub', autoDismiss: true})
-      }
-    })()
+          setNotes(newNotes)
+          toggleIsLoadingNotes()
+        } catch (e) {
+          setSnackMessage({text: 'Notes: Cannot fetch from GitHub', autoDismiss: true})
+        }
+      })()
+    }
     // TODO(pablo):
     // eslint-disable-next-line react-hooks/exhaustive-deps
-  }, [model, isCreateNoteVisible])
+  }, [isNotesVisible, model, isCreateNoteVisible])
 
 
   // TODO(pablo): hack, move into helper

src/Components/Notes/NotesControl.test.jsx

@@ -8,20 +8,49 @@ import model from '../../__mocks__/MockModel.js'
 
 
 describe('NotesControl', () => {
-  it('Fetch notes from issues endpoint and set notes in zustand', async () => {
+  it('Does not issue fetch on initial page load when not visible', async () => {
     const {result} = renderHook(() => useStore((state) => state))
-    await act(
-      async () => {
-        await act(() => {
-          result.current.setNotes(null)
-          result.current.setModel(model)
-          result.current.setRepository('pablo-mayrgundter', 'Share')
-        })
-      })
-      await act(async () => {
-        render(<ShareMock><NotesControl/></ShareMock>)
-      })
-      expect(result.current.notes).toHaveLength(4)
+    await act(async () => {
+      result.current.setNotes(null)
+      result.current.setModel(model)
+      result.current.setRepository('pablo-mayrgundter', 'Share')
     })
+    act(() => {
+      render(<ShareMock><NotesControl/></ShareMock>)
+    })
+    expect(result.current.notes).toBeNull()
+  })
+
+  it('Fetches issues on initial render when isNotesVisible in zustand', async () => {
+    const {result} = renderHook(() => useStore((state) => state))
+    await act(async () => {
+      result.current.setNotes(null)
+      result.current.setModel(model)
+      result.current.setRepository('pablo-mayrgundter', 'Share')
+      result.current.setIsNotesVisible(true)
+    })
+    await act(async () => {
+      render(<ShareMock><NotesControl/></ShareMock>)
+    })
+    expect(result.current.notes).toHaveLength(4)
+  })
+
+  it('Fetches issues when isNotesVisible in zustand', async () => {
+    const {result} = renderHook(() => useStore((state) => state))
+    await act(async () => {
+      result.current.setNotes(null)
+      result.current.setModel(model)
+      result.current.setRepository('pablo-mayrgundter', 'Share')
+      result.current.setIsNotesVisible(false)
+    })
+    await act(async () => {
+      render(<ShareMock><NotesControl/></ShareMock>)
+    })
+    expect(result.current.notes).toBeNull()
+    await act(async () => {
+      result.current.setIsNotesVisible(true)
+    })
+    expect(result.current.notes).toHaveLength(4)
   })
+})