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

Per user OCR flow is configuring global OCR flow for whole server #60

Closed
l00v3 opened this issue Jul 19, 2021 · 5 comments · Fixed by #61
Closed

Per user OCR flow is configuring global OCR flow for whole server #60

l00v3 opened this issue Jul 19, 2021 · 5 comments · Fixed by #61

Comments

@l00v3
Copy link

l00v3 commented Jul 19, 2021

nextcloud version: 21.0.3.1
workflow_ocr version: 1.21.0
ocrmypdf version 12.2.0

Hello.
We have an issue with the "workflow_ocr" app.
If user sets ocr flow in their settings, it will work for all the users on the server, not just for the specific user.
This was quite a surprise, because we set OCR flow for the test user and it works great, but it also started OCR convert for all the other users, even if they don't have OCR flow set in their settings.
There is no global OCR flow set in the admin panel.

So expected behaviour is that per-user settings work only for that specific user, not the whole server. Global settings would be made in the admin panel in the global flow section.

Is this a bug in the app? I can provide more information, if necessary.

Thank you very much!

@R0Wi
Copy link
Contributor

R0Wi commented Jul 19, 2021

HI @l00v3, thanks for reporting this. We have to check this behaviour to make a statement where the problem comes from. We'll keep you up to date.

@R0Wi R0Wi added the bug Something isn't working label Jul 19, 2021
@R0Wi
Copy link
Contributor

R0Wi commented Jul 19, 2021

Ok seems to be some kind of bug but i'm not sure if it's related to the app itself or the error resides inside the NC workflow core.

Ping @blizzz could you help us here, please? I compared our code from https://github.com/R0Wi/workflow_ocr/blob/master/lib/Operation.php#L91 to the one inside the workflow_pdf app (https://github.com/nextcloud/workflow_pdf_converter/blob/master/lib/Operation.php#L82) and i couldn't see much of a difference. I also debugged the explained scenario from above and indeed the Operation.onEvent method is called if user A created a personal workflow and user B uploaded a matching file. Could that be a bug inside NC workflow engine itself or are we doing anything wrong here? Thanks for your support 👍

@R0Wi R0Wi added the help wanted Extra attention is needed label Jul 19, 2021
R0Wi added a commit that referenced this issue Jul 20, 2021
Filter unwanted events for users
by invoking ruleMatchers getFlows method.
This will apply various checks and only
return a value if all checks passed.

Fixes #60
@R0Wi R0Wi removed the help wanted Extra attention is needed label Jul 20, 2021
@R0Wi
Copy link
Contributor

R0Wi commented Jul 20, 2021

Ok i think i found the culprit: like proposed in the workflow dev documentation we shall always call IRuleMatcher->getFlows() which will apply various checks (for example checking the user scope at https://github.com/nextcloud/server/blob/71109b74259fdc5ec78e278d1674cf314c414ae1/apps/workflowengine/lib/Service/RuleMatcher.php#L157). Since this flow here was first planned to be only available for admin scope but we also implemented the user scope in #16 we missed that part somehow.

Will hopefully be fixed by #61.

@bahnwaerter would be happy if you could do a quick check with the scenario mentioned above.

Since this seems to be a major bug we are going to backport it to NC22, NC21 and NC20.

@R0Wi R0Wi self-assigned this Jul 20, 2021
R0Wi added a commit that referenced this issue Jul 20, 2021
Filter unwanted events for users
by invoking ruleMatchers getFlows method.
This will apply various checks and only
return a value if all checks passed.

Fixes #60
R0Wi added a commit that referenced this issue Jul 20, 2021
Filter unwanted events for users
by invoking ruleMatchers getFlows method.
This will apply various checks and only
return a value if all checks passed.

Fixes #60
@R0Wi R0Wi closed this as completed in #61 Jul 20, 2021
R0Wi added a commit that referenced this issue Jul 20, 2021
* Use IRuleMatcher getFlows

Filter unwanted events for users
by invoking ruleMatchers getFlows method.
This will apply various checks and only
return a value if all checks passed.

Fixes #60

* "Fix" CI with mariadb

Inspired by
nextcloud/spreed@e8b1456
@bahnwaerter
Copy link
Collaborator

Reopen this issue because the fix in pull request #61 was not backported to any referenced stable branch. The backporting failed since the backport bot can't access any stable branch. @R0Wi, please take a look at it.

@l00v3: After this CI/CD issue is solved, we will release a new patch version for NC 20, NC 21 and NC 22 in the Nextcloud app store. So, have a little more patience ;-)

@bahnwaerter bahnwaerter reopened this Jul 21, 2021
bahnwaerter added a commit that referenced this issue Jul 22, 2021
* Use IRuleMatcher getFlows

Filter unwanted events for users
by invoking ruleMatchers getFlows method.
This will apply various checks and only
return a value if all checks passed.

Fixes #60

* "Fix" CI with mariadb

Inspired by
nextcloud/spreed@e8b1456

Co-authored-by: Robin Windey <[email protected]>
R0Wi added a commit that referenced this issue Jul 22, 2021
* Use IRuleMatcher getFlows

Filter unwanted events for users
by invoking ruleMatchers getFlows method.
This will apply various checks and only
return a value if all checks passed.

Fixes #60

* "Fix" CI with mariadb

Inspired by
nextcloud/spreed@e8b1456

Co-authored-by: Robin Windey <[email protected]>
R0Wi added a commit that referenced this issue Jul 22, 2021
* Use IRuleMatcher getFlows

Filter unwanted events for users
by invoking ruleMatchers getFlows method.
This will apply various checks and only
return a value if all checks passed.

Fixes #60

* "Fix" CI with mariadb

Inspired by
nextcloud/spreed@e8b1456

Co-authored-by: Robin Windey <[email protected]>
@R0Wi
Copy link
Contributor

R0Wi commented Jul 22, 2021

Fixed in

  • v1.20.3
  • v1.21.1
  • v1.22.1
    Thank you all 👍 Please reopen if issue still exists.

@R0Wi R0Wi closed this as completed Jul 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants