Api / Services / Fields: Only emitFilter if emitEvents is not set to false

[joselcvarela]

Scope

The purpose of this PR is to fix eventual recursivity when using Fields Service.
More specifically, if we are using a filter hook for fields.update, fields.create or fields.delete, then use FieldsService.updateField,FieldsService.createField or FieldsService.deleteField, this may cause recursion because the emitFilter is always called.
In order to prevent this, we just need to do the same as other services which is check if emitEvents is set to false. If so
do not emitFilter and use original payload.

What's changed:

• Use emitEvents to prevent emitFilter to be called on fields.update, fields.create and fields.delete

Potential Risks / Drawbacks

• I think that no one is relying on this recursion mechanism using emitEvents: false option, because it's not correct. But if there are, this will be impacting them. In order to keep the recursion, they can just put remove emitEvents: false option.

Review Notes / Questions

• This recursion seems to be infinite since there are no stop condition, which means project will go down after some iterations.

[changeset-bot]

🦋 Changeset detected

Latest commit: 4980a41

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@directus/api	Patch
directus	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[joselcvarela]

Here is a reproduction sample that will crash the instance once a field is updated. Create it under extensions/hooks/fields/index.js:

export default ({ filter }, { services, database, getSchema }) => {
	filter('fields.update', async (field) => {
		const fields = new services.FieldsService({ knex: database, schema: await getSchema() });

		await fields.updateField(field.collection, { field: field.field, sort: 1 });
		console.log('Updating field!');
	});
};

[br41nslug]

Does that example not crash because the getSchema function is not await instead of being emitEvents related? 🤔

[joselcvarela]

@br41nslug Nope and just updated the code 👍