Fix type error when e.with() wraps unlessConflict()

[clarkg]

discussed in Slack with Scott - insertUnlessConflict should be Withable also

there actually weren't any previous tests explicitly testing insert is withable, but added one anyway for unlessConflict

[gel-data-cla]

All commit authors signed the Contributor License Agreement.

[clarkg]

@scotttrinh any chance we can get this in?

also the "Regression Tests" have been hanging this whole time with no way for me to inspect their status - do they need to complete?

[scotttrinh]

@clarkg Apologies for the delay here. I have to kick off the CI runs manually due to security settings on the org, which I've just done. I'll get this merged today.

[clarkg]

@scotttrinh no worries, that makes sense.

selfishly I was being lazy and was YOLOing the test without actually setting up a test env. just attempted to fix the latest test failure, hope it works this time

[clarkg]

@scotttrinh omg i'm so sorry - i read characters in the schema and still wrote movie_characters. fixed again

fwiw i totally acknowledge my laziness here and apologize for the thrash asking you to re-run the tests, but it is surprising and adds too much friction to contribute. i contributed to Expo last week and they just automatically kickoff tests which let me pretty easily YOLO contribute

[scotttrinh]

it is surprising and adds too much friction to contribute. i contributed to Expo last week and they just manually kickoff tests which let me pretty easily YOLO contribute

It's all good: the rule is that if you've never had a PR merged, we have to manually trigger them just to add a bit of trust in the PR process. Actions have pretty powerful capabilities, so our security stance has been to be a bit more careful than the default in GitHub. Once we've merged this PR, your subsequent contributions (if you ever come back! 🙈 ) will not require this manual intervention. I also acknowledge that onboarding someone to being able to run the tests themselves locally is not great at the moment, and I'll work on that, too!

[clarkg]

@scotttrinh thanks. now finally getting to a more meaningful error, but probably still dumb Q:

FAIL ./insert.test.ts
  ● insert › with wrapping insert .unlessConflict()

    Expr or its aliases used outside of declared 'WITH' block scope

      311 |       ]) {
      312 |         if (scope === null || !validScopes.has(scope)) {
    > 313 |           throw new Error(
          |                 ^
      314 |             refData.boundScope
      315 |               ? `Expr or its aliases used outside of declared 'WITH' block scope`
      316 |               : `Cannot extract repeated or aliased expression into 'WITH' block, ` +

      at Proxy.$toEdgeQL (dbschema/edgeql-js/toEdgeQL.ts:313:17)
      at Proxy.$queryFunc (dbschema/edgeql-js/query.ts:30:22)
      at Object.<anonymous> (insert.test.ts:173:32)

what's wrong with this though - dep is inside the e.with?

    const query = e.with(
      [dep], // dependency used inside the insert expression
      e
        .insert(e.Movie, {
          title: "WithUnlessConflict – test",
          characters: e.select(dep, () => ({
            "@character_name": "Peter Quill",
          })),
        })
        .unlessConflict((movie) => ({
          on: movie.title,
        })),
    );
    ```

[clarkg]

@scotttrinh ok this is it for sure, thanks again. last test just failed for formatting and I ran prettier

[scotttrinh]

@clarkg I'm going to push up a fix for a few issues, I just ran the tests locally and was able to iterate to the simplest form that works here after a few more rounds with the tests. Stand by!

[clarkg]

@scotttrinh thanks, sorry was losing my mind because i don't understand why that assertion wasn't true . i know it's a compile time check but tried it in my env:

[scotttrinh]

I feel like we've been through a battle together @clarkg , thanks for sticking with us through that!