Action hook is triggered before the item is created

[gkielwasser]

Preflight Checklist

• [X] I have completed all Troubleshooting Steps.
• [X] I'm on the latest version of Directus.
• [X] There's no other issue that already describes my problem.

Describe the Bug

When an item is created indirectly by a relation, the item does not exist yet in the database when the action hook 'items.create' is triggered.

action('items.create', async function ({ collection, key: itemId }, { database: knex, schema, accountability }) {
  const { ItemsService } = services;
  const service = new ItemsService( collection, { knex, schema, accountability });
  const thisItemMightNotExistYet = await service.readOne(itemId)
}

To Reproduce

Create collections Craftsmen, Warehouses Add relation Craftsmen.warehouses O2M

add a new craftman with one warehouse inside:

//POST  /items/craftsmen 
{
  warehouses: [ { name: 'myWarehouse' } ]
}

A craftsman that has one warehouse is created.

From the action('items.create') the hook is trigered for the warehouse item. But we cannot retrieve the item from the database at this stage.

Errors Shown

No errors but the item does not exist in the database when the hook is triggered.

What version of Directus are you using?

9.4.3

What version of Node.js are you using?

14.17.6

What database are you using?

Postgres

What browser are you using?

Chrome

What operating system are you using?

Windows_NT 10.0.19042

How are you deploying Directus?

locally

[quomark]

Same here. When I run it locally, it works perfectly fine. But if I run it on public address, the action.create trigger before the item is created, and I cannot select the item by using knex.database, as it return null.

[joselcvarela]

Hey @gkielwasser I am trying locally and it seems the action('items.create') is dispatched for the top item you are currently saving. In your case, the hook should be fired for the creation of the item on collection craftsmen. I am not seeing nested items firing this hook. Although I can read nested item from the top level item.

What do you see on await service.readOne(itemId)?

Check code and results

This is the code I am using:

module.exports = ({ action }, { services }) => {
	action('items.create', async function ({ key: itemId }, { database: knex, schema, accountability }) {
		const { ItemsService } = services;
		const service = new ItemsService('categories', { knex, schema, accountability });

		const parent = await service.readOne(itemId);

		console.log(11111, parent);
	});
};

This is the result:

11111 {
  id: 13,
  status: 'draft',
  sort: null,
  user_created: 'bb81cec0-60b3-44db-9818-1c4cfb984c2c',
  date_created: '2022-01-20T16:21:39.156Z',
  user_updated: null,
  date_updated: null,
  name: '1111111',
  parent: null,
  children: [ 14 ]
}

And the request was made via app but should look like this:

POST /items/categories
{
  name: "11111"
  children: [{ name: "22222" }]
}

where children is O2M field in categories

[gkielwasser]

22222

Hey @joselcvarela thanks for help,

Ok I tried to create 2 freshs collections test_categories and test_categories_children. The POST request was triggered from the app with admin access.

I have a bit updated your code since I have an exception reading the children (I have this hook that is triggered).

action('items.create', async function ({ key: itemId, collection }, { database: knex, schema, accountability }) {
		try {
			console.log('Test action(create) Start', collection, itemId);
			const { ItemsService } = services;
			const service = new ItemsService(collection, { knex, schema, accountability });
			const item = await service.readOne(itemId);
			console.log('Test action(create) Result', collection, item);
		} catch(e){
			console.log('Test action(create) Exception', collection, itemId, e);
		}
	});

I have first action hook that is trigerred for test_categories_children but I have a permissions issue that throw an expection. Then I have the parent item hook triggered where I can see the output you sent (Excepts for sorting,status, user_created stuff I have not added to the collection).

{
  "id": 6,
  "name": "test",
  "children": [
    7
  ]
}

[joselcvarela]

@gkielwasser isn't that what you want? What are you expecting? In the original post you said you didn't have access to the item on the hook, but it seems you have now, right?

[gkielwasser]

@gkielwasser isn't that what you want? What are you expecting? In the original post you said you didn't have access to the item on the hook, but it seems you have now, right?

Hi @joselcvarela yep maybe I can rewrite original post if it is not clear. The hook is triggered for both: the craftmsen and the warehouse. But when it goes for the warehouse, I cannot retrieve the item.

I have noticed this morning that you have a self referenced relation in your test but it should not change anything about the hook. Here is some questions related to my rewritten upper test. Also I have 2 differents collections test_categories and test_categories_children

test_categories {
    children: test_categories_children 
}

1- Don't you have a create hook for both collection (one for test_categories and one for test_categories_children) as 2 differents items are created? 2- If yes, what do you see when the hook is for the test_categories_children collection?

My issue is only when the create hook for the test_categories_children will trigger

It looks like it is related to this ticket https://github.com/directus/directus/issues/5460 . So when my hook triggers I'm not sure the item will exists as the transaction is not done. Also it might be the reason I cannot read the item that will be "potentially created". I'm saying potentially as the transaction could still fail. What I think is a bit misleading is that depending on if the item has been created through a O2M or not we can not safely retrieve the item and performs an action.

Here i will try to summarize what I have understood so far: 1- when the hook action('items.create') triggers for test_categories_children . The transaction is still running. So reading the item in the regular database is not possible.

2- when the hook action('items.create') triggers for test_categories . The transaction is over and reading the item that is created in the regular database is possible.

It might be a wrong understanding but I tought this action hook items.create means: "the object has been created and there is nothing more you can do about it". But it seems more like "the object will probably be created and there is nothing more you can do about it"

@rijkvanzanten I would love to have also some feedback from you

[rijkvanzanten]

The action hooks run asynchronously and doesn't block the saving process. So when saving a single (non nested relational) item, it's fired after the database operation is done.

When you have nested relational values, the related values are inserted before or after the top level parent is saved, depending on if it's a many-to-one or one-to-many value respectively. These insertions themselves also call the hooks for those particular changes.

If you're using a hook on the top level collection, there's a chance the nested one to many records haven't been saved yet. If it's the top level transaction you're waiting for, you can access that by using the database connection passed through in the hook handler function:

export default ({ filter, action }) => {
	action('items.create', (payload, { database }) => {
		// `database` here is the currently active transaction
		console.log('Item created!');
	});
};

[gkielwasser]

he rela

@rijkvanzanten My point is about the hook triggered from the related value. From this hook reading the item from the active transaction is not working. The item doesn't exist yet.

For the top level collection there is no issue.

[rijkvanzanten]

@rijkvanzanten My point is about the hook triggered from the related value. From this hook reading the item from the active transaction is not working. The item doesn't exist yet.

Is that a many to one, or a one to many related item?

[gkielwasser]

test_categories { children: test_categories_children }

test_categories {
    children: O2M test_categories_children <----- Cannot read when this one will be created
}

[rijkvanzanten]

Are you using the current transaction as per my example above? That should allow you to read the previous item already as top level parents are created before any nested o2m items

[gkielwasser]

Are you using the current transaction as per my example above? That should allow you to read the previous item already as top level parents are created before any nested o2m items

Yes, I'm using the current transaction.

action('items.create', async function ({ key: itemId, collection }, { database: knex, schema, accountability }) {
        try {
            console.log('Test action(create) Start', collection, itemId);
            const { ItemsService } = services;
            const service = new ItemsService(collection, { knex, schema, accountability });
            const item = await service.readOne(itemId);
            console.log('Test action(create) Result', collection, item);
        }
        catch (e) {
            console.log('Test action(create) Exception', collection, itemId, e);
        }
    });

From my breakpoints I have set, I can always see first the related collection hook and Then the top level collection hook.

EDIT: also here it doesn't look like it is the transaction we pass https://github.com/directus/directus/blob/65bfe68b0c7993a1bd5b0daac0855265911313d0/api/src/services/items.ts#L207

[rijkvanzanten]

EDIT: also here it doesn't look like it is the transaction we pass

Ah! That could very well be the underlying source of the problem here then 🙂

I believe that was just tweaked as part of #10419, could you confirm @jaycammarano?

[jamescammarano]

EDIT: also here it doesn't look like it is the transaction we pass

Ah! That could very well be the underlying source of the problem here then 🙂

I believe that was just tweaked as part of #10419, could you confirm @jaycammarano?

Yup, it was changed!

[gkielwasser]

I have also to add when I read the item as described it throws

EDIT: also here it doesn't look like it is the transaction we pass

Ah! That could very well be the underlying source of the problem here then 🙂 I believe that was just tweaked as part of #10419, could you confirm @jaycammarano?

Yup, it was changed!

@jaycammarano I can see an update for the read action but not for create isn't it? https://github.com/directus/directus/blob/48003cb7257aceebc48a281c6592e7876417ec05/api/src/services/items.ts#L303

[jamescammarano]

I have also to add when I read the item as described it throws

EDIT: also here it doesn't look like it is the transaction we pass

Ah! That could very well be the underlying source of the problem here then 🙂 I believe that was just tweaked as part of #10419, could you confirm @jaycammarano?

Yup, it was changed!

@jaycammarano I can see an update for the read action but not for create isn't it?

https://github.com/directus/directus/blob/48003cb7257aceebc48a281c6592e7876417ec05/api/src/services/items.ts#L303

Oh yeah. You are right. I'll update the hooks throughout. Thank you.

EDIT: Commit pushed

[gkielwasser]

@rijkvanzanten Any chance passing the transaction for the action hook would be something considered as something like a critical bug and pushed in v9.5.0 milestone :) ? Maybe in a dedicated ticket ?

I'm not so sure what I should do stabilize again my app. I have dozen of methods that executes depending on the collection name, All are registered in this creation hook. Currently depending from where/how those items have been created lots of logic is broken. I'm not so sure what kind of hack I could apply my side waiting for the proper fix.

Also I didn't find the commit that switch the transaction with the getDatabase function. I have no idea so far from when my app is broken.

action('items.create', async function ({ key: itemId, collection }, { database: knex, schema, accountability }) {
  const { ItemsService } = services;
  const service = new ItemsService(collection, { knex, schema, accountability });
  const item = await service.readOne(itemId);

   switch(collection) {
      case 'collection1':
        handleItemCreationCollection1(item)
     case 'collection2':
        handleItemCreationCollection2(item)
   }
})

@jaycammarano Thanks for that fix and those upcomming tests :)

[rijkvanzanten]

@rijkvanzanten Any chance passing the transaction for the action hook would be something considered as something like a critical bug and pushed in v9.5.0 milestone :) ? Maybe in a dedicated ticket ?

I can't consider it mission critical for the whole of the user base of Directus, as it's a pretty use case specific problem 🙃 That being said, I believe @jaycammarano's PR is marked good to go, so I'll see what we can do to get that in the release 👍🏻

[gkielwasser]

@rijkvanzanten Any chance passing the transaction for the action hook would be something considered as something like a critical bug and pushed in v9.5.0 milestone :) ? Maybe in a dedicated ticket ?

I can't consider it mission critical for the whole of the user base of Directus, as it's a pretty use case specific problem 🙃 That being said, I believe @jaycammarano's PR is marked good to go, so I'll see what we can do to get that in the release 👍🏻

Nice ! Thanks both of you :wink:

[sarthakgaur]

@rijkvanzanten Can you please take a look at #11353. I have tried using the current transaction, but I am still getting an error https://github.com/directus/directus/issues/11353#issuecomment-1026533999.

[gkielwasser]

I just tested v9.5.1 with @jaycammarano ticket https://github.com/directus/directus/pull/10419

I don't really understand the error I have now: previously the item was not there on the database because the transaction was not ended. I tought item would be still in the transaction

But now we pass the transaction database in the hook https://github.com/directus/directus/blob/48003cb7257aceebc48a281c6592e7876417ec05/api/src/services/items.ts#L303

I have this error which is quite explicit :)

Error: Transaction query already complet
e, run with DEBUG=knex:tx for more info

@rijkvanzanten @jaycammarano any tought ?

[gkielwasser]

One more thing I have tried is catching the error Transaction query already completed and performs an other request but this time on the database.

It will give a 403 permission error You don't have permission to access this.

[eikaramba]

i am currently facing the same problem i think. i have a m2m relation and trying to clean up ghost entries which happen unfortunately when updating the relations. you can see my code and the issue for sqlite HERE (code update to working solution for me)

i also am getting

An error was thrown while executing action "posts_tags.items.create"
Transaction query already complete, run with DEBUG=knex:tx for more info

and a 403 permission error.

using latest directus as of now (9.5.1)

• If i encapsulate the code with try catch i at least don't get the permission error anymore. but the query is not executed and thus the ghost entries not deleted.
• more interesetingly the call worked when i got to a specific state of my relationship table. does not make any sense to me, so i just drop it here as reference but i guess it confuses more then it helps. Working only means the 403 permission error did not happen, the "transaction query already completed" error is always there. please know that i am using generic posts/tags examples above and below you see a screenshot with my real entities

update

it is working for me now with the following code. Basically i removed to reuse the same database transaction. i know i needed this in the past for sqlite because otherwise sometimes i got transaction conflicts due to sqlites single process blocking transactions. but it seems in this case here it is not needed and can even do more harm than good. hopefully it can also help someone else.

  action("devices_integrations.items.create", async ( {payload,key}, { accountability,schema,database }) => {
    //clean up
    try {
      const junctionServiceAdmin = new ItemsService("devices_integrations",{ schema });
      await junctionServiceAdmin.deleteByQuery({ filter: { _or:[{devices_id: {_null:true} },{integrations_id: {_null:true} }]}});
    } catch (err) {
      logger.error("error on devices_integrations.items.create cleanup %O",err);
    }

  });

[rijkvanzanten]

it is working for me now with the following code. Basically i removed to reuse the same database transaction. i know i needed this in the past for sqlite because otherwise sometimes i got transaction conflicts due to sqlites single process blocking transactions. but it seems in this case here it is not needed and can even do more harm than good. hopefully it can also help someone else.

Good one @eikaramba 👍🏻 This is actually more or less the same issue as https://github.com/directus/directus/issues/11885 then

[samdze]

I get the same ForbiddenException error even in this simple action hook:

action('users.create', async (input, { database, schema }) => {
	const usersService = new UsersService({ database, schema })
	const user = await usersService.readOne(input.key, { database, schema }) // <- Here.
})

No relations involved, just a simple user creation. It only happens sometimes, I'd say 80/90% of the times no error occurs. I'm on Directus 9.10.0.