Using manual database transactions requires an ugly type cast

[diesieben07]

We have a background job that makes heavy use of database transactions. Previously this background job used MongoDB directly, which was error prone (especially in the presence of versioning) and not very easy to understand. I migrated this code over to the new DatabaseAdapter API. This works very well, except for two (minor) issues):

1. Typescript error when trying to use the new DatabaseAdapter API. #3521

2. As far as I can tell the only way to supply the transaction ID to a local API operation is via the PayloadRequest object (handled here in initTransaction, which is called from all operations). PayloadRequest is a big type, with lots of properties. Just calling local operations, the following seems to work fine, but "feels wrong":

   const transactionId = await payload.db.beginTransaction();
   await payload.find({
       collection: 'collection',
       req: {
         payload: payload,
         transactionID: transactionId ?? undefined,
       } as PayloadRequest
   });

   This is not documented, the documentation simply states "When writing your own scripts or custom endpoints, you may wish to have direct control over transactions. This is useful for interacting with your database outside of Payload's local API."
   If I wasn't using Payload's local API I didn't need DatabaseAdapter in the first place, I'd just be using the API of whatever database adapter library I'm using (e.g. using Mongoose directly, etc.). The great thing about Payload's DatabaseAdapter is benefiting from Payload's abstractions and still leveraging transactions.

   It would be great if there was a simpler way to tell the local API which transaction to use.

[DanRibbens]

I love this feedback. I knew transactions would be a win for many people. How can we make this better?

I had a db.transaction function in development that would be something you give a callback to, but I removed before releasing because it wasn't complete and wouldn't work how I originally wanted.

The idea though is that you could use payload's transaction wrapper with one incoming req rather than passing it to each local operation.

I removed it because I didn't have a great pattern internal to Payload for how to get the transaction req when calling the local API operations. It would have lead to confusion in our codebase of where the source of truth is for the transaction to use. I will have to think about this more!

We certainly need more docs about this feature.

The way I was writing it while testing this feature was like this:

const req = {} as PayloadRequest
req.transactionID = await payload.db.beginTransaction()

await payload.create({
    collection: 'collection',
    data: { /* whatever you have to create */ }
    req,
});

await payload.find({
    collection: 'collection',
    req,
});

Same as you are doing effectively, just maybe slightly cleaner. I assign transactionID in one req object and you don't need to pass payload on req, it is this same thing as the local api payload instance.

I was looking to improve the typing of req so you don't need as PayloadRequest also but punted on it with our 2.0 deadline too near. We can revisit this also.

If you want to open a PR for our docs that would be helpful to us, otherwise I'll make an effort when time allows.

Thanks for the feedback!

[diesieben07]

A db.transaction function that hides the intricacies would be great! The only issue with it is that it introduces more global state, as it somehow needs to let the entirety of Payload know that there now is a transaction happening. But the code running "in the transaction" might be asynchronous, so it might be interleaved with other application code (which should not use this transaction) while it is waiting for I/O or other things. I am not sure how this would best be done.

I will look into doing a pull request to improve the documentation tonight!

[diesieben07]

I have created a pull request to improve the documentation: #3648