Skip to content

Refactor/project manager#7754

Merged
walterbender merged 3 commits into
sugarlabs:masterfrom
vanshika2720:refactor/project-manager
Jul 7, 2026
Merged

Refactor/project manager#7754
walterbender merged 3 commits into
sugarlabs:masterfrom
vanshika2720:refactor/project-manager

Conversation

@vanshika2720

@vanshika2720 vanshika2720 commented Jul 7, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Extracts project loading, saving, import, session restore, and initialization logic from js/activity.js into a new js/project-manager.js module
  • The new module exposes setupProjectManager(activity) which creates a ProjectManager instance wired to the activity — no new globals added
  • All extracted functions in activity.js are replaced with thin delegate wrappers that forward to this.projectManager, preserving the existing public API

What was moved

  • doLoadAnimation / stopLoadAnimation / showContents — loading animation lifecycle
  • _loadStart / _loadProject / loadStartWrapper / justLoadStart — load orchestration
  • doLoad / doMergeLoad / _afterDelete / newProject — UI-triggered load/new operations
  • prepareExport / saveLocally / __saveLocally — save/export logic
  • runProject — runtime entry point
  • getClosestStandardNoteValue — MIDI note helper
  • File chooser change event handler, __handleFileSelect, __handleDragOver — file/drag-drop import
  • Startup URL parameter parsing + initial load — consolidated into projectManager.start()
  • midiImportBlocks — MIDI import modal

No behavioral changes

This is a pure extraction refactor. All existing functionality, event sequencing (including the trash handshake), session restore behavior, merge logic, and planet integration are preserved exactly.

Test plan

  • All 5902 existing tests pass after refactoring
  • activity_startup_recovery.test.js updated to test ProjectManager._loadStart, runProject, and _loadProject directly from project-manager.js
  • activity_toolbar_integration.test.js updated to mock setupProjectManager in vm sandbox
  • Prettier run on modified files only
  • DCO sign-off included
  • Documentation

Refactoring Notes

  • Pure extraction only; no functional changes.
  • Existing public APIs remain unchanged.
  • activity.js now delegates project lifecycle operations to ProjectManager.
  • setupProjectManager(activity) uses dependency injection to avoid introducing globals.

@github-actions github-actions Bot added documentation Updates to docs, comments, or README size/XXL XXL: 1000+ lines changed area/javascript Changes to JS source files area/css Changes to CSS/SASS style files area/tests Changes to test files area/i18n Changes to localization files labels Jul 7, 2026
@vanshika2720 vanshika2720 force-pushed the refactor/project-manager branch from 0089c7f to 5972f7b Compare July 7, 2026 11:09
@github-actions github-actions Bot removed area/css Changes to CSS/SASS style files area/i18n Changes to localization files labels Jul 7, 2026
@codecov

codecov Bot commented Jul 7, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 69.96269% with 161 lines in your changes missing coverage. Please review.
✅ Project coverage is 57.43%. Comparing base (338362b) to head (97f4f6c).

Files with missing lines Patch % Lines
js/project-manager.js 72.39% 143 Missing ⚠️
js/activity.js 0.00% 18 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7754      +/-   ##
==========================================
+ Coverage   56.80%   57.43%   +0.62%     
==========================================
  Files         172      173       +1     
  Lines       57683    57707      +24     
==========================================
+ Hits        32767    33142     +375     
+ Misses      24916    24565     -351     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…anager

Move project loading, saving, import, session restore, and initialization
logic from activity.js into a new js/project-manager.js module. The module
exposes a clean public API via setupProjectManager(activity) and delegates
all calls from activity to the new ProjectManager class. No behavioral
changes; all existing functionality is preserved.

What was moved
- doLoadAnimation / stopLoadAnimation — loading animation lifecycle
- _loadStart / _loadProject / loadStartWrapper / justLoadStart — orchestration
- doLoad / doMergeLoad / _afterDelete / newProject — UI-triggered operations
- prepareExport / saveLocally — save/export logic
- runProject / getClosestStandardNoteValue — runtime helpers
- File chooser, drag-drop handlers — file/drag-drop import
- _midiImportBlocks — MIDI import modal
- start() — URL parameter parsing + initial load

loader.js: add "project-manager" to the activity/activity shim deps list
and paths map. Without these entries RequireJS resolved the module to
./project-manager.js (404), crashing startup before Activity() could run.

Updated activity_startup_recovery.test.js and activity_toolbar_integration
.test.js to exercise the extracted code directly from project-manager.js.

Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
Add 105 tests covering all public methods of the extracted ProjectManager
class, raising statement coverage from 0% to 72.79% and clearing the
Codecov patch coverage gate (≥56.80%).

Coverage highlights
- doLoadAnimation: interval, counter wrap at 9 ticks
- _loadStart: no session, existing session, bad session key removal,
  textMsg error recovery
- _loadProject: 2500ms timeout body, textMsg error, missing planet
  methods, __functionload branch (run:true / firstRun:true)
- prepareExport: value blocks (chord, note, default), named blocks
  (start, temperament1, nopValueBlock, matrixData, wrapmode), null entry
- saveLocally: happy path, allProjects setter error, thumbnail via
  Image.onload, canvas getContext error
- _midiImportBlocks: confirm button presence, cancel removes modal
- _setupFileHandlers: click clears value, change handler (null file,
  MIDI, read error, no file selected)
- start(): single-param id URL, multi-param id URL, unknown param,
  callback scheduling; window.history.replaceState used instead of
  delete window.location (non-configurable in jsdom 26)

Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
@vanshika2720 vanshika2720 force-pushed the refactor/project-manager branch from c176444 to 4f77131 Compare July 7, 2026 17:17
Replace four tests that asserted pubsub.on/off was called (implementation
details) with behavior-based equivalents:

- _loadStart: checks keyboardEnableFlag returns to 1 after finishedLoading
- _loadStart: emits event twice, verifies stage.update not called twice
- _loadProject: checks _toggleCollapsibleStacks, _changeBlockVisibility,
  and firstRun=false after event fires (instead of spying on pubsub.on)
- _loadProject: emits event twice, verifies no double-invocation
- runProject: pubsub.off there is a defensive no-op so the spy test is
  replaced with a direct assertion that the 5s work fires once per call

Behavior tests survive implementation refactors; spy-on-internal-call
tests do not.

Signed-off-by: Vanshika <pahalvanshikaa@gmail.com>
@vanshika2720 vanshika2720 marked this pull request as ready for review July 7, 2026 18:01
@Ashutoshx7

Copy link
Copy Markdown
Member

great work @vanshika2720
pulled and check it locally

@walterbender walterbender merged commit 170bac9 into sugarlabs:master Jul 7, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/javascript Changes to JS source files area/tests Changes to test files documentation Updates to docs, comments, or README size/XXL XXL: 1000+ lines changed

Projects

Development

Successfully merging this pull request may close these issues.

3 participants