Skip to content

Fix owner id in snapshot AI request#6016

Merged
Steve-Mcl merged 2 commits into
mainfrom
fix-owner-id-in-snapshot-ai-request
Sep 12, 2025
Merged

Fix owner id in snapshot AI request#6016
Steve-Mcl merged 2 commits into
mainfrom
fix-owner-id-in-snapshot-ai-request

Conversation

@Steve-Mcl
Copy link
Copy Markdown
Contributor

@Steve-Mcl Steve-Mcl commented Sep 12, 2025

Description

use id not hashid

NOTE: This was covered in tests & I proved tests were effective by running original test before updating them and epected it to fail - it did (i.e. test coverage existed, but it too was incorrect - false positive!) Test updated & passing as expected.

▼ Project API
  ▼ Generate snapshot change description
    ✖ returns 200 and forwards LLM response
AssertionError: expected Object {
  teamHashId: 'LDX62AVmMo',
  instanceId: 'c3d6d63b-bb05-4d0f-b455-609159364b25',
  instanceType: 'project',
  isTeamOnTrial: true
} to have property instanceId of '' (got 'c3d6d63b-bb05-4d0f-b455-609159364b25')
    at Assertion.fail (node_modules\should\cjs\should.js:275:17)
    at Assertion.value [as property] (node_modules\should\cjs\should.js:356:19)
    at Context.<anonymous> (test\unit\forge\routes\api\project_spec.js:3086:37)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

#6016

Related Issue(s)

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

@joepavitt
Copy link
Copy Markdown
Contributor

Thanks Steve

Copy link
Copy Markdown
Contributor

@cstns cstns left a comment

Choose a reason for hiding this comment

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

LGTM!

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.76%. Comparing base (9c5771c) to head (36de972).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6016   +/-   ##
=======================================
  Coverage   76.76%   76.76%           
=======================================
  Files         376      376           
  Lines       18869    18869           
  Branches     4492     4492           
=======================================
  Hits        14485    14485           
  Misses       4384     4384           
Flag Coverage Δ
backend 76.76% <ø> (ø)

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.

@joepavitt
Copy link
Copy Markdown
Contributor

joepavitt commented Sep 12, 2025

postgresql 08:23:24.14 INFO  ==> 
postgresql 08:23:24.14 INFO  ==> Welcome to the Bitnami postgresql container
postgresql 08:23:24.15 INFO  ==> Subscribe to project updates by watching https://github.com/bitnami/containers
postgresql 08:23:24.15 INFO  ==> Submit issues and feature requests at https://github.com/bitnami/containers/issues
postgresql 08:23:24.15 INFO  ==> 

ERROR:  insert or update on table "TeamMembers" violates foreign key constraint "TeamMembers_UserId_fkey"
DETAIL:  Key (UserId)=(3) is not present in table "Users".
pod pr-6016/flowfuse-setup-4 terminated (Error)
pod "flowfuse-setup-4" deleted from pr-6016 namespace

that doesn't look good?

@cstns
Copy link
Copy Markdown
Contributor

cstns commented Sep 12, 2025

I fail to see how that could be related to these changes

@joepavitt
Copy link
Copy Markdown
Contributor

I fail to see how that could be related to these changes

I agree - which is why i think it's even more concerning. @hardillb any thoughts?

@hardillb
Copy link
Copy Markdown
Contributor

This looks like something has failed in the pre-staging env @ppawlowski, best guess is that the user create failed and then the script tried to add the user to the team.

@Steve-Mcl Steve-Mcl merged commit 33800f4 into main Sep 12, 2025
22 of 23 checks passed
@Steve-Mcl Steve-Mcl deleted the fix-owner-id-in-snapshot-ai-request branch September 12, 2025 08:43
@Steve-Mcl
Copy link
Copy Markdown
Contributor Author

I missed the chitter chatter about the failed pre-staging - sorry if that means you cannot investigate @ppawlowski

@ppawlowski
Copy link
Copy Markdown
Contributor

This looks like something has failed in the pre-staging env @ppawlowski, best guess is that the user create failed and then the script tried to add the user to the team.

That's the story here - attempt to create a viewer user failed (app responded with 502 error for some reason). Next step in the script is to assign users to teams via direct insert into the database - this couldn't succeed since there was no viewer user available and resulted in a SQL error.

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.

5 participants