GDPR: Fixes https://github.com/gitcoinco/web/issues/1243 #1246

owocki · 2018-05-23T21:27:19Z

Description

Delete account ability in settings

Checklist

linter status: 100% pass
changes don't break existing behavior
commit message follows commit guidelines

Affected core subsystem(s)

settings

Testing

tested it

Refers/Fixes

#1243

stickler-ci · 2018-05-23T21:27:42Z

app/marketing/views.py

+            logout_redirect = redirect(reverse('logout') + "?next=/")
+            return logout_redirect
+        else:
+            msg = "Error: did not understand your request"


F841 local variable 'msg' is assigned to but never used

Do we want to set msg to response['output'] and wrap this in gettext_lazy ?

stickler-ci · 2018-05-23T21:27:42Z

app/marketing/views.py

+            msg = "Error: did not understand your request"
+
+
+    context = {


E303 too many blank lines (2)

stickler-ci · 2018-05-23T21:27:42Z

app/retail/views.py

@@ -16,6 +16,7 @@
    along with this program. If not, see <http://www.gnu.org/licenses/>.

 '''
+from django.contrib import messages


F401 'django.contrib.messages' imported but unused

codecov · 2018-05-23T21:32:40Z

Codecov Report

Merging #1246 into master will decrease coverage by 0.06%.
The diff coverage is 15.38%.

@@            Coverage Diff             @@
##           master    #1246      +/-   ##
==========================================
- Coverage   31.59%   31.53%   -0.07%     
==========================================
  Files         118      118              
  Lines        8168     8192      +24     
  Branches     1065     1069       +4     
==========================================
+ Hits         2581     2583       +2     
- Misses       5477     5499      +22     
  Partials      110      110

Impacted Files	Coverage Δ
app/app/urls.py	`94.28% <ø> (ø)`	⬆️
app/dashboard/models.py	`63.98% <100%> (ø)`	⬆️
app/marketing/views.py	`15.07% <8.33%> (-0.72%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 99c280d...3fbdb12. Read the comment docs.

SaptakS · 2018-05-23T23:27:20Z

app/app/templates/shared/messages.html

@@ -14,7 +14,7 @@
  You should have received a copy of the GNU Affero General Public License
  along with this program. If not, see <http://www.gnu.org/licenses/>.
 {% endcomment %}
-<script>
+<script id="messages">


what does this change do?

mbeacom

A few minor comments - I'll tackle these, unless there are objections to my comments.

mbeacom · 2018-05-23T23:28:00Z

app/dashboard/models.py

@@ -908,7 +913,7 @@ class Profile(SuperModel):

    """

-    user = models.OneToOneField(User, on_delete=models.CASCADE, null=True, blank=True)
+    user = models.OneToOneField(User, on_delete=models.SET(get_sentinel_user), null=True, blank=True)


Can we just set this to: on_delete=models.SET_NULL - deleted user can only have one associated profile.

mbeacom · 2018-05-23T23:28:57Z

app/marketing/views.py

@@ -66,8 +67,11 @@ def get_settings_navs(request):
        'body': 'Slack',
        'href': reverse('slack_settings'),
    }, {
-        'body': f"{subdomain}{settings.ENS_TLD}",
+        'body': f"ENS",


Might as well eliminate the f here.

mbeacom · 2018-05-23T23:29:08Z

app/marketing/views.py

        'href': reverse('ens_settings'),
+    }, {
+        'body': f"Account",


mbeacom · 2018-05-23T23:30:52Z

app/marketing/views.py

+            logout_redirect = redirect(reverse('logout') + "?next=/")
+            return logout_redirect
+        else:
+            msg = "Error: did not understand your request"


Do we want to set msg to response['output'] and wrap this in gettext_lazy ?

mbeacom · 2018-05-23T23:31:29Z

app/retail/templates/settings/account.html

+<script>
+    $("document").ready(function(){
+        $("#delete").submit(function(e){
+            var _continue = confirm('Are you sure you want to delete all of your data from Gitcoin?');


Should we wrap this in trans?

mbeacom · 2018-05-23T23:32:20Z

app/marketing/views.py

+            profile.hide_profile = True
+            profile.save()
+            request.user.delete()
+            messages.success(request, _(f'Your account has been deleted'))


We can eliminate the f string here and above.

stickler-ci · 2018-05-24T12:51:13Z

app/marketing/views.py

+            logout_redirect = redirect(reverse('logout') + "?next=/")
+            return logout_redirect
+        else:
+            msg = "Error: did not understand your request"


F841 local variable 'msg' is assigned to but never used

SaptakS · 2018-05-24T14:45:53Z

app/dashboard/models.py

@@ -52,6 +53,10 @@
 logger = logging.getLogger(__name__)


+def get_sentinel_user():


I think we can remove this function if we are using SET_NULL. @mbeacom

@SaptakS Addressed in: 8250d24

stickler-ci · 2018-05-24T14:52:06Z

app/dashboard/models.py

@@ -23,6 +23,7 @@
 from urllib.parse import urlsplit

 from django.conf import settings
+from django.contrib.auth import get_user_model


F401 'django.contrib.auth.get_user_model' imported but unused

SaptakS

LGTM

SaptakS

LGTM

GDPR: Fixes #1243

19f4dbf

ghost assigned owocki May 23, 2018

ghost added the in progress label May 23, 2018

owocki mentioned this pull request May 23, 2018

GDPR: DISCONNECT & DELETE #1243

Closed

stickler-ci reviewed May 23, 2018

View reviewed changes

SaptakS mentioned this pull request May 23, 2018

GDPR: Settings Content Updates #1235

Closed

SaptakS reviewed May 23, 2018

View reviewed changes

SaptakS mentioned this pull request May 23, 2018

GDPR: Adds disclosure and consent withdrawal information in privacy tab #1248

Merged

3 tasks

mbeacom reviewed May 23, 2018

View reviewed changes

Merge branch 'master' into GDPR-deleteaccount

5332e74

ghost assigned mbeacom May 24, 2018

stickler-ci reviewed May 24, 2018

View reviewed changes

mbeacom added 2 commits May 24, 2018 09:08

Update per comments

c37070f

Update remaining trans

ede1eb5

SaptakS reviewed May 24, 2018

View reviewed changes

Remove sentinel user method

8250d24

stickler-ci reviewed May 24, 2018

View reviewed changes

Remove unused imports

54d9b7c

SaptakS previously approved these changes May 24, 2018

View reviewed changes

Remove id from script

3fbdb12

mbeacom dismissed SaptakS’s stale review via 3fbdb12 May 24, 2018 14:54

SaptakS approved these changes May 24, 2018

View reviewed changes

thelostone-mc approved these changes May 24, 2018

View reviewed changes

mbeacom merged commit 3deda9b into master May 24, 2018

mbeacom deleted the GDPR-deleteaccount branch May 24, 2018 15:01

ghost removed the in progress label May 24, 2018

mbeacom added a commit that referenced this pull request May 24, 2018

Update migrations since #1246

61b7ecc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GDPR: Fixes https://github.com/gitcoinco/web/issues/1243 #1246

GDPR: Fixes https://github.com/gitcoinco/web/issues/1243 #1246

owocki commented May 23, 2018 •

edited

Loading

stickler-ci May 23, 2018

mbeacom May 23, 2018

stickler-ci May 23, 2018

stickler-ci May 23, 2018

codecov bot commented May 23, 2018 •

edited

Loading

SaptakS May 23, 2018

mbeacom left a comment

mbeacom May 23, 2018

mbeacom May 23, 2018

mbeacom May 23, 2018

mbeacom May 23, 2018

mbeacom May 23, 2018

mbeacom May 23, 2018

stickler-ci May 24, 2018

SaptakS May 24, 2018

mbeacom May 24, 2018

stickler-ci May 24, 2018

SaptakS left a comment

SaptakS left a comment

		@@ -52,6 +53,10 @@
		logger = logging.getLogger(__name__)


		def get_sentinel_user():

GDPR: Fixes https://github.com/gitcoinco/web/issues/1243 #1246

GDPR: Fixes https://github.com/gitcoinco/web/issues/1243 #1246

Conversation

owocki commented May 23, 2018 • edited Loading

Description

Checklist

Affected core subsystem(s)

Testing

Refers/Fixes

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

codecov bot commented May 23, 2018 • edited Loading

Codecov Report

Choose a reason for hiding this comment

mbeacom left a 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

SaptakS left a comment

Choose a reason for hiding this comment

SaptakS left a comment

Choose a reason for hiding this comment

owocki commented May 23, 2018 •

edited

Loading

codecov bot commented May 23, 2018 •

edited

Loading