Implement option to hide total receiving from others (issue #1683) #1721

Changaco · 2013-12-04T18:32:27Z

I saw #1683 on Bountysource… and I accidentally clicked the Start Work button, so I worked on it. :D

tshepang · 2013-12-04T19:52:11Z

sad feature :(

chadwhitacre · 2013-12-06T02:54:27Z

I'm trying to stand this up. I hit #1727, which isn't insurmountable (the server still starts).

chadwhitacre · 2013-12-06T02:55:32Z

schema.sql

@@ -949,3 +949,12 @@ ALTER TABLE homepage_top_givers ADD COLUMN twitter_pic text;
 -- https://github.com/gittip/www.gittip.com/pull/1610

 DROP TABLE homepage_new_participants;
+


This should go in a branch.sql file. Just create one with this content and add it on your branch.

chadwhitacre · 2013-12-06T02:57:47Z

I get this from the homepage:

Traceback (most recent call last):
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/website.py", line 81, in handle_safely
    response = self.handle(request)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/website.py", line 114, in handle
    response = request.resource.respond(request)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/resources/dynamic_resource.py", line 68, in respond
    response = self.get_response(context)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/resources/negotiated_resource.py", line 99, in get_response
    response.body = render(context)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen/renderers/__init__.py", line 99, in __call__
    return self.render_content(context)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/aspen_tornado_renderer.py", line 14, in render_content
    return self.compiled.generate(**context)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/tornado/template.py", line 129, in generate
    return execute()
  File "/Users/whit537/personal/gittip/www.gittip.com/www/index.html.spt", line 193, in _execute
    style="background-image: url('{{ _get_img_src(receiver) }}')">
AttributeError: 'UtterHack' object has no attribute 'anonymous'

chadwhitacre · 2013-12-06T02:57:57Z

I tried working around it by spinning up a fresh database, but then I got this error from make data:

[gittip] $ make data
./env/bin/swaddle local.env ./env/bin/fake_data fake_data
[SWADDLE] Skipping line: .
Traceback (most recent call last):
  File "./env/bin/fake_data", line 9, in <module>
    load_entry_point('gittip==10.1.46-dev', 'console_scripts', 'fake_data')()
  File "/Users/whit537/personal/gittip/www.gittip.com/gittip/utils/fake_data.py", line 231, in main
    populate_db(wireup.db())
  File "/Users/whit537/personal/gittip/www.gittip.com/gittip/utils/fake_data.py", line 159, in populate_db
    participants.append(fake_participant(db))
  File "/Users/whit537/personal/gittip/www.gittip.com/gittip/utils/fake_data.py", line 76, in fake_participant
    , number=number
  File "/Users/whit537/personal/gittip/www.gittip.com/gittip/utils/fake_data.py", line 31, in _fake_thing
    , column_values
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/postgres/__init__.py", line 357, in run
    cursor.run(sql, parameters)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/postgres/cursors.py", line 92, in run
    self.execute(sql, parameters)
  File "/Users/whit537/personal/gittip/www.gittip.com/env/lib/python2.7/site-packages/psycopg2/extras.py", line 288, in execute
    return super(NamedTupleCursor, self).execute(query, vars)
psycopg2.ProgrammingError: column "anonymous_giving" of relation "participants" does not exist
LINE 1: ...ername_lower, is_suspicious, ctime, claimed_time, anonymous_...
                                                             ^

make: *** [data] Error 1
[gittip] $

chadwhitacre · 2013-12-06T03:18:34Z

I was able to run make schema to get an empty database with the new schema. Now I can load the homepage and poke at this feature.

chadwhitacre · 2013-12-06T03:18:55Z

@Changaco We do need to fix make data, though.

chadwhitacre · 2013-12-06T03:32:53Z

@rummik Now that we have jstests, can you walk @Changaco (and the rest of us!) through adding client-side tests for this functionality?

chadwhitacre · 2013-12-06T03:39:37Z

I've confirmed that toggling the checkbox in the UI toggles the value in the database. However, it doesn't remove me from the homepage as I would expect. Presumably community pages are the same.

@Changaco Can you describe more precisely the behavior you're intending with this PR so we can flesh out this feature more and test it properly?

rummik · 2013-12-06T04:10:22Z

@Changaco Would you like to discuss on IRC? Or maybe we can set a time for desktop sharing via Skype, Google+, or shellcast. (There's also the option of a multiplayer SSH + Tmux)

Changaco · 2013-12-06T10:12:25Z

@rummik I'm okay to discuss on IRC.

@whit537 I don't see any problem with make data.

For the behavior, I just duplicated what the anonymous giving option was doing. You can still appear on the homepage and community pages, but "Anonymous" is displayed instead of your user name, your avatar is not displayed, and there is no link to your profile. Screenshot:

chadwhitacre · 2013-12-06T13:30:02Z

I don't see any problem with make data.

Right, sorry. I needed to make schema first.

zbynekwinkler · 2013-12-06T14:20:23Z

branch.sql

+-- https://github.com/gittip/www.gittip.com/issues/1683
+
+ALTER TABLE participants RENAME COLUMN anonymous TO anonymous_giving;
+ALTER TABLE participants DROP COLUMN anonymous;


I am wondering why this works. Isn't the column anonymous called anonymous_giving already here so dropping it should error out?

@zwn SQL errors are ignored unless they appear inside a transaction.

Speaking of which, we should probably do this in a transaction (in addition to moving this to branch.sql).

Sorry, didn't notice we're already in branch.sql. :-)

A transaction made of these two lines would be useless, it would always fail and rollback.

Sorry for saying the obvious but shouldn't we remove the line that does nothing useful and always fails?

@zwn If the anonymous_giving column already exists, the RENAME fails, thus anonymous is still there, but we don't want it, so we DROP it.

if anonymous_giving and anonymous both exist, something is badly wrong. We want to abort and get a human to look at it. Blinding dropping anonymous when something is wrong might loose data we care about.

chadwhitacre · 2013-12-06T16:03:42Z

Figured out what was up: I have UPDATE_HOMEPAGE_EVERY set to 1000 locally (to avoid console spam; it's 60 in production). That's why I wasn't seeing changes to anonymous_receiving showing up on the homepage as I expected. I'm seeing it now.

!m @Changaco

chadwhitacre · 2013-12-06T16:14:38Z

@Changaco I'm impressed that you found top.json. We don't call that internally and we don't advertise it on the API docs. It's actually 500 in production right now:

https://www.gittip.com/about/top.json

I've ticketed it's removal as #1729.

chadwhitacre · 2013-12-06T16:20:22Z

@Changaco I still see "whit537 receives $10.00 per week" on my profile page when viewed anonymously even though I've set anonymous_receiving=true.

chadwhitacre · 2013-12-06T16:22:21Z

@Changaco If I also set anonymous_giving=true then I get "whit537 gives and receives anonymously."

chadwhitacre · 2013-12-06T16:23:58Z

So here's my punchlist so far:

add a transaction to branch.sql
write jstests
fix receiving leak when anonymous_giving is false

chadwhitacre · 2013-12-06T16:24:16Z

@Changaco Back over to you! :-)

bruceadams · 2013-12-22T11:39:18Z

@Changaco is there anything I can do to help you with this? I'm happy to help.

Changaco · 2013-12-27T14:24:30Z

@bruceadams I don't thinks so, but thanks. The reason this PR has stagnated is because I was busy on another project.

@whit537 Do you still want jstests with this ?

chadwhitacre · 2013-12-27T16:14:39Z

@Changaco Yes, we should add a few jstests now that we have a framework in place for that.

Changaco · 2013-12-27T23:00:16Z

@whit537 I'm not sure what jstests I could add for this feature. I had already added python tests, but I don't see a reason to remake them in JS (and I don't see how it would be done for the ones that require access to the DB).

I think this PR should be reviewed and merged as it is, it's already big enough without jstests. The work on these can be done in #1804.

rummik · 2013-12-30T23:48:01Z

@Changaco Yeah, we currently have a couple issues with jstests anyway (there currently isn't a way to test them with a logged in state, and the random data makes it a trick to handle testing JS events -- such as tipping), so I wouldn't worry about it much.

zbynekwinkler · 2014-01-07T02:37:27Z

I'll try to review this PR (starting now).

zbynekwinkler · 2014-01-07T02:59:56Z

Closing in favor of #1847.

ghost assigned chadwhitacre Dec 6, 2013

chadwhitacre reviewed Dec 6, 2013
View reviewed changes

chadwhitacre mentioned this pull request Dec 6, 2013

Add option to hide total receiving from others #1683

Closed

zbynekwinkler reviewed Dec 6, 2013
View reviewed changes

ghost assigned chadwhitacre Dec 6, 2013

chadwhitacre mentioned this pull request Dec 6, 2013

kill top.json #1729

Closed

ghost assigned Changaco Dec 6, 2013

implement option to hide total receiving from others

5183652

Changaco added 9 commits January 1, 2014 13:36

add line break

78b2b1b

fix community pages

f55fe62

replace "???" by "Anonymous"

f5b7597

move SQL changes to branch.sql

58c5467

fix receiving amount leak

f0d6b36

fake_data: randomize anonymous_giving and anonymous_receiving

0f8e88a

put the SQL schema changes in a transaction

1a7b5bd

add missing <span>s with class="(my-)total-giving"

131b908

fix an {% if %} condition

e80b780

zbynekwinkler mentioned this pull request Jan 7, 2014

Implement option to hide total receiving from others (issue #1683) #1847

Merged

zbynekwinkler closed this Jan 7, 2014

Changaco deleted the add-anonymous-receiving branch January 10, 2014 18:19

chadwhitacre mentioned this pull request Nov 12, 2019

Provide anonymized dataset gratipay/inside.gratipay.com#1227

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement option to hide total receiving from others (issue #1683) #1721

Implement option to hide total receiving from others (issue #1683) #1721

Changaco commented Dec 4, 2013

tshepang commented Dec 4, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

rummik commented Dec 6, 2013

Changaco commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

zbynekwinkler Dec 6, 2013

Changaco Dec 6, 2013

chadwhitacre Dec 6, 2013

chadwhitacre Dec 6, 2013

Changaco Dec 6, 2013

zbynekwinkler Dec 6, 2013

chadwhitacre Dec 6, 2013

Changaco Dec 6, 2013

bruceadams Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

bruceadams commented Dec 22, 2013

Changaco commented Dec 27, 2013

chadwhitacre commented Dec 27, 2013

Changaco commented Dec 27, 2013

rummik commented Dec 30, 2013

zbynekwinkler commented Jan 7, 2014

zbynekwinkler commented Jan 7, 2014

		@@ -949,3 +949,12 @@ ALTER TABLE homepage_top_givers ADD COLUMN twitter_pic text;
		-- https://github.com/gittip/www.gittip.com/pull/1610

		DROP TABLE homepage_new_participants;

Implement option to hide total receiving from others (issue #1683) #1721

Implement option to hide total receiving from others (issue #1683) #1721

Conversation

Changaco commented Dec 4, 2013

tshepang commented Dec 4, 2013

chadwhitacre commented Dec 6, 2013

Choose a reason for hiding this comment

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

rummik commented Dec 6, 2013

Changaco commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

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

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

chadwhitacre commented Dec 6, 2013

bruceadams commented Dec 22, 2013

Changaco commented Dec 27, 2013

chadwhitacre commented Dec 27, 2013

Changaco commented Dec 27, 2013

rummik commented Dec 30, 2013

zbynekwinkler commented Jan 7, 2014

zbynekwinkler commented Jan 7, 2014