-
Notifications
You must be signed in to change notification settings - Fork 25
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
fix: properly delete rows in views #826
Conversation
@@ -435,8 +435,7 @@ public function delete(int $id, ?int $viewId, string $userId): Row2 { | |||
$this->logger->error($e->getMessage(), ['exception' => $e]); | |||
throw new InternalError(get_class($this) . ' - ' . __FUNCTION__ . ': '.$e->getMessage()); | |||
} | |||
$rowIds = $this->mapper->getRowIdsOfView($view, $userId); | |||
if(!in_array($id, $rowIds)) { | |||
if(!$this->row2Mapper->isRowInViewPresent($id, $view, $userId)) { |
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.
This seems to be the last usage of $this->mapper so we could drop it from the constructor which makes it one less dependency of the class to resolve
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.
@enjeck Could you check that? Then we can merge this :)
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.
Nice catch. Looks good and thanks for the test adjustments, small cleanup comment inline, otherwise good to go from my side 👍
Needs one small fix for the php codestyle check. 😉 You can likely autofix that by running |
Signed-off-by: Cleopatra Enjeck M <patrathewhiz@gmail.com>
Signed-off-by: Cleopatra Enjeck M <patrathewhiz@gmail.com>
Signed-off-by: Cleopatra Enjeck M <patrathewhiz@gmail.com>
0656fac
to
3cb2614
Compare
Signed-off-by: Cleopatra Enjeck M <patrathewhiz@gmail.com>
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! |
This PR fixes an issue with deleting rows in a view.
Steps to reproduce:
Error traces: