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

Tenant Scripting Enhancements #1964

Merged
merged 11 commits into from
Apr 8, 2024
Merged

Tenant Scripting Enhancements #1964

merged 11 commits into from
Apr 8, 2024

Conversation

idlira
Copy link
Contributor

@idlira idlira commented Apr 5, 2024

Description

  • Permits to disable scripts
image
  • Fixes parameter initialization.
  • The feature must be explicitly enabled in jobs by overwriting the ImportBatchProcessFactory.enableScriptableEvents method. For this to work, we changed the magic detection logic when only one script exists: with the feature disabled, no parameter is collected, thus the NOOP_DISPATCHER is used. If the parameter is collected but contains only one single entry, its hidden by default. One of the main advantages is, that even for hidden parameters the job will log the default script used during its initialization, eg:
image

Additional Notes

  • This PR fixes or works on following ticket(s): OX-10898

Checklist

  • Code change has been tested and works locally
  • Code was formatted via IntelliJ and follows SonarLint & best practices

idlira added 5 commits April 5, 2024 11:30
ensuring its loading the latest state of scripts

Fixes: OX-9839
enableScriptableEvents must be overwritten so scripting takes effect.

If active, and only one script exists, the parameter is collected hidden and the first entry will be the default one.

This also provides better tracking to which dispatcher was actually used (even a hidden one) since the parameter and its value is logged at job start.

Fixes: OX-9839
@idlira idlira added 🧬 Enhancement Contains new features 🐛 Bugfix Contains only a small fix for an existing bug labels Apr 5, 2024
Copy link
Member

@jakobvogel jakobvogel left a comment

Choose a reason for hiding this comment

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

The usual question: The use of ne constraints does not have adverse effects in terms of DB performance and the use of a search index?

idlira and others added 2 commits April 5, 2024 15:03
Co-authored-by: Jakob Vogel <jvo@scireum.de>
@idlira
Copy link
Contributor Author

idlira commented Apr 5, 2024

The usual question: The use of ne constraints does not have adverse effects in terms of DB performance and the use of a search index?

The usual question: The use of ne constraints does not have adverse effects in terms of DB performance and the use of a search index?

Should not. In fact the new field is not even in the index, I will add it...
We could directly use .eq(DISABLED, false) instead since there are no productive data yet.

and changes queries to use $eq instead of $ne. Since the entity is new, there is no need to resave them unless in a few local systems

Fixes: OX-9839
Copy link
Member

@jakobvogel jakobvogel left a comment

Choose a reason for hiding this comment

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

🙏

@andyHa andyHa merged commit c7d48af into develop Apr 8, 2024
3 checks passed
@andyHa andyHa deleted the ili/more-scripting branch April 8, 2024 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Bugfix Contains only a small fix for an existing bug 🧬 Enhancement Contains new features
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants