Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit an event after items are manually sorted 馃挴 #11622

Conversation

infomiho
Copy link

@infomiho infomiho commented Feb 14, 2022

This PR tries to address this discussion #10888

The idea is to add a new action called sort_update which can be an extension point for e.g. clearing cache on external CDN.

I've tested this locally with a test collection and it works as expected.

module.exports = function registerHook({ action }, { logger }) {
	action('test.items.sort_updated', () => {
		logger.info('test.items.sort_updated');
	});

	action('items.sort_updated', () => {
		logger.info('items.sort_updated');
	});
};

@infomiho infomiho changed the title Adds the 'sort_updated' event emit after items are sorted 馃挴 Emit an event after items are manually sorted 馃挴 Feb 14, 2022
@rijkvanzanten rijkvanzanten self-assigned this Feb 28, 2022
@rijkvanzanten rijkvanzanten added this to the v9-next milestone Feb 28, 2022
@rijkvanzanten
Copy link
Member

Thanks @infomiho!

@benhaynes Thoughts on the event name? sort_update is the only one so far with two words. It could just be sort? 馃

@benhaynes
Copy link
Sponsor Member

Thoughts on the event name? sort_update is the only one so far with two words. It could just be sort? 馃

Yeah, sort makes sense, because (AFAICT) there would never be any other sort operations (eg: Delete, Create). If you agree with that, then let's stay consistent with the single word. 馃憤

@infomiho
Copy link
Author

infomiho commented Mar 1, 2022

Makes sense, I'll update it today to be only sort 馃憤

@nickrum
Copy link
Member

nickrum commented Mar 1, 2022

I'm wondering if we should rather emit items.update events for the items affected by the sort order change, as manually sorting items simply causes the sort fields to update 馃

@rijkvanzanten
Copy link
Member

@nickrum Hmm that's an interesting one. Technically speaking, the sort operation functions differently from a regular update, so there is a case to be made to treat it as a separate event as well 馃

@benhaynes
Copy link
Sponsor Member

I worry about repurposing items.update` if that will save revisions for every sort change. Maybe good to decouple these? 馃

@infomiho
Copy link
Author

infomiho commented Mar 2, 2022

One extra argument for decoupling items.update and items.sort. To me those two events feel semantically different:

  • sort is more "collection level" hook
    • "hey collection sort was updated",
  • and update is more "single items" level hook
    • "hey these items got updated".

@rijkvanzanten
Copy link
Member

One extra argument for decoupling items.update and items.sort. To me those two events feel semantically different:

  • sort is more "collection level" hook

    • "hey collection sort was updated",
  • and update is more "single items" level hook

    • "hey these items got updated".

Agreed, I see it the same way. That being said, what Nicola said is also true: Doing the "collection sort was updated" in practice means that the sort field of the individual items was updated 馃

@benhaynes this is unrelated to revisions or revisions tracking, so that doesn't matter 馃憤馃徎

@rijkvanzanten rijkvanzanten changed the base branch from main to feat/items-sort-event March 5, 2022 00:44
@rijkvanzanten rijkvanzanten merged commit 3cd1e42 into directus:feat/items-sort-event Mar 5, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 3, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants