feat: forum monitor + batch download queue#22
Open
adi3433 wants to merge 1 commit into
Open
Conversation
Forum Monitor: - New Moodle AJAX service (mod_forum_get_forums_by_courses, mod_forum_get_forum_discussions) to fetch recent discussions across all enrolled courses - Forum store with persistence, staleness check, and recent-discussions selector - Dashboard "Forum Activity" section showing latest discussions with course name, reply count, and relative timestamps - Deferred fetch after initial dashboard load to avoid blocking startup - Forum state cleared on logout Download Queue: - New download queue store with concurrency-limited processor (2 parallel downloads) - "Download All" button on course resources header that enqueues all resource/folder files - Queue badge showing progress (completed/total) with live status - Full-screen queue modal with per-item progress bars, retry for failed items, remove/clear actions - Automatic queue processing — items download sequentially without user intervention TypeScript compiles clean. No changes to existing service logic.
📝 WalkthroughWalkthroughThe PR introduces forum discussion aggregation and display on the dashboard, plus a download queue system for managing bulk course resource downloads. It adds corresponding stores, API services, type definitions, and UI components to support both features, alongside auth store cleanup during logout. Changes
Sequence Diagram(s)sequenceDiagram
participant Dashboard as Dashboard Screen
participant ForumStore as Forum Store
participant ForumService as Forum Service
participant MoodleAPI as Moodle API
participant ForumSection as Forum Section
Dashboard->>ForumStore: useForumStore() hook + useEffect
Note over Dashboard: After hydration, not offline, sync complete
Dashboard->>ForumStore: fetchForumDiscussions({ silent: true })
ForumStore->>ForumService: fetchAllForumDiscussions()
ForumService->>MoodleAPI: GET /my/ (extract sesskey)
MoodleAPI-->>ForumService: sesskey
ForumService->>MoodleAPI: AJAX mod_forum_get_forums_by_courses
MoodleAPI-->>ForumService: forums list
ForumService->>MoodleAPI: AJAX mod_forum_get_forum_discussions (per forum, batch 3)
MoodleAPI-->>ForumService: discussions (enriched with course context)
ForumService-->>ForumStore: discussions sorted by timemodified desc
ForumStore->>ForumStore: persist to JSON storage
ForumStore-->>Dashboard: discussions loaded
Dashboard->>ForumSection: render with recent discussions
ForumSection-->>Dashboard: display cards
sequenceDiagram
participant CourseScreen as Course Resources Screen
participant QueueStore as Download Queue Store
participant DownloadService as Download Service
participant QueueModal as Download Queue Modal
CourseScreen->>QueueStore: useDownloadQueueStore()
CourseScreen->>CourseScreen: user taps "Download All"
CourseScreen->>QueueStore: enqueueAllFromCourse(courseId, tree)
QueueStore->>QueueStore: collect downloadable nodes, create items
QueueStore-->>CourseScreen: return count added
CourseScreen->>CourseScreen: show success toast, open queue modal
QueueStore->>QueueStore: process queue (max 2 concurrent)
QueueStore->>DownloadService: downloadLmsResourceWithSession(item)
DownloadService-->>QueueStore: onProgress updates (fraction)
QueueStore->>QueueStore: update item progress
DownloadService-->>QueueStore: completed/failed with uri/error
QueueStore->>QueueStore: mark item status, process next pending
QueueModal->>QueueStore: selectQueueStats() for UI
QueueModal->>QueueModal: render progress bar, item list, action buttons
QueueModal->>QueueStore: user taps retry/remove/clearFinished
QueueStore->>QueueStore: update items accordingly
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Preview |
There was a problem hiding this comment.
Pull request overview
Adds two user-facing features to the app: a dashboard “Forum Activity” feed sourced from Moodle’s AJAX forum APIs, and a concurrency-limited batch download queue for course resources.
Changes:
- Introduces forum types, a forum service, a persisted forum store, and a new dashboard forum section.
- Adds a download queue store with a 2-concurrent processor plus a full-screen queue modal UI.
- Integrates “Download All” + queue progress badge into the course resources screen and wires a deferred forum fetch into the dashboard.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/types/index.ts | Re-exports new forum and download-queue types. |
| src/types/forum.ts | Adds Moodle forum/discussion state/type definitions. |
| src/types/download-queue.ts | Adds batch download queue item/state types. |
| src/stores/forum-store.ts | Adds persisted forum store with staleness logic + selector. |
| src/stores/download-queue-store.ts | Implements queue state/actions and concurrency-limited processor. |
| src/stores/auth-store.ts | Clears forum store on logout (new behavior). |
| src/services/forum.ts | Adds Moodle AJAX calls for forums/discussions aggregation. |
| src/components/dashboard/forum-section.tsx | Renders recent forum discussions on dashboard. |
| src/components/download/download-queue-modal.tsx | Full-screen modal UI for queue items + progress. |
| src/app/(tabs)/index.tsx | Dashboard integration + deferred forum fetch. |
| src/app/course/[courseid].tsx | Adds “Download All”, queue badge, and queue modal integration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+196
to
+203
| if (!hasCompletedInitialRefresh.current) return; | ||
| hasDeferredForumFetch.current = true; | ||
|
|
||
| const task = InteractionManager.runAfterInteractions(() => { | ||
| void fetchForumDiscussions({ silent: true }); | ||
| }); | ||
|
|
||
| return () => task.cancel(); |
There was a problem hiding this comment.
The deferred forum fetch effect relies on hasCompletedInitialRefresh.current, but that ref changing to true does not trigger this useEffect to rerun (it’s not a dependency). If hasHydrated is already true when the initial refresh completes, this effect can exit early and never fetch forum discussions. Consider triggering the forum fetch directly when the initial refresh finishes (same async block where the ref is set), or replace the ref with state (e.g., initialRefreshCompleted) and include it in the dependency list.
Suggested change
| if (!hasCompletedInitialRefresh.current) return; | |
| hasDeferredForumFetch.current = true; | |
| const task = InteractionManager.runAfterInteractions(() => { | |
| void fetchForumDiscussions({ silent: true }); | |
| }); | |
| return () => task.cancel(); | |
| let timeoutId: ReturnType<typeof setTimeout> | null = null; | |
| let task: ReturnType< | |
| typeof InteractionManager.runAfterInteractions | |
| > | null = null; | |
| let cancelled = false; | |
| const scheduleForumFetch = () => { | |
| if (cancelled || hasDeferredForumFetch.current) return; | |
| if (!hasCompletedInitialRefresh.current) { | |
| timeoutId = setTimeout(scheduleForumFetch, 250); | |
| return; | |
| } | |
| hasDeferredForumFetch.current = true; | |
| task = InteractionManager.runAfterInteractions(() => { | |
| void fetchForumDiscussions({ silent: true }); | |
| }); | |
| }; | |
| scheduleForumFetch(); | |
| return () => { | |
| cancelled = true; | |
| task?.cancel(); | |
| if (timeoutId) { | |
| clearTimeout(timeoutId); | |
| } | |
| }; |
|
|
||
| // Skip if recently synced | ||
| const { lastSyncTime } = get(); | ||
| if (lastSyncTime && Date.now() - lastSyncTime < FORUM_STALE_MS && !silent) { |
There was a problem hiding this comment.
The staleness guard is bypassed when silent is true (&& !silent). This means background/silent fetches will always hit the network even if the cache is still fresh, which defeats the 15-minute staleness check. Consider applying the staleness check regardless of silent, or introduce an explicit force option if you need a way to bypass staleness intentionally.
Suggested change
| if (lastSyncTime && Date.now() - lastSyncTime < FORUM_STALE_MS && !silent) { | |
| if (lastSyncTime && Date.now() - lastSyncTime < FORUM_STALE_MS) { |
Comment on lines
115
to
121
| useTimetableStore.getState().clearTimetable(); | ||
| useFacultyStore.getState().clearRecentSearches(); | ||
| useLmsResourcesStore.getState().clearCourseResources(); | ||
| useAssignmentStore.getState().clearAssignmentCache(); | ||
| useForumStore.getState().clearForum(); | ||
| useAttendanceUIStore.getState().resetUI(); | ||
|
|
There was a problem hiding this comment.
Logout clears several user-scoped stores, but the new download queue store is not cleared. Since queue items include course names and file names, leaving them in memory after logout can leak data across accounts in the same app session. Consider clearing the download queue (e.g., useDownloadQueueStore.getState().clearAll()) alongside the other clear* calls here.
| ForumDiscussionWithCourse, | ||
| ForumInfo, | ||
| MoodleAjaxRequest, | ||
| MoodleAjaxResponse, |
There was a problem hiding this comment.
MoodleAjaxResponse is imported but never used in this module. Please remove the unused import (or use it for the response typing) to keep the file lint-clean and reduce noise.
Suggested change
| MoodleAjaxResponse, |
| const retry = useDownloadQueueStore((state) => state.retry); | ||
| const remove = useDownloadQueueStore((state) => state.remove); | ||
| const clearFinished = useDownloadQueueStore((state) => state.clearFinished); | ||
| const clearAll = useDownloadQueueStore((state) => state.clearAll); |
There was a problem hiding this comment.
clearAll is read from the store but never used. Either remove this unused selector or add UI that exposes the "Clear all" behavior so the modal matches the intended actions.
Suggested change
| const clearAll = useDownloadQueueStore((state) => state.clearAll); |
Comment on lines
+4
to
+8
| DownloadQueueItemStatus, | ||
| DownloadQueueState, | ||
| LmsCourseResourcesTree, | ||
| LmsResourceFileNode, | ||
| LmsResourceItemNode, |
There was a problem hiding this comment.
Several imported types here are unused (DownloadQueueItemStatus, LmsResourceFileNode, LmsResourceItemNode). Please remove the unused imports (or use them) to keep the store file lint-clean.
Suggested change
| DownloadQueueItemStatus, | |
| DownloadQueueState, | |
| LmsCourseResourcesTree, | |
| LmsResourceFileNode, | |
| LmsResourceItemNode, | |
| DownloadQueueState, | |
| LmsCourseResourcesTree, |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stores/auth-store.ts (1)
110-120:⚠️ Potential issue | 🟠 MajorCall
useDownloadQueueStore.getState().clearAll()in logout.The download queue is not cleared when logging out. Queue items carry user-scoped metadata (course names, URLs, local file URIs), so the queue will survive a logout/login cycle in the same JS session and expose the previous account's downloads. Add the queue reset here with the other store cleanups (line 110–120).
Store method available
clearAll: () => { set({ items: [] }); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/auth-store.ts` around lines 110 - 120, Add a call to useDownloadQueueStore.getState().clearAll() in the logout cleanup sequence so the download queue is reset with the other stores; locate the logout/cleanup block that currently calls useAttendanceStore.getState().clearAttendance(), useBunkStore.getState().clearBunks(), dashboardState.clearDashboard(), etc., and insert useDownloadQueueStore.getState().clearAll() alongside those calls to ensure per-user download metadata is removed on logout.
🧹 Nitpick comments (7)
src/app/(tabs)/index.tsx (1)
3-3: Prefer the dashboard barrel for this new tab component.Re-export
ForumSectionfrom@/components/dashboardand import it from there so this tab keeps the shared entrypoint pattern.As per coding guidelines: For tab-specific components, import directly from the tab's directory (e.g.,
@/components/dashboard).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/app/`(tabs)/index.tsx at line 3, The file imports ForumSection directly from "@/components/dashboard/forum-section"; instead re-export ForumSection from the dashboard barrel and update the import to use "@/components/dashboard". Add an export entry like `export { ForumSection } from "./forum-section";` to the dashboard index barrel (components/dashboard index) if it doesn't exist, then change the import in the tab component (index.tsx) to import { ForumSection } from "@/components/dashboard".src/stores/forum-store.ts (2)
82-84: Add null check inonRehydrateStoragecallback.If rehydration fails,
statecould beundefined. The optional chaining handlessetHasHydrated, but this is a good place to add defensive logging.🛡️ Suggested fix
onRehydrateStorage: () => (state) => { - state?.setHasHydrated(true); + if (state) { + state.setHasHydrated(true); + } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/forum-store.ts` around lines 82 - 84, The onRehydrateStorage callback should defensively handle a possible undefined state: inside the onRehydrateStorage handler (the function assigned to onRehydrateStorage) check if state is falsy and, if so, emit a simple warning log (or use console.warn) and return early; otherwise call state.setHasHydrated(true). Update the handler that currently does state?.setHasHydrated(true) to perform this explicit null check and logging so rehydration failures are visible (referencing onRehydrateStorage and setHasHydrated).
45-63: Consider adding debug logging for forum fetch operations.Per coding guidelines, use
debug.api()for API-related operations to aid debugging.♻️ Add debug logging
+import { debug } from "@/utils/debug"; ... try { const discussions = await fetchAllForumDiscussions(); + debug.api("Forum discussions fetched", { count: discussions.length }); set({ discussions, ... } catch (error) { const message = error instanceof Error ? error.message : "Failed to fetch forum discussions"; + debug.api("Forum fetch failed", { error: message }); if (!silent) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/forum-store.ts` around lines 45 - 63, Add debug.api logging around the forum fetch flow: call debug.api(...) before calling fetchAllForumDiscussions() to record the fetch start (include any relevant context such as silent flag), call debug.api(...) after a successful fetch to log the number of discussions and timestamp (use discussions.length and Date.now()), and call debug.api(...) inside the catch to log the full error object along with the message you already set. Target the try/catch block that calls fetchAllForumDiscussions() and updates state via set(...) so logs sit immediately before the fetch, after set(...) on success, and inside the catch where you currently compute message and call set(...).src/components/download/download-queue-modal.tsx (2)
160-166: Unused store selectorclearAll.
clearAllis destructured from the store but never used in this component. If "Clear All" functionality is intended for a future iteration, consider removing it to keep the code clean.♻️ Remove unused import
const retry = useDownloadQueueStore((state) => state.retry); const remove = useDownloadQueueStore((state) => state.remove); const clearFinished = useDownloadQueueStore((state) => state.clearFinished); - const clearAll = useDownloadQueueStore((state) => state.clearAll);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/download/download-queue-modal.tsx` around lines 160 - 166, The component destructures clearAll from the download queue store but never uses it; remove the unused selector to clean up the code by deleting clearAll from the useDownloadQueueStore destructuring (the line with const clearAll = useDownloadQueueStore((state) => state.clearAll)) or, if you intend to add "Clear All" later, wire up clearAll to a button/handler in DownloadQueueModal so the selector is actually used; reference useDownloadQueueStore and clearAll in download-queue-modal.tsx to locate the spot to change.
19-19: Hard-coded background color doesn't respect theme.The
bg-zinc-800class is hard-coded for the progress bar track, which won't adapt to light mode. Consider using the theme'sbackgroundSecondaryor similar for consistency.♻️ Suggested fix
- <View className="h-1.5 overflow-hidden rounded-full bg-zinc-800"> + <View + className="h-1.5 overflow-hidden rounded-full" + style={{ backgroundColor: isDark ? '#27272a' : '#e4e4e7' }} + >Note: This would require passing
isDarkas a prop toProgressBar, or moving the component inside the parent to access theme context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/download/download-queue-modal.tsx` at line 19, The progress bar uses a hard-coded class "bg-zinc-800" which breaks theming; update the ProgressBar (or the View in download-queue-modal.tsx) to use the theme token (e.g., backgroundSecondary) instead of bg-zinc-800 so it respects light/dark modes — either pass an isDark prop into ProgressBar from the parent and switch classes based on that prop, or move ProgressBar into the parent so it can read the theme context directly and apply the appropriate background token.src/stores/download-queue-store.ts (2)
64-74: Potential race condition in download status transition.Between checking
item.status !== "pending"on line 67 and callingsetStateon line 70, another call could modify the item's status (e.g., ifremoveis called). While unlikely in practice, consider using a compare-and-swap pattern.🛡️ Suggested fix with atomic status transition
const downloadItem = async (itemId: string) => { const store = useDownloadQueueStore; - const item = store.getState().items.find((i) => i.id === itemId); - if (!item || item.status !== "pending") return; - - // Mark as downloading - store.setState((state) => ({ - items: state.items.map((i) => - i.id === itemId ? { ...i, status: "downloading" as const, error: null } : i, - ), - })); + + // Atomically transition from pending to downloading + let item: DownloadQueueItem | undefined; + store.setState((state) => { + const found = state.items.find((i) => i.id === itemId); + if (!found || found.status !== "pending") { + item = undefined; + return state; // No change + } + item = found; + return { + items: state.items.map((i) => + i.id === itemId ? { ...i, status: "downloading" as const, error: null } : i, + ), + }; + }); + + if (!item) return;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/download-queue-store.ts` around lines 64 - 74, The code has a race between reading item.status and later calling setState; update downloadItem to perform an atomic compare-and-swap inside the state updater instead of relying on the earlier read: call useDownloadQueueStore.setState with a functional updater that locates the item by id, checks that its current status === "pending", and only then returns a new state with that item's status set to "downloading" and error cleared (otherwise return state unchanged), then re-read the item from useDownloadQueueStore.getState() to decide whether to proceed with the download; reference downloadItem, useDownloadQueueStore, setState, items, and status when making this change.
80-86: Frequent state updates during download progress may impact performance.Every progress tick triggers a full
setStateand potential re-render of subscribed components. Consider throttling progress updates (e.g., every 100ms or 5% change).⚡ Throttle progress updates
+// At module level +const PROGRESS_THROTTLE_MS = 100; +const lastProgressUpdate = new Map<string, number>(); const result = await downloadLmsResourceWithSession( item.url, item.fileName, { onProgress: (progress) => { + const now = Date.now(); + const lastUpdate = lastProgressUpdate.get(itemId) ?? 0; + if (now - lastUpdate < PROGRESS_THROTTLE_MS) return; + lastProgressUpdate.set(itemId, now); + store.setState((state) => ({ items: state.items.map((i) => i.id === itemId ? { ...i, progress: progress.fraction } : i, ), })); }, }, ); + lastProgressUpdate.delete(itemId);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/stores/download-queue-store.ts` around lines 80 - 86, Throttled progress updates: avoid calling store.setState on every onProgress tick (the onProgress handler currently updating items by id using progress.fraction) — implement throttling or change detection inside the onProgress handler so updates only occur at most every ~100ms or when progress.fraction changes by a threshold (e.g., 5%); modify the onProgress callback to track lastUpdateTime/lastFraction and only call store.setState((state) => ({ items: state.items.map(...) })) when the time or fraction threshold is met, referencing the existing onProgress, store.setState, items, progress.fraction, and itemId identifiers.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/`(tabs)/index.tsx:
- Around line 192-204: The effect that defers fetchForumDiscussions can never
run after initial refresh because it reads and returns based on
hasCompletedInitialRefresh.current (a ref) but does not depend on its change;
update the useEffect to include hasCompletedInitialRefresh (e.g., a derived
boolean from the ref or a state flag) in its dependency array or trigger the
effect when the refresh completes, and move the hasDeferredForumFetch.current =
true assignment inside the InteractionManager.runAfterInteractions callback
(only after the task actually starts) so cancellations don't permanently flip
the flag; keep fetchForumDiscussions and the existing guards (hasHydrated,
isOffline) but ensure the returned cleanup cancels the scheduled task correctly.
In `@src/app/course/`[courseid].tsx:
- Around line 161-170: The UI uses enqueueAllFromCourse(tree) return value
(count) to decide message, but count===0 conflates "no files downloadable" with
"files exist but already queued"; update the logic: either change
enqueueAllFromCourse to return a richer result (e.g., { enqueuedCount,
hasDownloadables }) or add a selector/function like
hasDownloadablesFromCourse(tree) to detect whether the course contains
downloadable resources, then in handleDownloadAll use hasDownloadables to
early-return or hide/disable the download-all button when false, and use
enqueuedCount strictly to decide the "queued X files" vs "all files already in
queue" Toast messaging; reference functions: enqueueAllFromCourse and
handleDownloadAll (and new hasDownloadablesFromCourse or enriched return) when
making the change.
- Around line 157-159: The header stats use a course-scoped filter
(selectQueueStats(queueItems.filter(i => i.courseId === courseId))) but
DownloadQueueModal is currently global; update the DownloadQueueModal invocation
to accept a courseId prop and then filter its displayed items internally (use
the passed courseId to filter queueItems inside DownloadQueueModal before render
and for retry/remove actions). Ensure you update the DownloadQueueModal
props/type to include courseId and adjust any handlers (retry/remove) to only
operate on items matching that courseId; alternatively, if you prefer a global
modal, remove the local filter from selectQueueStats so both badge and modal use
the same unfiltered queueItems.
In `@src/stores/download-queue-store.ts`:
- Around line 246-248: clearAll currently just empties items but doesn't stop
in-progress downloads; update the download flow to support cancellation by
creating and storing an AbortController per item (e.g., maintain a controllers
Map keyed by item id), pass its signal into downloadItem so fetch/async work can
be aborted and have downloadItem bail out without calling setState when aborted,
then have clearAll iterate controllers to call abort() and clear the controllers
Map before calling set({ items: [] }); also add a guard in
downloadItem/processQueue to check the item still exists before calling
setState; alternatively, if you prefer not to implement cancellation, add clear
documentation in the clearAll JSDoc explaining that active downloads are not
canceled and may complete after items are cleared.
---
Outside diff comments:
In `@src/stores/auth-store.ts`:
- Around line 110-120: Add a call to useDownloadQueueStore.getState().clearAll()
in the logout cleanup sequence so the download queue is reset with the other
stores; locate the logout/cleanup block that currently calls
useAttendanceStore.getState().clearAttendance(),
useBunkStore.getState().clearBunks(), dashboardState.clearDashboard(), etc., and
insert useDownloadQueueStore.getState().clearAll() alongside those calls to
ensure per-user download metadata is removed on logout.
---
Nitpick comments:
In `@src/app/`(tabs)/index.tsx:
- Line 3: The file imports ForumSection directly from
"@/components/dashboard/forum-section"; instead re-export ForumSection from the
dashboard barrel and update the import to use "@/components/dashboard". Add an
export entry like `export { ForumSection } from "./forum-section";` to the
dashboard index barrel (components/dashboard index) if it doesn't exist, then
change the import in the tab component (index.tsx) to import { ForumSection }
from "@/components/dashboard".
In `@src/components/download/download-queue-modal.tsx`:
- Around line 160-166: The component destructures clearAll from the download
queue store but never uses it; remove the unused selector to clean up the code
by deleting clearAll from the useDownloadQueueStore destructuring (the line with
const clearAll = useDownloadQueueStore((state) => state.clearAll)) or, if you
intend to add "Clear All" later, wire up clearAll to a button/handler in
DownloadQueueModal so the selector is actually used; reference
useDownloadQueueStore and clearAll in download-queue-modal.tsx to locate the
spot to change.
- Line 19: The progress bar uses a hard-coded class "bg-zinc-800" which breaks
theming; update the ProgressBar (or the View in download-queue-modal.tsx) to use
the theme token (e.g., backgroundSecondary) instead of bg-zinc-800 so it
respects light/dark modes — either pass an isDark prop into ProgressBar from the
parent and switch classes based on that prop, or move ProgressBar into the
parent so it can read the theme context directly and apply the appropriate
background token.
In `@src/stores/download-queue-store.ts`:
- Around line 64-74: The code has a race between reading item.status and later
calling setState; update downloadItem to perform an atomic compare-and-swap
inside the state updater instead of relying on the earlier read: call
useDownloadQueueStore.setState with a functional updater that locates the item
by id, checks that its current status === "pending", and only then returns a new
state with that item's status set to "downloading" and error cleared (otherwise
return state unchanged), then re-read the item from
useDownloadQueueStore.getState() to decide whether to proceed with the download;
reference downloadItem, useDownloadQueueStore, setState, items, and status when
making this change.
- Around line 80-86: Throttled progress updates: avoid calling store.setState on
every onProgress tick (the onProgress handler currently updating items by id
using progress.fraction) — implement throttling or change detection inside the
onProgress handler so updates only occur at most every ~100ms or when
progress.fraction changes by a threshold (e.g., 5%); modify the onProgress
callback to track lastUpdateTime/lastFraction and only call
store.setState((state) => ({ items: state.items.map(...) })) when the time or
fraction threshold is met, referencing the existing onProgress, store.setState,
items, progress.fraction, and itemId identifiers.
In `@src/stores/forum-store.ts`:
- Around line 82-84: The onRehydrateStorage callback should defensively handle a
possible undefined state: inside the onRehydrateStorage handler (the function
assigned to onRehydrateStorage) check if state is falsy and, if so, emit a
simple warning log (or use console.warn) and return early; otherwise call
state.setHasHydrated(true). Update the handler that currently does
state?.setHasHydrated(true) to perform this explicit null check and logging so
rehydration failures are visible (referencing onRehydrateStorage and
setHasHydrated).
- Around line 45-63: Add debug.api logging around the forum fetch flow: call
debug.api(...) before calling fetchAllForumDiscussions() to record the fetch
start (include any relevant context such as silent flag), call debug.api(...)
after a successful fetch to log the number of discussions and timestamp (use
discussions.length and Date.now()), and call debug.api(...) inside the catch to
log the full error object along with the message you already set. Target the
try/catch block that calls fetchAllForumDiscussions() and updates state via
set(...) so logs sit immediately before the fetch, after set(...) on success,
and inside the catch where you currently compute message and call set(...).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ccb43828-547b-4d9a-b2ec-967714082192
📒 Files selected for processing (11)
src/app/(tabs)/index.tsxsrc/app/course/[courseid].tsxsrc/components/dashboard/forum-section.tsxsrc/components/download/download-queue-modal.tsxsrc/services/forum.tssrc/stores/auth-store.tssrc/stores/download-queue-store.tssrc/stores/forum-store.tssrc/types/download-queue.tssrc/types/forum.tssrc/types/index.ts
Comment on lines
+192
to
+204
| // Deferred forum discussions fetch | ||
| useEffect(() => { | ||
| if (!hasHydrated || isOffline) return; | ||
| if (hasDeferredForumFetch.current) return; | ||
| if (!hasCompletedInitialRefresh.current) return; | ||
| hasDeferredForumFetch.current = true; | ||
|
|
||
| const task = InteractionManager.runAfterInteractions(() => { | ||
| void fetchForumDiscussions({ silent: true }); | ||
| }); | ||
|
|
||
| return () => task.cancel(); | ||
| }, [fetchForumDiscussions, hasHydrated, isOffline]); |
There was a problem hiding this comment.
This effect can miss the deferred forum fetch entirely.
hasCompletedInitialRefresh.current is mutated inside the dashboard refresh callbacks, but it is not a dependency here. Because ref writes do not retrigger effects, the first pass returns on Line 196 and never reruns when the initial refresh completes. hasDeferredForumFetch.current is also flipped before the InteractionManager task executes, so a cancellation/failure path suppresses later retries too.
💡 One deterministic pattern
- const hasCompletedInitialRefresh = useRef(false);
- const hasDeferredForumFetch = useRef(false);
+ const [hasCompletedInitialRefresh, setHasCompletedInitialRefresh] =
+ useState(false);
...
- hasCompletedInitialRefresh.current = true;
+ setHasCompletedInitialRefresh(true);
...
- hasCompletedInitialRefresh.current = true;
+ setHasCompletedInitialRefresh(true);
...
useEffect(() => {
- if (!hasHydrated || isOffline) return;
- if (hasDeferredForumFetch.current) return;
- if (!hasCompletedInitialRefresh.current) return;
- hasDeferredForumFetch.current = true;
+ if (!hasHydrated || isOffline || !hasCompletedInitialRefresh) return;
const task = InteractionManager.runAfterInteractions(() => {
void fetchForumDiscussions({ silent: true });
});
return () => task.cancel();
- }, [fetchForumDiscussions, hasHydrated, isOffline]);
+ }, [
+ fetchForumDiscussions,
+ hasCompletedInitialRefresh,
+ hasHydrated,
+ isOffline,
+ ]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/`(tabs)/index.tsx around lines 192 - 204, The effect that defers
fetchForumDiscussions can never run after initial refresh because it reads and
returns based on hasCompletedInitialRefresh.current (a ref) but does not depend
on its change; update the useEffect to include hasCompletedInitialRefresh (e.g.,
a derived boolean from the ref or a state flag) in its dependency array or
trigger the effect when the refresh completes, and move the
hasDeferredForumFetch.current = true assignment inside the
InteractionManager.runAfterInteractions callback (only after the task actually
starts) so cancellations don't permanently flip the flag; keep
fetchForumDiscussions and the existing guards (hasHydrated, isOffline) but
ensure the returned cleanup cancels the scheduled task correctly.
Comment on lines
+157
to
+159
| const queueStats = selectQueueStats( | ||
| queueItems.filter((i) => i.courseId === courseId), | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
modal_file="$(fd -i 'download-queue-modal\.tsx$' src/components/download | head -n1)"
if [ -z "$modal_file" ]; then
echo "download-queue-modal.tsx not found" >&2
exit 1
fi
echo "== $modal_file =="
sed -n '1,320p' "$modal_file"
echo
echo "== course filtering signals =="
rg -n -C2 'courseId|useDownloadQueueStore|filter\(|items\b' "$modal_file" 'src/app/course/[courseid].tsx'Repository: Noelithub77/bunkialo2
Length of output: 30525
Pass courseId to DownloadQueueModal and filter items internally, or make both badge and modal global.
The header stats are scoped to the current course (queueItems.filter((i) => i.courseId === courseId)), but the modal opened from that button has no course awareness. It displays and allows retry/remove on queue items from all courses. Either pass the current courseId into the modal and filter its item list, or remove the filter from the badge/stats so both reflect the full queue.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/course/`[courseid].tsx around lines 157 - 159, The header stats use a
course-scoped filter (selectQueueStats(queueItems.filter(i => i.courseId ===
courseId))) but DownloadQueueModal is currently global; update the
DownloadQueueModal invocation to accept a courseId prop and then filter its
displayed items internally (use the passed courseId to filter queueItems inside
DownloadQueueModal before render and for retry/remove actions). Ensure you
update the DownloadQueueModal props/type to include courseId and adjust any
handlers (retry/remove) to only operate on items matching that courseId;
alternatively, if you prefer a global modal, remove the local filter from
selectQueueStats so both badge and modal use the same unfiltered queueItems.
Comment on lines
+161
to
+170
| const handleDownloadAll = () => { | ||
| if (!tree) return; | ||
| const count = enqueueAllFromCourse(tree); | ||
| if (count > 0) { | ||
| Toast.show(`Queued ${count} file${count === 1 ? "" : "s"} for download`, { | ||
| type: "success", | ||
| }); | ||
| setShowDownloadQueue(true); | ||
| } else { | ||
| Toast.show("All files already in queue", { type: "info" }); |
There was a problem hiding this comment.
count === 0 is ambiguous here.
enqueueAllFromCourse() returns the number of newly enqueued items, not whether the course had downloadable files at all. On a course with zero downloadable resources, this branch still tells the user "All files already in queue", which is misleading. Hide the button when there are no downloadables, or have the store return a separate status for that case.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/course/`[courseid].tsx around lines 161 - 170, The UI uses
enqueueAllFromCourse(tree) return value (count) to decide message, but count===0
conflates "no files downloadable" with "files exist but already queued"; update
the logic: either change enqueueAllFromCourse to return a richer result (e.g., {
enqueuedCount, hasDownloadables }) or add a selector/function like
hasDownloadablesFromCourse(tree) to detect whether the course contains
downloadable resources, then in handleDownloadAll use hasDownloadables to
early-return or hide/disable the download-all button when false, and use
enqueuedCount strictly to decide the "queued X files" vs "all files already in
queue" Toast messaging; reference functions: enqueueAllFromCourse and
handleDownloadAll (and new hasDownloadablesFromCourse or enriched return) when
making the change.
Comment on lines
+246
to
+248
| clearAll: () => { | ||
| set({ items: [] }); | ||
| }, |
There was a problem hiding this comment.
clearAll doesn't cancel in-progress downloads.
Calling clearAll while downloads are active will clear the items array, but the downloadItem async functions will continue running. When they complete, they'll call setState with item IDs that no longer exist (harmless but wasteful), and then processQueue which will find nothing to do.
Consider whether active downloads should be abortable, or at minimum document this behavior.
📝 Document behavior or add cancellation
clearAll: () => {
+ // Note: Does not cancel in-progress downloads; they will complete
+ // but their results will be discarded since items are cleared.
set({ items: [] });
},For full cancellation support, you'd need to track AbortController instances per download.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/stores/download-queue-store.ts` around lines 246 - 248, clearAll
currently just empties items but doesn't stop in-progress downloads; update the
download flow to support cancellation by creating and storing an AbortController
per item (e.g., maintain a controllers Map keyed by item id), pass its signal
into downloadItem so fetch/async work can be aborted and have downloadItem bail
out without calling setState when aborted, then have clearAll iterate
controllers to call abort() and clear the controllers Map before calling set({
items: [] }); also add a guard in downloadItem/processQueue to check the item
still exists before calling setState; alternatively, if you prefer not to
implement cancellation, add clear documentation in the clearAll JSDoc explaining
that active downloads are not canceled and may complete after items are cleared.
Noelithub77
requested changes
Apr 9, 2026
Owner
Noelithub77
left a comment
Noelithub77
left a comment
There was a problem hiding this comment.
Forums are kinda unnecessary and like a bloat, but we can merge the batch download, can you make it a separate pr. Also please attach screenshot
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two new features:
1. Forum Monitor
mod_forum_get_forums_by_courses,mod_forum_get_forum_discussions)2. Batch Download Queue
resourceandfolderfile URLsdownloadLmsResourceWithSessionservicecompleted/total)New files
src/types/forum.ts— forum/discussion typessrc/types/download-queue.ts— queue item typessrc/services/forum.ts— Moodle forum AJAX callssrc/stores/forum-store.ts— forum state managementsrc/stores/download-queue-store.ts— download queue with concurrency controlsrc/components/dashboard/forum-section.tsx— dashboard forum cardsrc/components/download/download-queue-modal.tsx— queue UI modalModified files
src/app/(tabs)/index.tsx— integrated forum section + deferred fetchsrc/app/course/[courseid].tsx— added download all button, queue badge, queue modalsrc/stores/auth-store.ts— clear forum store on logoutsrc/types/index.ts— export new types+1,179 lines. TypeScript compiles clean. No changes to existing service logic.
Test plan