Berickson/20260210 dashboard updates

[bradley-erickson]

Various updates to the dashboards based on teacher feedback.

• Need to confirm Schoology api changes before merging

[bradley-erickson]

@pmitros these changes are tested. I just need a reviewer to approve

[pmitros]

If of help, Claude says:

  Issues & concerns

  High severity

  1. course_api has no auth check and linear scans the course list (rosters.py:584-594):
  The new course_api handler calls courselist(request) and iterates to find a matching course. There's no explicit authorization — any authenticated user could probe for arbitrary course_id values. Also, course['id'] vs course_id — one comes
  from match_info (always a string), the other from the course data (could be int). If the course list returns {'id': 12345}, comparing to '12345' will never match. The existing courseroster_api does int(request.match_info['course_id']) but
  course_api does not — these should be consistent.
  2. courseassignments_api doesn't cast course_id (rosters.py:605-610):
  The roster API does int(request.match_info['course_id']) but courseassignments_api passes the raw string. If downstream code expects an int, this will fail silently or raise.
  3. Duplicated utility functions across two JS modules: hashObject, simpleHash, createDashComponent, fetchSelectedItemsFromOptions, createProcessTags, checkForResponse/checkForBulkResponse, buildEmptyState/buildBulkEmptyState, walkthrough
  infrastructure — all of these are copy-pasted between wo_bulk_essay_analysis/assets/scripts.js and wo_classroom_text_highlighter/assets/scripts.js. This is a maintenance hazard. Even a shared liblo.js or similar would help. There was a TODO
   about this that got removed.
  4. Commented-out code left in (scripts.js lines ~620-627 in bulk essay):
  // const DASH_HTML_COMPONENTS = 'dash_html_components';
  // const DASH_CORE_COMPONENTS = 'dash_core_components';
  // ...
  // function createDashComponent (namespace, type, props) { ... }
  4. These commented-out blocks should be removed before merge.

  Medium severity

  5. console.log statements left in production code: Multiple console.log calls in scripts.js for both dashboards (e.g., [sendToLOConnection], [computeAppliedHash], [applyOptionsAndCloseModal], [updateLoadingInformation], [populateOutput],
  [updateCurrentOptionHash]). These are fine for debugging but should be removed or gated behind a debug flag for production.
  6. print() statements in Python (schoology.py:189, schoology.py:207):
  print('Warning: Schoology assignments have not yet been implemented.') — these should use the logging framework (logging.warning(...)) instead of raw print.
  7. Typo in docstrings: schoology.py:186 and schoology.py:204 both say TODO implmement (missing 'e'). Minor, but these are the first thing someone reads.
  8. Bitwise & used instead of logical && (scripts.js:815):
  if (!!replacementId & replacementId !== label) {
  8. This is an existing pattern (savePlaceholder, disableAttachmentSaveButton at line ~746), but since the PR touches these lines it would be good to fix: & is bitwise AND, && is logical AND. It works here by accident because the operands
  are boolean-ish, but it's a footgun.
  9. createStudentCard returns null for missing users (scripts.js:761):
  Changed from return {} to return null, and the caller in updateStudentGridOutput checks if (card) before concat. However, the original contract returned an object — make sure no other callers depend on getting an object back.
  10. Modal backdrop CSS override is global (general.css:25-30):
  .modal-backdrop.show {
      backdrop-filter: blur(20px) saturate(0.5);
      ...
      opacity: 1 !important;
  }
  10. This overrides all Bootstrap modal backdrops site-wide with !important. If any other page or module uses Bootstrap modals, they'll all get this heavy blur effect. Consider scoping this to a wrapper class.
  11. clientside_callback for send_to_loconnection has a commented-out line (bulk scripts.js:406):
  // if (clicks === 0) { return window.dash_clientside.no_update; }
  11. Remove this dead comment.

  Low severity / nits

  12. Missing newline at end of WOStudentTextTile.react.js: The diff shows \ No newline at end of file was introduced.
  13. Walkthrough step content is hardcoded in JS: The walkthrough content (titles, bodies, icons) is embedded as JS string arrays. If this content needs to change frequently or be i18n'd, it might be better as data loaded from the backend —
  but this is fine for now.
  14. showHideHeader callback naming is confusing (scripts.js:877): The function fills an array with show ? 'd-none' : '' — so when show is true, it hides headers (sets d-none). The name/logic seems inverted from what you'd expect. This may
  match the Switch component's semantics but it's worth a comment or rename.
  15. The Schoology course_assignments endpoint changed URL from https://api.schoology.com/v1/sections/{courseId}/assignments to the LTI AGS endpoint. This is correct architecturally (moving to LTI), but the function body now just return []
  with a warning. Callers should handle this gracefully, which they seem to — the frontend hides the Assignment option when the list is empty.
  16. LODocumentSourceSelectorAIO removed the "Apply" button: The old flow was: pick source → click Apply. The new flow is reactive (updates on every change). This is better UX but means every keystroke in the "Text in Title" input triggers a
   store update. Consider debouncing the title text input.

codex says:

  1. High: course lookup in new course_id API route compares IDs with mismatched
     types

  - learning_observer/learning_observer/rosters.py:568 (and surrounding logic
    around course_api route)
  - course_id comes from the URL as a string, while roster course IDs can be
    integers depending on source.
  - if course['id'] != course_id can fail spuriously and return an empty {} even
    when the course exists.
  - Impact: new /webapi/course/{course_id} endpoint may return no data for valid
    courses.

  2. Medium: new assignment-doc-source path is wired but still returns no
     assignment docs for Schoology

  - learning_observer/learning_observer/integrations/schoology.py
    (assignment_results endpoint + clean_course_assignments /
    clean_assigned_docs)
  - The route now exposes /webapi/courseassignments/{course_id}, and the new
    document source selector includes this path, but cleaning functions still
    return empty assignment doc lists.
  - Impact: dashboards can present an assignment option that appears functional
    but yields no results.

  3. Medium: doc source defaults differ between UI state and request payload
     path

  - modules/wo_bulk_essay_analysis/wo_bulk_essay_analysis/dashboard/
    layout.py:314
  - modules/wo_bulk_essay_analysis/wo_bulk_essay_analysis/assets/scripts.js
    (send callback path using doc_source/doc_source_kwargs)
  - modules/wo_classroom_text_highlighter/wo_classroom_text_highlighter/
    dash_dashboard.py:184, 359, 371
  - _applied_doc_src is initialized as {} while selector defaults imply {'src':
    'latest'}; first submission path reads _applied_doc_src directly.
  - Impact: first-run payloads can omit doc_source state unless Apply/Run flow
    has populated it, changing behavior versus prior selector-driven behavior.

  4. Low/Medium: modal close behavior added with close_button=True but no
     explicit close-input callbacks in some new modals

  - modules/wo_classroom_text_highlighter/wo_classroom_text_highlighter/
    dash_dashboard.py
  - modules/wo_bulk_essay_analysis/wo_bulk_essay_analysis/dashboard/layout.py
  - UX risk: added close-button enabled modals may not close deterministically
    under all callback states unless Dash Modal internals handle it in your
    setup.
  - Suggestion: verify close interactions are covered in integration tests/
    manual QA.

  5. Low: dead/unreferenced JS helper introduces maintenance risk

  - modules/wo_classroom_text_highlighter/wo_classroom_text_highlighter/assets/
    scripts.js:125
  - studentHasResponded appears to duplicate/replace existing path but is not
    used.
  - Impact: low; indicates partial refactor and can mislead future maintenance.

[pmitros]

• I did a pass, but I don't understand this codebase well enough to say my review was particularly robust.
• I also passed it through a few LLMs

Do make sure tests pass before merge, please.