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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMP] model: allow to batch commands in one history step #4077

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hokolomopo
Copy link
Contributor

@hokolomopo hokolomopo commented Apr 15, 2024

[IMP] model: allow to batch commands in one history step

We sometime need to create a UI plugin specifically to handle a command
so that its sub-commands are batched in a single history step.

This is kind of a problem when working with stores, because then we
need both a store to handle all the business logic, and a plugin that
handle a command created specifically to batch sub-commands (looking
at you find & replace).

We can fix that easily by creating a local command BATCH_COMMANDS
that take a callback as argument, and that batches every command
and sub-command executed in the callback within one history step.

Task: 3870119

[IMP] f&r: remove find & replace plugin

Now that we can batch commands, the plugin find & replace
is now useless.

Task: 3870119

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_t("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Collaborator

robodoo commented Apr 15, 2024

Copy link
Contributor

@anhe-odoo anhe-odoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure for the BATCH_COMMANDS name, as this is more like a command that can run any kind of callback, and, by design, every commands in the callback will be batched into a single history step, but this command could be used for something else with some other kind of callback ...
But I can't find a good name ATM :/ Maybe something like RUN_CALLBACK ? (But I don't like this one either)

We sometime need to create a UI plugin specifically to handle a command
so that its sub-commands are batched in a single history step.

This is kind of a problem when working with stores, because then we
need both a store to handle all the business logic, and a plugin that
handle a command created specifically to batch sub-commands (looking
at you find & replace).

We can fix that easily by creating a local command `BATCH_COMMANDS`
that take a callback as argument, and that batches every command
and sub-command executed in the callback within one history step.

Task: 3870119
Now that we can batch commands, the plugin find & replace
is now useless.

Task: 3870119
@hokolomopo
Copy link
Contributor Author

hokolomopo commented Apr 23, 2024

I'm not sure for the BATCH_COMMANDS name, as this is more like a command that can run any kind of callback, and, by design, every commands in the callback will be batched into a single history step, but this command could be used for something else with some other kind of callback ... But I can't find a good name ATM :/ Maybe something like RUN_CALLBACK ? (But I don't like this one either)

Yeah the name isn't great. I'll go with the name Pierre proposed (ONE_HISTORY_STEP) for now if nobody has a better idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants