Users can delete their account #1447

beagleknight · 2017-06-06T08:03:34Z

🎩 What? Why?

This adds the deleting accounts feature. A user will be able to delete his account filling in an optional reason. The account will become inaccessible and anonymizes his data.

📌 Related Issues

Closes Delete your account #900

📋 Subtasks

Delete account form
Fix menu highlight
Anonymize data for proposals
Anonymize data for comments

📷 Screenshots (optional)

👻 GIF

codecov · 2017-06-06T08:17:21Z

Codecov Report

Merging #1447 into master will increase coverage by <.01%.
The diff coverage is 98.21%.

@@            Coverage Diff            @@
##           master   #1447      +/-   ##
=========================================
+ Coverage   95.49%   95.5%   +<.01%     
=========================================
  Files         439     441       +2     
  Lines        7574    7627      +53     
=========================================
+ Hits         7233    7284      +51     
- Misses        341     343       +2

josepjaume

Nice!

josepjaume · 2017-06-06T10:08:51Z

decidim-core/app/commands/decidim/destroy_account.rb

+    private
+
+    def destroy_user_account!
+      @user.email = "deleted-user-#{SecureRandom.uuid}@example.org"


email can be empty when the user is deleted. There's a special index for that (not sure about the model's validation) but I think is way better to deal it this way.

And erase their name

josepjaume · 2017-06-06T10:09:13Z

decidim-core/app/commands/decidim/destroy_account.rb

+
+    private
+
+    def destroy_user_account!


We should delete all the user's identities as well, right?

And authorizations?

@mrcasals Authorizations are anonymous and doesn't container user data so we are fine.

mrcasals · 2017-06-06T10:10:48Z

decidim-comments/app/frontend/support/schema.ts

@@ -106,6 +114,8 @@ export interface CommentDataFragment extends UpVoteButtonFragment, DownVoteButto
    name: string,
    // The author's avatar url
    avatarUrl: string,
+    // Wheter the author's account has been deleted or not


s/Wheter/Whether

This is auto-generated so I made the change but didn't regenerate the schema 😄

mrcasals · 2017-06-06T10:12:02Z

decidim-core/app/assets/javascripts/decidim/account_form.js.es6

@@ -0,0 +1,27 @@
+/**
+ * Since the delete account has a modal to confirm it we need to copy the content of the
+ * reason field to the hidden field in the form inside the modal.


What the hell? Is this a normal thing to do?

No, it isn't. The problem is the foundation reveal is rendered outside the real form and it is also a copy of the template marked as a popup. I know it's not a perfect solution but I needed this to solve the problem.

mrcasals · 2017-06-06T10:12:46Z

decidim-core/app/assets/javascripts/decidim/account_form.js.es6

+        $deleteAccountModalForm.find('input#delete_account_delete_reason').val(reasonValue);
+        $modal.foundation('open');
+      } catch (error) {
+        console.error(error); // eslint-disable-line no-console


Shouldn't we report this to Sentry instead of logging to the console

We don't have sentry configured for js errors but I think it's a good idea. I will do it in a separate PR

mrcasals · 2017-06-06T10:15:16Z

decidim-core/config/locales/en.yml

@@ -13,6 +15,9 @@ en:
        user_group_document_number: Organization document number
        user_group_name: Organization name
        user_group_phone: Organization phone
+    placeholders:
+      account:
+        delete_reason: Reason


Do you really need a placeholder here?

I just copied the template but I think you are right.

josepjaume · 2017-06-06T10:20:13Z

decidim-core/config/locales/en.yml

@@ -2,6 +2,8 @@
 en:
  activemodel:
    attributes:
+      account:
+        delete_reason: Reason


I would change this to: "Reason to delete your account"

mrcasals · 2017-06-06T10:15:43Z

decidim-core/lib/decidim/core/api/author_interface.rb

    field :avatarUrl, !types.String, "The author's avatar url"
+
+    field :deleted, !types.Boolean, "Wheter the author's account has been deleted or not"


s/Wheter/Whether/

mrcasals · 2017-06-06T10:17:05Z

decidim-core/lib/decidim/core/api/user_type.rb

@@ -13,11 +13,15 @@ module Decidim
    field :name, !types.String, "The user's name"

    field :avatarUrl, !types.String, "The user's avatar url" do
-      resolve ->(obj, _args, _ctx) { obj.avatar.url }
+      resolve ->(obj, _args, _ctx) { obj.deleted? ? ActionController::Base.helpers.asset_path("decidim/default-avatar.svg") : obj.avatar.url }


This smells. Shouldn't we change the avatar to this when deleting the user? This way you don't need to check this value in the API level, you just change it when deleting the user.

Ok, since we don't need to preserve the old ones I think it is fine.

mrcasals · 2017-06-06T10:17:50Z

decidim-core/lib/decidim/core/api/user_type.rb

    end

    field :organizationName, !types.String, "The user's organization name" do
      resolve ->(obj, _args, _ctx) { obj.organization.name }
    end
+
+    field :deleted, !types.Boolean, "Whether the user's account has been deleted or not" do


field :deleted, type, "description", property: :deleted? should work just fine, and you avoid having to use the block

mrcasals · 2017-06-06T10:19:14Z

decidim-proposals/app/views/decidim/proposals/proposals/show.html.erb

@@ -11,11 +11,21 @@
    <div class="author-data__main">
      <div class="author author--inline">
        <span class="author__avatar">
-          <%= image_tag @proposal.author_avatar_url %>
+          <% if @proposal.author&.deleted? %>
+            <%= image_tag ActionController::Base.helpers.asset_path("decidim/default-avatar.svg") %>


Same, changing the avatar when deleting the user will make you skip this logic in the views

mrcasals · 2017-06-06T10:20:18Z

decidim-proposals/app/views/decidim/proposals/proposals/show.html.erb

        </span>
        <%= l @proposal.created_at, format: :long %>
        <span class="author__name">
-          <%= @proposal.author_name %>
+          <% if @proposal.author&.deleted? %>


Don't we use this in the admin section too? We should update it there

We are not displaying users in the admin section so I think it is not needed.

I made all the changes requested :D

beagleknight added 3 commits June 6, 2017 09:56

Add static delete account page

57182cc

Add double form to support both js and non js

1a0e129

Users can delete their account

2e66212

beagleknight added the in-progress label Jun 6, 2017

beagleknight self-assigned this Jun 6, 2017

beagleknight added 2 commits June 6, 2017 11:24

Anonymize user data

c3bec35

Add production bundle

a9fb4c3

beagleknight added in-review and removed in-progress labels Jun 6, 2017

oriolgual previously approved these changes Jun 6, 2017

View reviewed changes

Fix broken tests

5fcdad0

beagleknight dismissed oriolgual’s stale review via 5fcdad0 June 6, 2017 09:51

josepjaume approved these changes Jun 6, 2017

View reviewed changes

josepjaume suggested changes Jun 6, 2017

View reviewed changes

mrcasals reviewed Jun 6, 2017

View reviewed changes

josepjaume reviewed Jun 6, 2017

View reviewed changes

mrcasals previously requested changes Jun 6, 2017

View reviewed changes

beagleknight added 2 commits June 6, 2017 15:07

Add feedback

1cb16f9

Destroy user group memberships

03b2298

oriolgual previously approved these changes Jun 6, 2017

View reviewed changes

josepjaume previously approved these changes Jun 6, 2017

View reviewed changes

Fix rubocop issues

3a44d08

beagleknight dismissed stale reviews from josepjaume and oriolgual via 3a44d08 June 6, 2017 13:55

oriolgual approved these changes Jun 6, 2017

View reviewed changes

beagleknight merged commit ead7613 into master Jun 6, 2017

beagleknight deleted the feature/delete-account branch June 6, 2017 14:14

beagleknight removed the in-review label Jun 6, 2017

ricola mentioned this pull request Aug 10, 2017

Review and explain privacy implications of account deletion #1735

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Users can delete their account #1447

Users can delete their account #1447

beagleknight commented Jun 6, 2017 •

edited

Loading

codecov bot commented Jun 6, 2017 •

edited

Loading

josepjaume left a comment

josepjaume Jun 6, 2017

beagleknight Jun 6, 2017 •

edited

Loading

josepjaume Jun 6, 2017

mrcasals Jun 6, 2017

beagleknight Jun 6, 2017

mrcasals Jun 6, 2017

beagleknight Jun 6, 2017

mrcasals Jun 6, 2017

beagleknight Jun 6, 2017

mrcasals Jun 6, 2017 •

edited

Loading

beagleknight Jun 6, 2017

mrcasals Jun 6, 2017

beagleknight Jun 6, 2017

josepjaume Jun 6, 2017

mrcasals Jun 6, 2017

mrcasals Jun 6, 2017

beagleknight Jun 6, 2017

mrcasals Jun 6, 2017

mrcasals Jun 6, 2017

mrcasals Jun 6, 2017

beagleknight Jun 6, 2017

		field :avatarUrl, !types.String, "The author's avatar url"

		field :deleted, !types.Boolean, "Wheter the author's account has been deleted or not"

Users can delete their account #1447

Users can delete their account #1447

Conversation

beagleknight commented Jun 6, 2017 • edited Loading

🎩 What? Why?

📌 Related Issues

📋 Subtasks

📷 Screenshots (optional)

👻 GIF

codecov bot commented Jun 6, 2017 • edited Loading

Codecov Report

josepjaume left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beagleknight Jun 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrcasals Jun 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beagleknight commented Jun 6, 2017 •

edited

Loading

codecov bot commented Jun 6, 2017 •

edited

Loading

beagleknight Jun 6, 2017 •

edited

Loading

mrcasals Jun 6, 2017 •

edited

Loading