Skip to content

ENG-2040 Test the generic upsert route#1238

Open
maparent wants to merge 2 commits into
eng-2039-create-a-generic-upsert-routefrom
eng-2040-test-the-generic-upsert-route
Open

ENG-2040 Test the generic upsert route#1238
maparent wants to merge 2 commits into
eng-2039-create-a-generic-upsert-routefrom
eng-2040-test-the-generic-upsert-route

Conversation

@maparent

@maparent maparent commented Jul 19, 2026

Copy link
Copy Markdown
Collaborator

https://linear.app/discourse-graphs/issue/ENG-2040/test-the-generic-upsert-route

https://www.loom.com/share/7a958930a0e641d0b5552f42bb2863f2

Added after the loom: I also now have to start the next server in the GitHub test database action.

@linear-code

linear-code Bot commented Jul 19, 2026

Copy link
Copy Markdown

ENG-2040

@supabase

supabase Bot commented Jul 19, 2026

Copy link
Copy Markdown

This pull request has been ignored for the connected project zytfjzqyijgagqxrzbmz because there are no changes detected in packages/database/supabase directory. You can change this behaviour in Project Integrations Settings ↗︎.


Preview Branches by Supabase.
Learn more about Supabase Branching ↗︎.

@vercel

vercel Bot commented Jul 19, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
discourse-graph Ready Ready Preview, Comment Jul 20, 2026 2:40pm

Request Review

@devin-ai-integration devin-ai-integration Bot 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.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 1 additional finding.

Open in Devin Review

@mdroidian mdroidian left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we want to start Next for the entire database Cucumber suite? Only addGeneric.feature needs it, so this adds another startup dependency and failure mode to every database test.

I think the route-level coverage is worthwhile, but I’d isolate it in a separate command/job that starts Next and runs only this feature. The regular database suite could remain Supabase-only. We should also poll an HTTP health check instead of relying on the "Ready in " console output.

Also, are we missing relationTypeSchemas?

const servingReady = new Promise((rsc, rjc) => {
resolveCallback = rsc;
rejectCallback = rjc;
// Add timeout

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

.

// Add timeout
this.timeoutClear = setTimeout(() => {
rjc(new Error("Timeout waiting for functions to serve"));
}, 30000); // 30 second timeout

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

.

console.error("Could not kill the process");
// maybe it just ended on its own.
}
if (nextServer.process.pid)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

On Windows process.kill(-pid) returned ESRCH and left the detached child alive, leaking port 3000.

],
"relations": [
{
"name": "a supports relation",

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should this match CrossAppRelation?

@maparent
maparent force-pushed the eng-2040-test-the-generic-upsert-route branch from 2c5ad8e to 109ed57 Compare July 20, 2026 13:13
@maparent
maparent force-pushed the eng-2039-create-a-generic-upsert-route branch from 0dcfe78 to 9817046 Compare July 20, 2026 13:13
@graphite-app

graphite-app Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

PR size/scope check

This PR is over our review-size guideline.

  • Recommended: ~200 lines changed
  • Acceptable limit: up to 400 lines when well-scoped/self-contained
  • Preferred file count: fewer than 5 files

Please split this into smaller PRs unless there is a clear reason the changes need to land together.

If keeping it as one PR, please add a brief justification covering:

  • What single problem this PR solves
  • Why the files/changes are coupled

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.

2 participants