Skip to content

Lift the notebook's note listing into a hook and name a note's kind #1120

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
yattias wants to merge 1 commit into
base: ai-mode-workspace/start-screen
Choose a base branch
from
Open

Lift the notebook's note listing into a hook and name a note's kind #1120

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 6 additions & 26 deletions components/Notebook/NoteEditorLayout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,10 @@
import { FeatureFlag, isFeatureEnabled } from '@/utils/featureFlags';
import { LegacyNoteBanner } from '@/components/LegacyNoteBanner';
import {
getNoteKind,
isChangelogNote,
isPublishedRegisteredReportNote,
isRegisteredReportNote,
NOTE_KIND_LABELS,
} from '@/types/note';

// Persisted (per-user) flag so the guided tour auto-runs only once — the very
Expand All @@ -45,30 +46,6 @@
// one), which is the only moment we want to auto-launch the tour.
const NEW_NOTE_PARAMS = ['newChangelog', 'newGrant', 'newFunding', 'template'];

// Friendly label for the note's work type, shown at the top-left of the doc.
function getWorkTypeLabel(
documentType?: string | null,
contentType?: string | null,
isRegisteredReport?: boolean
): string | undefined {
if (isRegisteredReport) {
return 'Registered Report';
}

switch (documentType) {
case 'GRANT':
return 'Request for Proposal';
case 'PREREGISTRATION':
return 'Proposal';
case 'DISCUSSION':
return 'Preprint';
}
if (contentType === 'funding_request') return 'Request for Proposal';
if (contentType === 'preregistration') return 'Proposal';
if (contentType) return 'Preprint';
return undefined;
}

interface NoteEditorLayoutProps {
/**
* Fires when the assistant docks or undocks. Docking is decided here — it
Expand Down Expand Up @@ -238,9 +215,12 @@
const isPublishedRegisteredReport = isPublishedRegisteredReportNote(note);
const isEditorReadOnly =
isPublishedRegisteredReport || (isLegacyNote && isFeatureEnabled(FeatureFlag.LegacyNoteBanner));
const noteKind = getNoteKind(note);
const workTypeLabel = isChangelog
? 'ChangeLog'
: getWorkTypeLabel(note?.documentType, note?.post?.contentType, isRegisteredReportNote(note));
: noteKind === 'other'
? undefined
: NOTE_KIND_LABELS[noteKind];

Check warning on line 223 in components/Notebook/NoteEditorLayout.tsx

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Extract this nested ternary operation into an independent statement. 


See more on https://sonarcloud.io/project/issues?id=ResearchHub_web&issues=AaDIX78ww8wUaF9auAfs&open=AaDIX78ww8wUaF9auAfs&pullRequest=1120

const renderEditor = () => {
// No note is targeted (notebook home) — render the landing view directly so
Expand Down
126 changes: 16 additions & 110 deletions contexts/NotebookContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type { ID } from '@/types/root';
import type { OrganizationUsers } from '@/types/organization';
import { useOrganizationContext } from './OrganizationContext';
import { useNoteDetailsSaver, type NoteDetailsSaver } from '@/hooks/useNoteDetailsSaver';
import { useOrganizationNotes } from '@/hooks/useOrganizationNotes';
import { Editor } from '@tiptap/core';
import { useParams } from 'next/navigation';

Expand Down Expand Up @@ -68,9 +69,6 @@ interface NotebookContextType {

const NotebookContext = createContext<NotebookContextType | null>(null);

const mergeNotesById = (notes: Note[], otherNotes: Note[]): Note[] =>
Array.from(new Map([...notes, ...otherNotes].map((note) => [note.id, note])).values());

interface NotebookProviderProps {
readonly children: ReactNode;
readonly noteId?: string;
Expand All @@ -82,13 +80,9 @@ export function NotebookProvider({ children, noteId: explicitNoteId }: NotebookP

const { selectedOrg, isLoading: isLoadingOrg } = useOrganizationContext();

// Notes list state
const [notes, setNotes] = useState<Note[]>([]);
const [isLoadingNotes, setIsLoadingNotes] = useState(true);
const [isLoadingMoreNotes, setIsLoadingMoreNotes] = useState(false);
const [notesError, setNotesError] = useState<Error | null>(null);
const [totalCount, setTotalCount] = useState(0);
const [nextPageUrls, setNextPageUrls] = useState<string[]>([]);
// The organization's notes, loaded as soon as the organization is known.
const notesList = useOrganizationNotes(selectedOrg?.slug, { waiting: isLoadingOrg });
const { setNotes, refresh: refreshNotes } = notesList;

// Organization users state
const [users, setUsers] = useState<OrganizationUsers | null>(null);
Expand All @@ -106,40 +100,6 @@ export function NotebookProvider({ children, noteId: explicitNoteId }: NotebookP

const { saveDetailsSoon, saveDetailsNow } = useNoteDetailsSaver(currentNote?.id);

const fetchNotes = useCallback(async (slug?: string) => {
if (!slug) {
setNotesError(new Error('No organization slug provided'));
return;
}

setIsLoadingNotes(true);
setIsLoadingMoreNotes(false);
setNotesError(null);
setNextPageUrls([]);

try {
const [organizationNotes, registeredReports] = await Promise.all([
NoteService.getOrganizationNotes(slug),
NoteService.getOrganizationNotes(slug, {
documentType: 'REGISTERED_REPORT',
}),
]);
const mergedNotes = mergeNotesById(organizationNotes.results, registeredReports.results);

setNotes(mergedNotes);
setTotalCount(Math.max(organizationNotes.count, mergedNotes.length));
setNextPageUrls(
[organizationNotes.next, registeredReports.next].filter((url) => url !== null)
);
} catch (err) {
setNotesError(err instanceof Error ? err : new Error('Failed to load notes'));
setNotes([]);
setTotalCount(0);
} finally {
setIsLoadingNotes(false);
}
}, []);

const fetchUsers = useCallback(async (orgId: string, silently = false) => {
if (!silently) {
setIsLoadingUsers(true);
Expand Down Expand Up @@ -173,51 +133,6 @@ export function NotebookProvider({ children, noteId: explicitNoteId }: NotebookP
[selectedOrg?.id, fetchUsers]
);

const refreshNotes = useCallback(async () => {
if (!selectedOrg?.slug) {
setNotesError(new Error('No organization slug provided'));
return;
}
await fetchNotes(selectedOrg.slug);
}, [selectedOrg?.slug, fetchNotes]);

const loadMoreNotes = () => {
if (isLoadingMoreNotes || nextPageUrls.length === 0) return;

setNotesError(null);
setIsLoadingMoreNotes(true);
};

useEffect(() => {
const slug = selectedOrg?.slug;
if (!slug || !isLoadingMoreNotes || nextPageUrls.length === 0) return;

let cancelled = false;

const fetchNextNotes = async () => {
try {
const nextPages = await Promise.all(
nextPageUrls.map((nextUrl) => NoteService.getOrganizationNotes(slug, { nextUrl }))
);
if (cancelled) return;

const newNotes = nextPages.flatMap((page) => page.results);
setNotes((currentNotes) => mergeNotesById(currentNotes, newNotes));
setNextPageUrls(nextPages.flatMap(({ next }) => (next ? [next] : [])));
} catch (err) {
if (cancelled) return;
setNotesError(err instanceof Error ? err : new Error('Failed to load more notes'));
} finally {
if (!cancelled) setIsLoadingMoreNotes(false);
}
};

void fetchNextNotes();
return () => {
cancelled = true;
};
}, [isLoadingMoreNotes, nextPageUrls, selectedOrg?.slug]);

const loadNote = useCallback(async (noteId: string) => {
if (noteId === lastLoadedNoteIdRef.current) {
return;
Expand Down Expand Up @@ -266,38 +181,30 @@ export function NotebookProvider({ children, noteId: explicitNoteId }: NotebookP
const refreshAll = useCallback(async () => {
if (!selectedOrg?.slug || !selectedOrg?.id) return;

const promises = [fetchNotes(selectedOrg.slug), fetchUsers(selectedOrg.id.toString())];
const promises = [refreshNotes(), fetchUsers(selectedOrg.id.toString())];
if (activeNoteId) {
promises.push(loadNote(activeNoteId));
}

await Promise.all(promises);
}, [selectedOrg?.slug, selectedOrg?.id, activeNoteId, fetchNotes, fetchUsers, loadNote]);
}, [selectedOrg?.slug, selectedOrg?.id, activeNoteId, refreshNotes, fetchUsers, loadNote]);

// Initial data loading when organization changes
// Users load when the organization changes; the notes list does the same on its own.
useEffect(() => {
if (isLoadingOrg) {
setIsLoadingNotes(true);
setIsLoadingUsers(true);
return;
}

if (!selectedOrg) {
setNotes([]);
setTotalCount(0);
setIsLoadingMoreNotes(false);
setNextPageUrls([]);
setUsers(null);
setNotesError(null);
setUsersError(null);
setIsLoadingNotes(false);
setIsLoadingUsers(false);
return;
}

fetchNotes(selectedOrg.slug);
fetchUsers(selectedOrg.id.toString());
}, [selectedOrg?.slug, selectedOrg?.id, isLoadingOrg, fetchNotes, fetchUsers]);
}, [selectedOrg?.id, isLoadingOrg, fetchUsers]);

useEffect(() => {
if (activeNoteId) {
Expand All @@ -306,19 +213,18 @@ export function NotebookProvider({ children, noteId: explicitNoteId }: NotebookP
}, [activeNoteId, loadNote]);

// Calculate overall loading state ignoring isLoadingNote
const isLoading = isLoadingNotes || isLoadingUsers || isLoadingOrg;
const hasMoreNotes = nextPageUrls.length > 0;
const isLoading = notesList.isLoading || isLoadingUsers || isLoadingOrg;

const value = {
notes,
notes: notesList.notes,
setNotes,
isLoadingNotes,
isLoadingMoreNotes,
notesError,
totalCount,
hasMoreNotes,
isLoadingNotes: notesList.isLoading,
isLoadingMoreNotes: notesList.isLoadingMore,
notesError: notesList.error,
totalCount: notesList.totalCount,
hasMoreNotes: notesList.hasMore,
refreshNotes,
loadMoreNotes,
loadMoreNotes: notesList.loadMore,
users,
isLoadingUsers,
usersError,
Expand Down
Loading
Loading