Open JohnieBraaf opened 3 years ago
JohnieBraaf
commented
3 years ago I've written a simple fix for the above described issue, by pushing down the workflow object id into the query. Please see commit: https://github.com/JohnieBraaf/django-river/commit/58f49624c37e1e5d1bdb4c7680810126454e36a6 I see that I have left some debugging code, so please ignore that.
Let me know what you think @javrasya
javrasya
commented
3 years ago Hi @JohnieBraaf. Thanks for reporting this and for the PR. I think this is a bug that should definitely be fixed probably in a similar way you did in your PR.
The thing is that the get_available_approvals method is an abstract method that both MSSQL and other DB versions are implementing. So we can't just update it for the MSSQL one.
The other thing is that this method is for fetching all available approvals for a user on every object but also can be used for a single workflow object. In such case I also think the object id should be passed down instead of applying the ORM filter on top of that (in here).
I will try to drop some comments on your changes. Let's try to discuss it there. Maybe even initialize a PR to have the conversation there.
JohnieBraaf
commented
3 years ago Thnx @javrasya for your feedback, I will have a look
Hi,
Sadly I've stubled upon another (critical) bug. This time in the part that fetches the available approvals. https://github.com/javrasya/django-river/blob/master/river/driver/mssql_driver.py#L20-L32
A list of approvals is fetched and then used as a filter on the id array for the TransactionApproval model. When the list of id's is larger than 2000, SQL server runs into a limitation:
My observation is that this part of the framework is quite inefficient, as I only want the approvals related to a single object and yet it is fetching all approvals and the again uses this to fire an additional parameterized query.
Is there a reasoning behind this?