Skip to content

Conversation

@odeimaiz
Copy link
Member

@odeimaiz odeimaiz commented May 8, 2025

What do these changes do?

All templates were loaded at start up, adding some extra calls (/templates and their /services) that were not necessary, except for the TI products that do require the templates for the New Plans button.

This PR improves that logic and should speed up the start up process and relieve the webserver a bit. This change will be more noticeable in those deployments that have many templates.

S4L:
templates_S4L

osparc:
templates_osparc

tip:
templates_tip

Related issue/s

How to test

Dev-ops

@odeimaiz odeimaiz self-assigned this May 8, 2025
@odeimaiz odeimaiz added t:enhancement Improvement or request on an existing feature a:frontend issue affecting the front-end (area group) labels May 8, 2025
@odeimaiz odeimaiz added this to the Bazinga! milestone May 8, 2025
@odeimaiz odeimaiz marked this pull request as ready for review May 9, 2025 11:01
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 refactors the way templates are loaded by delaying their fetching until needed and converting the Templates store from a singleton to a static class. Key changes include:

  • Modifying the Templates store to use static methods with caching and promise‐based fetching.
  • Updating UI components (TemplatesList, TemplateBrowser, NewStudies, NewPlusMenu, etc.) to work asynchronously with the new Templates API.
  • Minor adjustments in test files and other resource browsers to incorporate lazy loading and improved asynchronous handling.

Reviewed Changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

File Description
tests/e2e/tests/startupCalls.js Commented out tests for Tasks and Templates were adjusted.
services/static-webserver/client/source/class/osparc/study/Conversations.js Returned the promise from the fetch call for addMessage.
services/static-webserver/client/source/class/osparc/store/Templates.js Changed the class design from a singleton to a static class with promise‐based fetching and caching.
(Other UI components) Updated calls to Templates methods to use the new static async API.
Comments suppressed due to low confidence (3)

tests/e2e/tests/startupCalls.js:68

  • [nitpick] There are tests for Tasks and Templates that have been commented out. If these tests are no longer needed, consider removing them or adding a clarifying comment so future maintainers understand the intent.
-      /*

services/static-webserver/client/source/class/osparc/store/Templates.js:18

  • Converting the Templates class from a singleton to a static class changes how 'this' is used inside its methods. Ensure that all static methods correctly reference and update class-level variables (such as __templates and __templatesPromisesCached) as intended in the framework.
qx.Class.define("osparc.store.Templates", { type: "static",

services/static-webserver/client/source/class/osparc/study/Conversations.js:93

  • The added return statement now ensures the fetch promise is propagated; please confirm that all its callers are updated to handle the asynchronous result appropriately.
return osparc.data.Resources.fetch("conversations", "addMessage", params)

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

odeimaiz commented May 9, 2025

@Mergifyio queue

@mergify
Copy link
Contributor

mergify bot commented May 9, 2025

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 5a1666d

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
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.

👍

Copy link
Collaborator

@matusdrobuliak66 matusdrobuliak66 left a comment

Choose a reason for hiding this comment

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

👍

@sonarqubecloud
Copy link

@mergify mergify bot merged commit 5a1666d into ITISFoundation:master May 12, 2025
59 checks passed
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Jun 6, 2025
92 tasks
@matusdrobuliak66 matusdrobuliak66 mentioned this pull request Aug 5, 2025
88 tasks
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.

4 participants