Remove MCPRegistration entries on device and project deletion

[Steve-Mcl]

Description

Remove MCPRegistration entries on device and project deletion

NOTE: Adding unit tests will take longer than I have right now, so in the interest time (impending release and other outstanding tasks), I will defer unit tests to a later time (will raise separate issue as follow up)
However, if you insist on tests to accompany this, please let me know.

Added model unit test

Related Issue(s)

closes #6423

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

[Steve-Mcl]

bah, postgres test fail...

error: database "flowforge" already exists
ERROR: "test:unit:forge" exited with 1.
ERROR: "test:unit" exited with 1.
Error: Process completed with exit code 1.

@hardillb any thoughts?

[Steve-Mcl]

Added unit tests - lets see if they pass.

[hardillb]

Withdrawing approval while tests are sorted out

[Steve-Mcl]

Ran PG tests locally and I am seeing below errors but I cannot understand why the project afterDestroy hook causes errors:

test/unit/forge/routes/api/project_spec.js

      ✔ Non-owner member cannot delete instance
Error cleaning up MCPRegistrations for deleted project: Error: ConnectionManager.getConnection was called after the connection manager was closed!
    at ConnectionManager.getConnection (flowfuse\node_modules\sequelize\lib\dialects\abstract\connection-manager.js:70:13)        
    at flowfuse\node_modules\sequelize\lib\sequelize.js:305:111
    at flowfuse\node_modules\retry-as-promised\dist\index.js:59:25
    at new Promise (<anonymous>)
    at retryAsPromised (flowfuse\node_modules\retry-as-promised\dist\index.js:48:12)
    at Sequelize.query (flowfuse\node_modules\sequelize\lib\sequelize.js:300:12)
    at PostgresQueryInterface.bulkDelete (flowfuse\node_modules\sequelize\lib\dialects\abstract\query-interface.js:403:33)        
    at MCPRegistration.destroy (flowfuse\node_modules\sequelize\lib\model.js:1838:42)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Project.afterDestroy (flowfuse\forge\db\models\Project.js:246:25)
    at async Project.runHooks (flowfuse\node_modules\sequelize\lib\hooks.js:96:7)
    at async model.destroy (flowfuse\node_modules\sequelize\lib\model.js:2630:7)
    at async Object.<anonymous> (flowfuse\forge\routes\api\project.js:283:13)
      ✔ Owner can delete an instance (822ms)
Error cleaning up MCPRegistrations for deleted project: Error: ConnectionManager.getConnection was called after the connection manager was closed!
    at ConnectionManager.getConnection (flowfuse\node_modules\sequelize\lib\dialects\abstract\connection-manager.js:70:13)        
    at flowfuse\node_modules\sequelize\lib\sequelize.js:305:111
    at flowfuse\node_modules\retry-as-promised\dist\index.js:59:25
    at new Promise (<anonymous>)
    at retryAsPromised (flowfuse\node_modules\retry-as-promised\dist\index.js:48:12)
    at Sequelize.query (flowfuse\node_modules\sequelize\lib\sequelize.js:300:12)
    at PostgresQueryInterface.bulkDelete (flowfuse\node_modules\sequelize\lib\dialects\abstract\query-interface.js:403:33)        
    at MCPRegistration.destroy (flowfuse\node_modules\sequelize\lib\model.js:1838:42)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async Project.afterDestroy (flowfuse\forge\db\models\Project.js:246:25)
    at async Project.runHooks (flowfuse\node_modules\sequelize\lib\hooks.js:96:7)
    at async model.destroy (flowfuse\node_modules\sequelize\lib\model.js:2630:7)
    at async Object.<anonymous> (flowfuse\forge\routes\api\project.js:283:13)
      ✔ Handles trying to delete an already deleted container without error (564ms)

What I did note was a similar error was occurring in test/unit/forge/routes/api/projectSnapshots_spec.js and that was (it seems) due to a duplicate after( test clean up handler.

• ref: flowfuse unit/forge/routes/api/projectSnapshots_spec.js line 74 and line 89 duplicate handlers

Some kinda weird timing issue that I am not confident are just test related.

Switching to draft for now.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.31%. Comparing base (687c618) to head (c5352e3).

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6424   +/-   ##
=======================================
  Coverage   76.30%   76.31%           
=======================================
  Files         395      395           
  Lines       19821    19829    +8     
  Branches     4747     4751    +4     
=======================================
+ Hits        15124    15132    +8     
  Misses       4697     4697           

Flag	Coverage Δ
backend	76.31% <100.00%> (+<0.01%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 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]

This weird!

[hardillb]

@Steve-Mcl happy for me to review this again, or want to sleep on it?

[Steve-Mcl]

Not sure!

The try catch is containing things, I just dont like that I have to do that.

I mean tests are passing now!

WDYT?

Would be nice to avoid orphaned MCP entries!