Skip to content

Installed node settings hook - #7880

Merged
hardillb merged 14 commits into
cache-catalogue-versionsfrom
installed-node-settings-hook
Jul 27, 2026
Merged

Installed node settings hook#7880
hardillb merged 14 commits into
cache-catalogue-versionsfrom
installed-node-settings-hook

Conversation

@hardillb

Copy link
Copy Markdown
Contributor

fixes #7736

Description

Uses DB hooks to keep latest version of Node-RED nodes list up to date

This PR merges into #7772

Related Issue(s)

#7736

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production
  • Link to Changelog Entry PR, or note why one is not needed.

Labels

  • Includes a DB migration? -> add the area:migration label

@hardillb
hardillb requested a review from Steve-Mcl July 21, 2026 09:02
@hardillb hardillb self-assigned this Jul 21, 2026
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.75000% with 10 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.54%. Comparing base (f6381d8) to head (2794982).

Files with missing lines Patch % Lines
forge/db/models/StorageSettings.js 76.92% 3 Missing ⚠️
forge/ee/lib/bom/tasks/cache-catalogues.js 0.00% 3 Missing ⚠️
forge/db/models/NodeREDNodeVersions.js 77.77% 2 Missing ⚠️
...igrations/20260717-01-add-node-red-node-version.js 80.00% 1 Missing ⚠️
forge/db/models/Project.js 50.00% 1 Missing ⚠️
Additional details and impacted files
@@                     Coverage Diff                      @@
##           cache-catalogue-versions    #7880      +/-   ##
============================================================
- Coverage                     75.55%   75.54%   -0.01%     
============================================================
  Files                           434      436       +2     
  Lines                         23064    23096      +32     
  Branches                       6112     6115       +3     
============================================================
+ Hits                          17425    17447      +22     
- Misses                         5639     5649      +10     
Flag Coverage Δ
backend 75.54% <68.75%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@Steve-Mcl

Steve-Mcl commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

I see I was tagged as reviewer for this but Is it ready for review Ben? Just from a very quick glance I see things like swallowed errors (// console.log(err)), no support for devices (even though associations to device is setup), no tests, and the target branch PR is still in draft/has no reviewer. 🤷

note also, I believe you might need to drop the association for device and switch to application logic as we have done for things like access_token (casting from int to string for the device.id/ownerId will likely fail for PG) - but in saying that I see existing tests did not fail

@hardillb

hardillb commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

I see I was tagged as reviewer for this but Is it ready for review Ben? Just from a very quick glance I see things like swallowed errors (// console.log(err)), no support for devices (even though associations to device is setup), no tests, and the target branch PR is still in draft/has no reviewer. 🤷

Yes all intentional, especially the devices as we have no way to gather that information as it is not shared. The current table is built that way to make it work if/when it ever is available.

The errors get dropped because this is a async hook fired in the background and the only possible errors are back end db so failing to update is a small problem

Comment thread forge/db/models/Project.js Outdated
@hardillb
hardillb requested a review from knolleary July 27, 2026 15:34
@hardillb

Copy link
Copy Markdown
Contributor Author

@Steve-Mcl The AccessTokens still has belongsTo for Device in the associations. But we are limited to just Projects for this pass, just need to make sure we don't do anything that will break later if/when we can add Device installed nodes.

@hardillb

Copy link
Copy Markdown
Contributor Author

Something strange is going on with package flies and lint, I'm going to merge this down to the other PR so I only need to fix it once and it can all be reviewed as one

@hardillb
hardillb merged commit 28fd7fb into cache-catalogue-versions Jul 27, 2026
19 of 24 checks passed
@hardillb
hardillb deleted the installed-node-settings-hook branch July 27, 2026 16:12

@Steve-Mcl Steve-Mcl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I have a some thoughts on the design...

  1. Why we are not using the existing controller methods getProjectModule? - but then I found your proposal in a grandparent issue #6756 comment

    extract dependences table from settings object into it's own table to make searching/comparing displaying quicker/eastier

    Fair enough - a flat/normalised table could be useful to improve performance of bom operations

  2. As there is only a single table, every row has a latestVersion meaning potentially hundreds/thousands of duplicates. I would prefer a separate table e.g. NodeREDPackageVersions where each row simply represents a single package - then a simple join with this table NodeREDNodeVersions would remove the need for a static updateAllLatest method.

However, this is not blocking and could be later retrod so I wont block.

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.

Add hook to instance settings update to extract installed nodes and versions

3 participants