-
Notifications
You must be signed in to change notification settings - Fork 68
Fix hasProcesses lookup and improve ProjectForm performance #6778
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
base: main
Are you sure you want to change the base?
Conversation
1491af1 to
793dde4
Compare
2c70631 to
950b15c
Compare
|
@BartChris just one question concerning the following remark
which you made here #6776 (comment) Doesn't that mean we introduce a new issue when merging this pull request? Currently, processes can be found by updated project titles immediately after saving the project, don't they? Do you think performance gains should have higher priority over bug-free functions? Maybe I just missed something, but that seems like a suboptimal tradeoff to me. In my opinion we should have a solution for this new issue ready (for example making use of the mentioned |
|
@solth Speaking about effects you are right. But i think the reindexing of processes happened just by accident. Because we triggered some Hibernate behaviour, not by clear intent. If we want a reindex, it should by done in a better way, not by breaking the app with catastrophic runtimes because we reindex thousands of processes during a UI cycle, as it seems to me. But you are right in a way, so i am conflicted and have no clear answer. My general stance on this issue is, that we allow project search by the database and the indexing of project title in a catch all field brings little value, so i would not index project title in the free search which is mostly intended for metadata. But this is debatable as well. Edit: Using the normal: "project:ABC" will still work after this change, as projetc search uses the database not the index. Only in the case of someone just putting "ABC" and expects to find projects which are still indexed as "DEF" under their new name "ABC" in the index wil break. |
|
Looking at the entity and indexing ( This leaves us in a constellation where we have only non-optimal options
|
I understand and agree that still being able to find processes using the updated project title via a search/filter with "project:" prefix should be sufficient. Considering that it is possible to immediately find processes via general index search using a recently updated project title right now, though, this should be documented as a regression when we merge this pull request. After all, we cannot be sure if there are some institutions who specifically use this functionality and consider it a deal breaker if it does not work anymore in the next release. I would prospose to open an issue or discussion for this afterwards to gauge how important this feature is after all and if we have to somehow re-implement it or not. @BartChris please rebase this pull request against the current |
Fixes #6777
Fixes #6776