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

Delete polls for deleted user #1490

Merged
merged 224 commits into from
Apr 27, 2021
Merged

Delete polls for deleted user #1490

merged 224 commits into from
Apr 27, 2021

Conversation

jotoeri
Copy link
Member

@jotoeri jotoeri commented Mar 17, 2021

Hey @dartcafe,
this is a first step to delete a users polls after the user got deleted.
I recently implemented this for forms and as i originally realised this for polls on the forum and having nextcloud/forms#274 in mind, i thought why not just pushing the same here. 😉
However:

  • Currently this is not working, as the cron-job does not get the permission to delete on pollService
  • I'm not sure, if this $this->pollService->delete($poll->getId()); really deletes all information including options and votes for a poll. Probably somewhere is another procedure to delete all of that?

Unfortunately it's not just straight forward for me, so i'd need to dig deeper into the whole app. So if you want to use this part, just take it over (as far as i see, only the effective internal delete-action has to be fixed now). If you have too much other stuff around or don't want this, just close this PR. 😉

Greets,
Jonas

kauron and others added 6 commits February 12, 2021 10:42
Signed-off-by: kauron <9221197+kauron@users.noreply.github.com>
Updates the requirements on [phpunit/phpunit](https://github.com/sebastianbergmann/phpunit) to permit the latest version.
- [Release notes](https://github.com/sebastianbergmann/phpunit/releases)
- [Changelog](https://github.com/sebastianbergmann/phpunit/blob/master/ChangeLog-9.5.md)
- [Commits](sebastianbergmann/phpunit@8.5.0...9.5.2)

Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: Jonas Rittershofer <jotoeri@users.noreply.github.com>
@dartcafe
Copy link
Collaborator

👍 That saved me some time 'cause that is still on my to do list. But so far I didn't dig into the event listner thing. Thanks.
With the deletion of a poll, all child elements of that poll get deleted due to db constraints.

But there is a little bit to do, we have to anonymize every action of the user, but votes should still stand after the user left the system.

@jotoeri
Copy link
Member Author

jotoeri commented Mar 17, 2021

Yeah, i don't know which data you store additionally, but for forms i now left the submissions as is, with the thought that submissions, also in connection to a user-name, are 'property' of the form-owner once submitted. But that's indeed discussable and needs some handling of e.g. the DisplayName if a user can not be found anymore.

Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
enhanced error handling in watchPolls
@dartcafe dartcafe added this to the 2.0 milestone Mar 17, 2021
dartcafe and others added 3 commits March 17, 2021 21:08
@dartcafe
Copy link
Collaborator

I will complete this after 1.8 is out (estimated for Saturday).

nextcloud-bot and others added 8 commits March 19, 2021 03:07
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
dartcafe and others added 14 commits April 24, 2021 13:15
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
…into redesign/comments

Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: Jonas Rittershofer <jotoeri@users.noreply.github.com>
…/polls into pr-1490/nextcloud/enh/user_deleted_forms

Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
@dartcafe
Copy link
Collaborator

I did a rebase and switched the logic to delete via sql.
@jotoeri Any reason, why you put this to a background job? Wouldn't it be better to delete the polls synchronously?

@jotoeri
Copy link
Member Author

jotoeri commented Apr 25, 2021

Just that doing it synchronously slows down the user-deletion process... So if several apps insert their job synchronously that can become quite annoying. So afaik such things should always be done asynchronously. Iirc there also was some discussion on that on the community-chat somehwen...

@dartcafe
Copy link
Collaborator

I see. I come over it, because the user gets deleted and his/her polls are still available with an invalid owner. This could confuse other users and looks like a bug.

@jotoeri
Copy link
Member Author

jotoeri commented Apr 25, 2021

Well typically cron-jobs are run quite often (each five minutes typ. with cron?)... So i don't think that should be too much of a problem... 🤷‍♂️ 😉

Signed-off-by: dartcafe <github@dartcafe.de>
Signed-off-by: dartcafe <github@dartcafe.de>
@dartcafe
Copy link
Collaborator

Finished the PR with deleting

  • shares
  • preferences
  • logs
  • subscriptions
  • shares

The userId in votes, comments and options will get renamed to a deleted status, to avoid changing poll options and results.

Signed-off-by: dartcafe <github@dartcafe.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants