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

[16.0] pos_receipt_replace_user_by_trigram: Drop compute function #1274

Open
wants to merge 3 commits into
base: 16.0
Choose a base branch
from

Conversation

grindtildeath
Copy link
Contributor

The compute function does not allow any customization on how the trigram had to be generated, and there a multiple ways to define how trigram must be generated.

Moreover, the compute did generate duplicates and there was no way to fix manually since the field was not stored. Also, making it stored but still computed did risk unwanted updates in case name was updated.

Having a basic stored field allows each company to customize how trigram can be generated using a server action. The logic previously provided is then moved to a cron that can be customized to suit trigram generation for each company.

@grindtildeath grindtildeath changed the title pos_receipt_replace_user_by_trigram: Drop compute function [16.0] pos_receipt_replace_user_by_trigram: Drop compute function Dec 10, 2024
The compute function does not allow any customization on how the
trigram had to be generated, and there a multiple ways to define
how trigram must be generated.

Moreover, the compute did generate duplicates and there was no way
to fix manually since the field was not stored. Also, making it
stored but still computed did risk unwanted updates in case name
was updated.

Having a basic stored field allows each company to customize how
trigram can be generated using a server action. The logic previously
provided is then moved to a cron that can be customized to suit
trigram generation for each company.
@grindtildeath grindtildeath force-pushed the 16.0-imp-remove_trigram_compute branch from a2aee40 to a1740b2 Compare December 10, 2024 18:14
@grindtildeath
Copy link
Contributor Author

Copy link
Contributor

@trisdoan trisdoan left a comment

Choose a reason for hiding this comment

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

Sounds like a great idea to me! Thanks

Could you squash the fixup?

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.

2 participants