-
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
[stable31] Fix user collaborators returned when searching for mail collaborators #56385
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
Conversation
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
013674e to
5bdf2f6
Compare
danxuliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works 👍
However, I will wait to mark it as Ready for review until CI has finished.
danxuliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CI failures are legit due to changes between Nextcloud 31 and 32. I will fix them.
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The OCS endpoint expects either an int or an array for "shareType". However, when using "getRowsHash()" only a single key is taken into account, so instead of: | shareType[] | 0 | | shareType[] | 4 | the share types are provided in a single row like: | shareTypes | 0 4 | and then converted to "shareType[]=0&shareType[]=4" when sending the request. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
The "sharebymail" app is enabled by default, so it needs to be enabled once the scenario ends as other scenarios could expect that the app is enabled. To solve that now a special step is added that records the enabled state of the given app and restores it once the scenario ends. This step only restores the state of already installed apps. If an app is installed during the test it will not be neither disabled nor uninstalled after the test ends. Therefore, at least for now, it is necessary to explicitly call the step to record the app to be restored, rather than automatically keeping track of the changes in the enabled state of the apps during the scenario. Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
…ators The MailPlugin collaborator returned results for both user and mail collaborators, but it was registered only for mail collaborators. While it might make sense to move the user results to the UserPlugin instead that change would be more complex and riskier, so for now the MailPlugin is now registered for both user and mail collaborators and the results are limited only to the registered type. As the plugins are registered only with their class and then resolved when needed using dependency injection it is not possible (as far as I know) to provide an explicit parameter in the constructor to differentiate whether the MailPlugin should return user or mail collaborators. To overcome this two subclasses are introduced, MailByMailPlugin and UserByMailPlugin, which just hardcode in their constructor the collaborator type that their parent MailPlugin must use, and those subclasses are the ones registered instead of the MailPlugin (which still contains all the logic). Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
5bdf2f6 to
a4bf16e
Compare
danxuliu
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests adjusted, as in Nextcloud 31 UserPlugin does not return duplicated results when searching for e-mail. That happens only in Nextcloud 32 and later because, with the database user backend, getting the user by id also returns the user if the system e-mail address is given (due to #47686, see the default value for tryEmail).
Backport of #52012