Skip to content

Conversation

@odeimaiz
Copy link
Member

@odeimaiz odeimaiz commented Sep 8, 2025

What do these changes do?

This PR simplifies the frontend codebase by removing the dual-path system for handling study document updates. It standardizes on the event-driven patch mechanism regardless of whether Real-Time Collaboration (RTC) is enabled, eliminating the legacy auto-save timer and polling-based approach.

Related issue/s

How to test

Dev-ops

@odeimaiz odeimaiz self-assigned this Sep 8, 2025
@odeimaiz odeimaiz added t:enhancement Improvement or request on an existing feature a:frontend issue affecting the front-end (area group) labels Sep 8, 2025
@odeimaiz odeimaiz added this to the Cheops milestone Sep 8, 2025
@odeimaiz odeimaiz requested a review from Copilot September 8, 2025 13:42

This comment was marked as outdated.

@mergify
Copy link
Contributor

mergify bot commented Sep 8, 2025

🧪 CI Insights

Here's what we observed from your CI run for 7b816fc.

✅ Passed Jobs With Interesting Signals

Pipeline Job Signal Health on master Retries 🔍 CI Insights 📄 Logs
CI system-tests Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View
unit-tests Base branch is broken, but the job passed. Looks like this might be a real fix 💪 Broken 0 View View

@odeimaiz odeimaiz requested a review from Copilot September 8, 2025 14:33
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR simplifies the frontend codebase by removing the dual-path system for handling study document updates. It standardizes on the event-driven patch mechanism regardless of whether Real-Time Collaboration (RTC) is enabled, eliminating the legacy auto-save timer and polling-based approach.

  • Removes the eventDrivenPatch() utility function and its conditional logic throughout the codebase
  • Eliminates auto-save timers and saving timers that were used for the non-event-driven approach
  • Consolidates node change handling to use only the event-driven projectDocumentChanged mechanism

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
osparc/utils/Utils.js Removes the eventDrivenPatch() function that determined which update mechanism to use
osparc/desktop/StudyEditor.js Removes auto-save and saving timer functionality, simplifies initialization logic
osparc/data/model/Workbench.js Removes deprecated deserialization methods and timer-related event listeners
osparc/data/model/Node.js Removes legacy updateStudyDocument event and moves listenToChanges() call
osparc/workbench/WorkbenchUI.js Adds check for auto-start disabled setting in double-tap handler
osparc/node/LifeCycleView.js Replaces legacy event with event-driven patch mechanism
osparc/node/BootOptionsView.js Removes legacy updateStudyDocument event fire

@odeimaiz odeimaiz changed the title 🎨 [Frontend] Use event driven patch regardless RTC enabled or not 🎨 [Frontend] Use event driven patch regardless RTC enabled Sep 8, 2025
@odeimaiz odeimaiz changed the title 🎨 [Frontend] Use event driven patch regardless RTC enabled 🎨 [Frontend] Event-driven patch regardless of whether RTC is enabled Sep 8, 2025
@odeimaiz odeimaiz marked this pull request as ready for review September 8, 2025 14:36
Copy link
Member

@sanderegg sanderegg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

Copy link
Member

@pcrespov pcrespov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@odeimaiz odeimaiz enabled auto-merge (squash) September 8, 2025 15:47
Copy link
Contributor

@bisgaard-itis bisgaard-itis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice cleanup! Thanks 👍🏻

Copy link
Contributor

@GitHK GitHK left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@odeimaiz odeimaiz disabled auto-merge September 9, 2025 06:42
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 9, 2025

@odeimaiz odeimaiz added the 🤖-automerge marks PR as ready to be merged for Mergify label Sep 9, 2025
@odeimaiz
Copy link
Member Author

odeimaiz commented Sep 9, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented Sep 9, 2025

queue

🛑 Configuration not compatible with a branch protection setting

The branch protection setting Require branches to be up to date before merging is not compatible with max_parallel_checks>1, queue_conditions != merge_conditions and must be unset.

@odeimaiz odeimaiz merged commit a98b2fb into ITISFoundation:master Sep 9, 2025
58 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🤖-automerge marks PR as ready to be merged for Mergify a:frontend issue affecting the front-end (area group) t:enhancement Improvement or request on an existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants