Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable connector templates per default #4474

Merged
merged 7 commits into from
Aug 26, 2024
Merged

Conversation

nikku
Copy link
Member

@nikku nikku commented Aug 21, 2024

Proposed Changes

Implements #4455, specifically:

  • Adds a way to hook up with activeTab.changed in the app
  • Updates templates on switch to a Camunda 8 BPMN tab, tries every 24 hours
    • Periodic checks are needed on MacOS where the app never really stops
  • enable-connector-templates=true is the default
  • Change is test covered on the client
  • Change is test covered on the backend (app)
  • Updates what's new communication

Closes #4455
Related to camunda/camunda-docs#4187

Checklist

To ensure you provided everything we need to look at your PR:

  • Brief textual description of the changes present
  • Visual demo attached
  • Steps to try out present, i.e. using the @bpmn-io/sr tool
  • Related issue linked via Closes {LINK_TO_ISSUE} or Related to {LINK_TO_ISSUE}

@bpmn-io-tasks bpmn-io-tasks bot added the in progress Currently worked on label Aug 21, 2024
@nikku nikku force-pushed the enable-connector-templates branch from bbd08dc to d29a2e1 Compare August 21, 2024 22:22
@nikku

This comment was marked as outdated.

@nikku

This comment was marked as outdated.

nikku added a commit to camunda/camunda-docs that referenced this pull request Aug 22, 2024
@nikku
Copy link
Member Author

nikku commented Aug 22, 2024

What's new communication, v3:

image

@nikku nikku force-pushed the enable-connector-templates branch 2 times, most recently from 34c5855 to 72bd1cc Compare August 22, 2024 17:57
@nikku nikku marked this pull request as ready for review August 22, 2024 17:58
@bpmn-io-tasks bpmn-io-tasks bot added needs review Review pending and removed in progress Currently worked on labels Aug 22, 2024
@barmac
Copy link
Collaborator

barmac commented Aug 23, 2024

I've tested this and it works fine. There's only one issue on MacOS which was probably introduced with the feature (not this PR). When I close the window while the templates are being fetched, there's an unhandled error in the log:

(node:47611) UnhandledPromiseRejectionWarning: TypeError: Cannot read properties of null (reading 'webContents')
    at Object.send (/Users/maciej/workspace/bpmn-io/camunda-modeler/app/lib/util/renderer.js:68:18)
    at updateConnectorTemplates (/Users/maciej/workspace/bpmn-io/camunda-modeler/app/lib/connector-templates/index.js:71:14)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
(node:47611) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 4)

It's non-critical though.

@nikku nikku changed the title Enable connector templates Enable connector templates per default Aug 23, 2024
@nikku
Copy link
Member Author

nikku commented Aug 23, 2024

#4474 (comment)

Related to this I've seen the issue previously. We should be fail-safe and not fail when sending messages to non-existing main windows.

@nikku nikku force-pushed the enable-connector-templates branch from 29d2c13 to 4e3eaf3 Compare August 23, 2024 17:43
nikku added a commit that referenced this pull request Aug 23, 2024
Ensure we don't fail with an app error once the main window is closed.

Related to #4474 (comment)
@nikku
Copy link
Member Author

nikku commented Aug 23, 2024

With 7dc67f4 we'll now log a warning to the console, and not attempt to send the message.

Copy link
Collaborator

@barmac barmac left a comment

Choose a reason for hiding this comment

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

@nikku
Copy link
Member Author

nikku commented Aug 26, 2024

CC @philippfromme as you worked on this previously.

Ensure we don't fail with an app error once the main window is closed.

Related to #4474 (comment)
@nikku nikku force-pushed the enable-connector-templates branch from 7dc67f4 to d9f7c5b Compare August 26, 2024 07:08
@nikku nikku merged commit 361bbf8 into develop Aug 26, 2024
9 checks passed
@bpmn-io-tasks bpmn-io-tasks bot removed the needs review Review pending label Aug 26, 2024
@nikku nikku deleted the enable-connector-templates branch August 26, 2024 07:08
@github-actions github-actions bot added this to the M80 milestone Aug 26, 2024
nikku added a commit to camunda/camunda-docs that referenced this pull request Aug 26, 2024
* @param {string} userPath
*/
function registerConnectorTemplateUpdater(renderer, userPath) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice! 👏🏻

Copy link
Contributor

@philippfromme philippfromme left a comment

Choose a reason for hiding this comment

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

Good job! 💪🏻

nikku added a commit to camunda/camunda-docs that referenced this pull request Aug 28, 2024
@nikku nikku mentioned this pull request Sep 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify enabling of connectors within modeler
3 participants